You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stephen Butler <sb...@elego.de> on 2008/10/06 11:43:40 UTC

Re: Tree conflicts - see what happens in the current state

Quoting Julian Foad <ju...@btopenworld.com>:

> Tree conflict fans,
>
> To become aware of what happens in the current state of tree conflict
> detection, please try the following exercises. You'll see several things
> that work better than in v1.5, and several things that remain to be
> done.

Hi Julian,

Thanks for the exercises.  They were instructive!  I would like to get the
"deep" tree conflict detection tests working.  E.g.,  
tree_conflicts_on_update_* in update_tests.py.

By "working", I mean "not XFAIL".  I'll tweak the expected output to use
your new treeconflict='C' for victims instead of the old status='C ' for
parent-dirs.

On the svn side, we'll need the update/merge/etc. commands to skip tree
conflict victims.  I'm afraid that will break a lot of other tests that
we've discussed already, the ones that create tree conflicts.  But I'm no
stranger to patching tests.

Does this sound like a reasonable interim goal?

(And one more comment below)

Regards,
Steve

>
> [[[
>
> # Apply the attached patch (which adds a pseudo-test to
> update_tests.py).
> patch -p0 < ...
>
> # Run the pseudo-test. [*1]
> cd subversion/tests/cmdline/
> ./update_tests.py 51
>
> # Change directory to the WC.
> cd svn-test-work/working-copies/update_tests-51/
> cd uc1
>
> # Try one of these:
> svn up  # or ...
> svn switch ^/uc2  # or ...
> svn merge -c3 ^/uc1
>
> # Try some of these:
> svn status      # status currently shows on both parent and victim
> svn info .      # info is shown on the parent
> svn commit -m 'try to commit with unresolved conflicts'
> svn resolved .  # resolving is currently done to the parent
> svn commit -m ''
>
> # For UC2, don't be in the directory "uc2" when running "svn up": [*2]
> cd ..
> svn up uc2  # or ...
> svn switch ^/uc2 uc2  # or ...
> svn merge -c3 ^/uc1 uc2
>
> ...
> ]]]
>
>
> The changes set up by the pseudo-test are:
>
> Branch   Incoming change (-c3)  Local scheduled change
> ------   --------------------   --------------------
> uc1/     edit Foo.c             move Foo.c Bar.c
> uc2/     move Foo.c Bar.c       edit Foo.c
> uc3/     move Foo.c Bar.c       move Foo.c Bix.c
>
> These correspond to Use Cases 1/2/3 in
> notes/tree-conflicts/use-cases.txt if you exercise them with "update",
> or cases 4/5/6 if you exercise them with "merge".
>
>
> [*1] I think that's how you would run an individual test if you build in
> the source tree. I build Subversion in an object tree that is separate
> from the source tree, and running an individual test is a bit more
> complex. I need a "bin" directory somewhere (mine is in the source tree)
> containing either installed executables or symlinks to the versions of
> "svn" and "svnadmin" in the object tree. I run the test like this:
> [[[
> (cd $OBJECT/subversion/tests/cmdline/ &&
> $SOURCE/subversion/tests/cmdline/update_tests.py 51 --bin=$SOURCE/bin)
> ]]]
>
> [*2] I just found a bug triggered by UC2 where, if you try to do the
> update from within the directory "uc2" itself, it throws an error
> because it doesn't like raising a tree conflict in a directory whose
> path is passed as the empty string "".

This is because add_file_with_history() (update_editor.c:3314) calls
check_tree_conflict() with the parent path instead of the victim path.

I think we don't need to check for tree conflicts in that function,
because it's called from the add_file() callback.  It calls its caller,
so we end up checking for tree conflicts twice.  Maybe I can unroll
the recursion.

--Steve

>
>
> - Julian
>
>



-- 
Stephen Butler | Software Developer
elego Software Solutions GmbH
Gustav-Meyer-Allee 25 | 13355 Berlin | Germany
fon: +49 30 2345 8696 | mobile: +49 163 25 45 015
fax: +49 30 2345 8695 | http://www.elegosoft.com
Geschäftsführer: Olaf Wagner | Sitz der Gesellschaft: Berlin
Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: Tree conflicts - see what happens in the current state

Posted by Julian Foad <ju...@btopenworld.com>.
On Wed, 2008-10-08 at 15:38 +0100, Julian Foad wrote:
> On Wed, 2008-10-08 at 14:47 +0100, Julian Foad wrote:
> > On Mon, 2008-10-06 at 13:43 +0200, Stephen Butler wrote:
> > > I think we don't need to check for tree conflicts in that function,
> > > because it's called from the add_file() callback.  It calls its caller,
> > > so we end up checking for tree conflicts twice.  Maybe I can unroll
> > > the recursion.
> > 
> > That would be good.
> 
> That's not only good but it was necessary to stop it checking twice in
> order to fix the above bug properly.
> 
> Committed revision 33549.
> 
> I am now unrolling the recursion because that was just confusing and
> unnecessary...

