[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r966822 - in /subversion/trunk/subversion: libsvn_client/repos_diff.c tests/cmdline/merge_tests.py

From: Paul Burba <ptburba_at_gmail.com>
Date: Thu, 12 Aug 2010 18:01:46 -0400

On Tue, Aug 3, 2010 at 12:40 PM, Paul Burba <ptburba_at_gmail.com> wrote:
> Ok, I am waving the white flag as far as implementing a fix for issue
> #3657 in the RA layer.  I simply don't see how it can be done outside
> of this:  Put the DAV update report handler into send-all=TRUE
> <S:update-report send-all=TRUE> mode when creating a
> svn_ra_reporter3_t * during a merge/diff.  This would prevent
> <S:fetch-props/> response that causes the client to fetch *all* the
> properties of a path which subsequently pushes these to the
> svn_delta_editor_t callbacks as if they were all prop *changes* (as
> Mike discussed here
> http://subversion.tigris.org/issues/show_bug.cgi?id=3657#desc9).
>
> fyi: Currently this is how the two DAV RA layers use send-all mode:
>
>               send-all   send-all
>               mode       mode
>  operation    ra_serf    ra_neon
>  ---------    --------   ---------
>  update       FALSE      TRUE
>  status       FALSE      TRUE
>  switch       FALSE      TRUE
>  diff         FALSE      FALSE
>
> In that attached patch I reverted r966822 and tried the aforementioned
> approach by tweaking ra_neon so the send-all mode on diff/merge is
> TRUE.  The whole test suite passes (including the test for issue
> #3657).  I haven't got an equivalent change to ra_serf to work yet,
> but I'm not too worried about that yet because...
>
> ...After testing out this idea, I realized the problem with this
> approach: It reopens issue #2048 'Primary connection (of parallel
> network connections used) in ra-dav diff/merge timeout
> unnecessarily.':
>
>  http://subversion.tigris.org/issues/show_bug.cgi?id=2048
>  http://svn.apache.org/viewvc?view=revision&revision=853457
>
> So I'm at a dead-end right now.  I'm happy to revert r966822 and
> reopen issue #3657 if the consensus is that my initial fix is a sloppy
> band-aid to cover incorrect ra_neon/ra_serf behavior and that the
> latter two are where the fix belongs.
>
> Paul

Mike suggested to me that the fix for this problem can be made in
mod_dav_svn, since the update reporter already knows which properties
have changed, *regardless* of the send-all mode, but it discards this
information if send-all=FALSE and instead instead sends the client a
'fetch-props' response. As described previously in this thread, this
causes the client to fetch all the properties of the path, not just
the changes, and *all* the props are pushed through the editor
callbacks as if they were all actual changes (which again is the core
problem of issue #3657).

I'm working on that approach, but in the meantime I have a question
about the behavior of svn diff --summarize when describing the
addition of paths with properties.

### Let's take a vanilla greek tree, add a file and directory,
### set a prop on each, and commit as r2:

>echo nu file > nu

>svn add nu
  A nu

>svn ps prop val nu
  property 'prop' set on 'nu'

>svn mkdir J
  A J

>svn ps prop val J
  property 'prop' set on 'J'

### Status looks right, two scheduled additions:

>svn st
  A nu
  A J

### Commit it

>svn ci -m ""
  Adding J
  Adding nu
  Transmitting file data .
  Committed revision 2.

### Check diff --summarize for r2

>svn diff -r1:2 --summarize
  AM nu
  AM J

Should we really be describing the props for this added (not copied!)
file as modified? It seems incorrect to call the props modified in
this case. I ask because this is one of the problems I am running
into with fixing this issue in mod_dav_svn, and before I spend more
time trying to "fix" it I want to be sure the current behavior is
really what we expect.

Paul "Dying slowly at the hands of mod_dav_svn" Burba
Received on 2010-08-13 00:02:23 CEST

This is an archived mail posted to the Subversion Dev mailing list.