Overwriting properties AND EditorV2 relation (Was: RE: Mergeinfo overwritten from successive merges)

[Bert Huijben <be...@qqmail.nl>]

> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: vrijdag 14 maart 2014 23:14
> To: 'Philip Martin'
> Cc: 'Branko Čibej'; 'Subversion Development'
> Subject: RE: Mergeinfo overwritten from successive merges
> 
> 
> 
> > -----Original Message-----
> > From: Philip Martin [mailto:philip.martin@wandisco.com]
> > Sent: vrijdag 14 maart 2014 15:17
> > To: Bert Huijben
> > Cc: Branko Čibej; Subversion Development
> > Subject: Re: Mergeinfo overwritten from successive merges
> >
> > Bert Huijben <be...@qqmail.nl> writes:
> >
> > > The base revision for out of date checks could be different per
> > > node... (E.g. Mixed revision working copies). At which revision we
> > > open the transaction shouldn't really matter if we check the revision
> > > for the individual nodes.
> > >
> > > For content changes we supply the baseline rev for v2 via an
> > > header... And my guess would be that we forgot to do that for the
> > > proppatch.(v1 supplied base URL). Most tests in our test suite would
> > > check for out of date via the root of the commit editor... So that
> > > would explain things.
> >
> > The PROPPATCH looks like:
> >
> > PROPPATCH /obj/repo/!svn/txr/4-4/B HTTP/1.1\r
> > Host: localhost:9630\r
> > User-Agent: SVN/1.9.0-dev (x86_64-unknown-linux-gnu) serf/1.3.4\r
> > DAV: http://subversion.tigris.org/xmlns/dav/svn/depth\r
> > DAV: http://subversion.tigris.org/xmlns/dav/svn/mergeinfo\r
> > DAV: http://subversion.tigris.org/xmlns/dav/svn/log-revprops\r
> > X-SVN-Version-Name: 3\r
> > Transfer-Encoding: chunked\r
> > \r
> > 132\r
> > <?xml version="1.0" encoding="utf-8"?><D:propertyupdate
> xmlns:D="DAV:"
> > xmlns:V="http://subversion.tigris.org/xmlns/dav/"
> > xmlns:C="http://subversion.tigris.org/xmlns/custom/"
> >
> xmlns:S="http://subversion.tigris.org/xmlns/svn/"><D:set><D:prop><S:mer
> > geinfo>/A:3</S:mergeinfo></D:prop></D:set></D:propertyupdate>\r
> > 0\r
> > \r
> >
> > So we are sending X-SVN-Version-Name and in this case it does indicate
> > that the resource is out-of-date as B was modified in r4.
> >
> > It seems that the do_out_of_date_check is not working properly:
> >
> > #0  do_out_of_date_check (comb=0x7faeaeb210b8, r=0x7faeaeb590a0)
> >     at ../src/subversion/mod_dav_svn/repos.c:1794
> > 1794	  if (comb->priv.version_name < created_rev)
> > (gdb) p r->the_request
> > $14 = 0x7faeaeb5a610 "PROPPATCH /obj/repo/!svn/txr/4-4/B HTTP/1.1"
> > (gdb) p comb->priv.version_name
> > $15 = 3
> > (gdb) p created_rev
> > $16 = -1
> >
> > SVN_INVALID_REVNUM is less than the baseline revision but the check
> > makes no sense.
> 
> I haven't gotten to the full details yet, but...
> 
> The check would make sense for a 'checkout' that is required before any
> change in a subtree, which is exactly what happens in http v1.
> And it also works for leaves of the tree, BUT NOT for parent directories
> when one of its descendants is already modified... which is what happens
in
> v2.
> 
> I added a python test for this issue in r1577739.
> (Based on Brane's script, but using just one working copy)

One additional thought.

Editor V2 aims to remove the depth first restriction for the other RA layers
(opening the intermediate dirs; which allows the out of date checks), which
is exactly what currently catches this class of out of date error on the
other ra layers.

Is there some way the FS layer could still deliver that original revision
after making parts of the transaction mutable?
That would make this easy to fix and I think we will also need it for a true
editor v2 commit.

Without it the EditorV2 commit won't be able to trigger this out of date
error in even more of the scenarios, it was designed for...


Interesting thought that this matches the problems with the tree conflict
handling on the client side... I didn't think of that before this issue.



	Bert

Re: Overwriting properties AND EditorV2 relation (Was: RE: Mergeinfo overwritten from successive merges)

[Stefan Fuhrmann <st...@wandisco.com>]

On Sat, Mar 15, 2014 at 3:03 PM, Bert Huijben <be...@qqmail.nl> wrote:

>
>
> > -----Original Message-----
> > From: Bert Huijben [mailto:bert@qqmail.nl]
> >
> > > -----Original Message-----
> > > From: Philip Martin [mailto:philip.martin@wandisco.com]
>
...

> > >
> > > SVN_INVALID_REVNUM is less than the baseline revision but the check
> > > makes no sense.
> >
> > I haven't gotten to the full details yet, but...
> >
> > The check would make sense for a 'checkout' that is required before any
> > change in a subtree, which is exactly what happens in http v1.
> > And it also works for leaves of the tree, BUT NOT for parent directories
> > when one of its descendants is already modified... which is what happens
> in
> > v2.
> >
> > I added a python test for this issue in r1577739.
> > (Based on Brane's script, but using just one working copy)
>
> One additional thought.
>
> Editor V2 aims to remove the depth first restriction for the other RA
> layers
> (opening the intermediate dirs; which allows the out of date checks), which
> is exactly what currently catches this class of out of date error on the
> other ra layers.
>
> Is there some way the FS layer could still deliver that original revision
> after making parts of the transaction mutable?
>

The information is trivially available inside FSFS, FSX
and probably BDB (e.g. via noderev predecessor ID) -
as long as the respective path has not been deleted
within that transaction. So far, there is no API for it.


> That would make this easy to fix and I think we will also need it for a
> true
> editor v2 commit.
>
> Without it the EditorV2 commit won't be able to trigger this out of date
> error in even more of the scenarios, it was designed for...
>
>
> Interesting thought that this matches the problems with the tree conflict
> handling on the client side... I didn't think of that before this issue.
>

-- Stefan^2.