Committed revision 33554.

- Julian



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Tree conflicts - see what happens in the current state

Posted by Julian Foad <ju...@btopenworld.com>.
On Wed, 2008-10-08 at 14:47 +0100, Julian Foad wrote:
> On Mon, 2008-10-06 at 13:43 +0200, Stephen Butler wrote:
> > Quoting Julian Foad <ju...@btopenworld.com>:
> > > [*2] I just found a bug triggered by UC2 where, if you try to do the
> > > update from within the directory "uc2" itself, it throws an error
> > > because it doesn't like raising a tree conflict in a directory whose
> > > path is passed as the empty string "".
> > 
> > This is because add_file_with_history() (update_editor.c:3314) calls
> > check_tree_conflict() with the parent path instead of the victim path.
> 
> Thanks. Committed revision 33545.
> 
> And the same call was passing an otherwise-unused log accumulator to
> store any conflict data in, where it should have been passing the
> existing parent directory's log accumulator. Fixing now...
> 
> > I think we don't need to check for tree conflicts in that function,
> > because it's called from the add_file() callback.  It calls its caller,
> > so we end up checking for tree conflicts twice.  Maybe I can unroll
> > the recursion.
> 
> That would be good.

That's not only good but it was necessary to stop it checking twice in
order to fix the above bug properly.

Committed revision 33549.

I am now unrolling the recursion because that was just confusing and
unnecessary...

- Julian



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Tree conflicts - see what happens in the current state

Posted by Julian Foad <ju...@btopenworld.com>.
On Mon, 2008-10-06 at 13:43 +0200, Stephen Butler wrote:
> Quoting Julian Foad <ju...@btopenworld.com>:
> 
> > Tree conflict fans,
> >
> > To become aware of what happens in the current state of tree conflict
> > detection, please try the following exercises. You'll see several things
> > that work better than in v1.5, and several things that remain to be
> > done.
> 
> Hi Julian,
> 
> Thanks for the exercises.  They were instructive!  I would like to get the
> "deep" tree conflict detection tests working.  E.g.,  
> tree_conflicts_on_update_* in update_tests.py.
> 
> By "working", I mean "not XFAIL".  I'll tweak the expected output to use
> your new treeconflict='C' for victims instead of the old status='C ' for
> parent-dirs.
> 
> On the svn side, we'll need the update/merge/etc. commands to skip tree
> conflict victims.  I'm afraid that will break a lot of other tests that
> we've discussed already, the ones that create tree conflicts.  But I'm no
> stranger to patching tests.
> 
> Does this sound like a reasonable interim goal?
> 
> (And one more comment below)
> 
> Regards,
> Steve
> 
> >
> > [[[
> >
> > # Apply the attached patch (which adds a pseudo-test to
> > update_tests.py).
> > patch -p0 < ...
> >
> > # Run the pseudo-test. [*1]
> > cd subversion/tests/cmdline/
> > ./update_tests.py 51
> >
> > # Change directory to the WC.
> > cd svn-test-work/working-copies/update_tests-51/
> > cd uc1
> >
> > # Try one of these:
> > svn up  # or ...
> > svn switch ^/uc2  # or ...
> > svn merge -c3 ^/uc1
> >
> > # Try some of these:
> > svn status      # status currently shows on both parent and victim
> > svn info .      # info is shown on the parent
> > svn commit -m 'try to commit with unresolved conflicts'
> > svn resolved .  # resolving is currently done to the parent
> > svn commit -m ''
> >
> > # For UC2, don't be in the directory "uc2" when running "svn up": [*2]
> > cd ..
> > svn up uc2  # or ...
> > svn switch ^/uc2 uc2  # or ...
> > svn merge -c3 ^/uc1 uc2
> >
> > ...
> > ]]]
> >
> >
> > The changes set up by the pseudo-test are:
> >
> > Branch   Incoming change (-c3)  Local scheduled change
> > ------   --------------------   --------------------
> > uc1/     edit Foo.c             move Foo.c Bar.c
> > uc2/     move Foo.c Bar.c       edit Foo.c
> > uc3/     move Foo.c Bar.c       move Foo.c Bix.c
> >
> > These correspond to Use Cases 1/2/3 in
> > notes/tree-conflicts/use-cases.txt if you exercise them with "update",
> > or cases 4/5/6 if you exercise them with "merge".
> >
> >
> > [*1] I think that's how you would run an individual test if you build in
> > the source tree. I build Subversion in an object tree that is separate
> > from the source tree, and running an individual test is a bit more
> > complex. I need a "bin" directory somewhere (mine is in the source tree)
> > containing either installed executables or symlinks to the versions of
> > "svn" and "svnadmin" in the object tree. I run the test like this:
> > [[[
> > (cd $OBJECT/subversion/tests/cmdline/ &&
> > $SOURCE/subversion/tests/cmdline/update_tests.py 51 --bin=$SOURCE/bin)
> > ]]]
> >
> > [*2] I just found a bug triggered by UC2 where, if you try to do the
> > update from within the directory "uc2" itself, it throws an error
> > because it doesn't like raising a tree conflict in a directory whose
> > path is passed as the empty string "".
> 
> This is because add_file_with_history() (update_editor.c:3314) calls
> check_tree_conflict() with the parent path instead of the victim path.

Thanks. Committed revision 33545.

And the same call was passing an otherwise-unused log accumulator to
store any conflict data in, where it should have been passing the
existing parent directory's log accumulator. Fixing now...

> I think we don't need to check for tree conflicts in that function,
> because it's called from the add_file() callback.  It calls its caller,
> so we end up checking for tree conflicts twice.  Maybe I can unroll
> the recursion.

That would be good.

- Julian



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Tree conflicts - see what happens in the current state

Posted by Stephen Butler <sb...@elego.de>.
Quoting Julian Foad <ju...@btopenworld.com>:

> On Tue, 2008-10-07 at 22:49 +0100, Julian Foad wrote:
>> On Mon, 2008-10-06 at 13:43 +0200, Stephen Butler wrote:
>> > On the svn side, we'll need the update/merge/etc. commands to skip tree
>> > conflict victims.  I'm afraid that will break a lot of other tests that
>> > we've discussed already, the ones that create tree conflicts.  But I'm no
>> > stranger to patching tests.
>> >
>> > Does this sound like a reasonable interim goal?
>>
>> Yes, that's perhaps the main coding task we need to do now.
>
> Here's what I think we need to do.
>
> After placing a directory into conflict in dir_open(), all edits in the
> victim sub-tree must be inhibited until the corresponding dir_close().
> All of the normal editor call-backs can be called by the edit driver
> within this victim sub-tree, so they all must recognise that the edit
> drive is in this state and skip their normal work. In dir_close(), if
> closing _this_ victim dir (rather than one of its sub-dirs), that's when
> we get out of this "skipping" state.
>
> Under edit drive rules, there can only be one such victim sub-tree being
> processed (skipped) at any time, so there only needs to be one flag
> saying "we're currently processing within a victim sub-tree", and one
> way to identify which directory is the root of the sub-tree being
> skipped. There does not need to be a list or tree of such flags.
>
> A way to implement this, I think, is with a flag in the main edit baton
> saying "we're currently skipping", and a flag in the directory baton
> saying "this is the root of the sub-tree being skipped". All edit
> functions check the former, and the close_dir() function resets both
> flags when it finds the latter set.

Sounds good.  Maybe a similar approach will solve the "double delete"
problem during merge, where the client sends redundant delete requests
while walking a tree that's just been deleted on the server.

Steve



-- 
Stephen Butler | Software Developer
elego Software Solutions GmbH
Gustav-Meyer-Allee 25 | 13355 Berlin | Germany
fon: +49 30 2345 8696 | mobile: +49 163 25 45 015
fax: +49 30 2345 8695 | http://www.elegosoft.com
Geschäftsführer: Olaf Wagner | Sitz der Gesellschaft: Berlin
Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: Tree conflicts - see what happens in the current state

Posted by Julian Foad <ju...@btopenworld.com>.
On Tue, 2008-10-07 at 22:49 +0100, Julian Foad wrote:
> On Mon, 2008-10-06 at 13:43 +0200, Stephen Butler wrote:
> > On the svn side, we'll need the update/merge/etc. commands to skip tree
> > conflict victims.  I'm afraid that will break a lot of other tests that
> > we've discussed already, the ones that create tree conflicts.  But I'm no
> > stranger to patching tests.
> > 
> > Does this sound like a reasonable interim goal?
> 
> Yes, that's perhaps the main coding task we need to do now.

Here's what I think we need to do.

After placing a directory into conflict in dir_open(), all edits in the
victim sub-tree must be inhibited until the corresponding dir_close().
All of the normal editor call-backs can be called by the edit driver
within this victim sub-tree, so they all must recognise that the edit
drive is in this state and skip their normal work. In dir_close(), if
closing _this_ victim dir (rather than one of its sub-dirs), that's when
we get out of this "skipping" state.

Under edit drive rules, there can only be one such victim sub-tree being
processed (skipped) at any time, so there only needs to be one flag
saying "we're currently processing within a victim sub-tree", and one
way to identify which directory is the root of the sub-tree being
skipped. There does not need to be a list or tree of such flags.

A way to implement this, I think, is with a flag in the main edit baton
saying "we're currently skipping", and a flag in the directory baton
saying "this is the root of the sub-tree being skipped". All edit
functions check the former, and the close_dir() function resets both
flags when it finds the latter set.

- Julian



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Tree conflicts - see what happens in the current state

Posted by Julian Foad <ju...@btopenworld.com>.
On Mon, 2008-10-06 at 13:43 +0200, Stephen Butler wrote:
> Quoting Julian Foad <ju...@btopenworld.com>:
> 
> > Tree conflict fans,
> >
> > To become aware of what happens in the current state of tree conflict
> > detection, please try the following exercises. You'll see several things
> > that work better than in v1.5, and several things that remain to be
> > done.
> 
> Hi Julian,
> 
> Thanks for the exercises.  They were instructive!  I would like to get the
> "deep" tree conflict detection tests working.  E.g.,  
> tree_conflicts_on_update_* in update_tests.py.
> 
> By "working", I mean "not XFAIL".  I'll tweak the expected output to use
> your new treeconflict='C' for victims instead of the old status='C ' for
> parent-dirs.
> 
> On the svn side, we'll need the update/merge/etc. commands to skip tree
> conflict victims.  I'm afraid that will break a lot of other tests that
> we've discussed already, the ones that create tree conflicts.  But I'm no
> stranger to patching tests.
> 
> Does this sound like a reasonable interim goal?

Yes, that's perhaps the main coding task we need to do now.

> (And one more comment below)
> 
> Regards,
> Steve
> 
> >
> > [[[
> >
> > # Apply the attached patch (which adds a pseudo-test to
> > update_tests.py).
> > patch -p0 < ...
> >
> > # Run the pseudo-test. [*1]
> > cd subversion/tests/cmdline/
> > ./update_tests.py 51
> >
> > # Change directory to the WC.
> > cd svn-test-work/working-copies/update_tests-51/
> > cd uc1
> >
> > # Try one of these:
> > svn up  # or ...
> > svn switch ^/uc2  # or ...
> > svn merge -c3 ^/uc1
> >
> > # Try some of these:
> > svn status      # status currently shows on both parent and victim
> > svn info .      # info is shown on the parent
> > svn commit -m 'try to commit with unresolved conflicts'
> > svn resolved .  # resolving is currently done to the parent
> > svn commit -m ''
> >
> > # For UC2, don't be in the directory "uc2" when running "svn up": [*2]
> > cd ..
> > svn up uc2  # or ...
> > svn switch ^/uc2 uc2  # or ...
> > svn merge -c3 ^/uc1 uc2
> >
> > ...
> > ]]]
> >
> >
> > The changes set up by the pseudo-test are:
> >
> > Branch   Incoming change (-c3)  Local scheduled change
> > ------   --------------------   --------------------
> > uc1/     edit Foo.c             move Foo.c Bar.c
> > uc2/     move Foo.c Bar.c       edit Foo.c
> > uc3/     move Foo.c Bar.c       move Foo.c Bix.c
> >
> > These correspond to Use Cases 1/2/3 in
> > notes/tree-conflicts/use-cases.txt if you exercise them with "update",
> > or cases 4/5/6 if you exercise them with "merge".
> >
> >
> > [*1] I think that's how you would run an individual test if you build in
> > the source tree. I build Subversion in an object tree that is separate
> > from the source tree, and running an individual test is a bit more
> > complex. I need a "bin" directory somewhere (mine is in the source tree)
> > containing either installed executables or symlinks to the versions of
> > "svn" and "svnadmin" in the object tree. I run the test like this:
> > [[[
> > (cd $OBJECT/subversion/tests/cmdline/ &&
> > $SOURCE/subversion/tests/cmdline/update_tests.py 51 --bin=$SOURCE/bin)
> > ]]]
> >
> > [*2] I just found a bug triggered by UC2 where, if you try to do the
> > update from within the directory "uc2" itself, it throws an error
> > because it doesn't like raising a tree conflict in a directory whose
> > path is passed as the empty string "".
> 
> This is because add_file_with_history() (update_editor.c:3314) calls
> check_tree_conflict() with the parent path instead of the victim path.

Ah, thanks for finding that.

> I think we don't need to check for tree conflicts in that function,
> because it's called from the add_file() callback.  It calls its caller,
> so we end up checking for tree conflicts twice.  Maybe I can unroll
> the recursion.

It would be great if you can. I'll take a look too.

- Julian



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org