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

Re: tree-conflicts: please review to determine desired behaviour in detail

From: Stephen Butler <sbutler_at_elego.de>
Date: Mon, 01 Sep 2008 10:08:09 +0200

Quoting Neels Hofmeyr <neels_at_elego.de>:

> Hi tree-conflicts,
>
> I am going through the tree-conflicts tests that use deep_trees (in
> subversion/tests/cmdline/update_tests.py, switch_tests.py and
> merge_tests.py).
>
> Currently, these pass the exact behaviour that the svn tree-conflicts branch
> shows now (r32837). I want to change the tests so that they will pass the
> actually desired behaviour, showing what still needs to be fixed.
>
> But, when going into detail, it is not quite easy to determine the desired
> behaviour.
>
> I'm pasting the expected trees from the tests as they are now, which show
> the behaviour of svn as it is now (r32837), and add my comments (see
> attached file). Please review to death.
>
> Thanks!
> ~Neels

Hi Neels,

Here are my comments on the update behavior. I'll start with the first
two test functions to get the ball rolling.

Following notes/tree-conflicts/detection.txt, we want 'svn update' to skip
any directory scheduled for deletion, and to instead record the tree
conflict in the parent directory.

> -------------------------------------------------------------
> --- UPDATE --------------------------------------------------
> -------------------------------------------------------------
>
>
> tree_conflicts_on_update_1_1
> Local tree delete, incoming leaf edit.
>
> [[[
> expected_output = wc.State('', {
> 'F' : Item(status='C '),
> 'F/alpha' : Item(status='U '),
> 'D' : Item(status='C '),
> 'D/D1/delta' : Item(status='A '),
> 'DF' : Item(status='C '),
> 'DF/D1' : Item(status='C '),
> 'DF/D1/beta' : Item(status='U '),
> 'DD' : Item(status='C '),
> 'DD/D1' : Item(status='C '),
> 'DD/D1/D2/epsilon' : Item(status='A '),
> 'DDF' : Item(status='C '),
> 'DDF/D1' : Item(status='C '),
> 'DDF/D1/D2' : Item(status='C '),
> 'DDF/D1/D2/gamma' : Item(status='U '),
> 'DDD' : Item(status='C '),
> 'DDD/D1' : Item(status='C '),
> 'DDD/D1/D2' : Item(status='C '),
> 'DDD/D1/D2/D3/zeta' : Item(status='A '),
> })
> ]]]
> Good: Local tree delete is "undone". Incoming leaf edit is carried
> out (marked
> U and A), all directories are in conflict.

In detection.txt, we have:

   ==========
   USE CASE 1
   ==========

   Files:

   If 'svn update' modifies a file that has been scheduled for deletion
   in the working copy, the file is a tree conflict victim.

   Directories:

   If 'svn update' opens a directory that is scheduled for deletion in
   the working copy, the directory is a tree conflict victim. The update
   of the directory and its children will be skipped.

The corresponding output would show that alpha (in a non-deleted dir) has
been updated and the other locally deleted items skipped.

[[[
   expected_output = wc.State('', {
     'F' : Item(status='C '),
     'F/alpha' : Item(status='U '),
     'D' : Item(status='C '),
     'DF' : Item(status='C '),
     'DD' : Item(status='C '),
     'DDF' : Item(status='C '),
     'DDD' : Item(status='C '),
     })
]]]

Maybe we need a 'Skipped due to tree conflict' message for each such tree?

> [[[
> expected_disk = state_after_leaf_edit
> ]]]
> Good: The incoming leaf edit is on the disk.

Bad! ;) We want the editing of scheduled-delete trees to be skipped.
A modification to the original code in svntest/actions.py:

deep_trees_after_leaf_edit = wc.State('', {
   'F/alpha' : Item("This is the file 'alpha'.\nMore text
for file alpha.\n"),
   'D/D1' : Item(),
   'DF/D1/beta' : Item("This is the file 'beta'.\n"),
   'DD/D1/D2' : Item(),
   'DDF/D1/D2/gamma' : Item("This is the file 'gamma'.\n"),
   'DDD/D1/D2/D3' : Item(),
    })

> [[[
> expected_status = wc.State('', {
> '' : Item(status=' ', wc_rev='3'),
> 'F' : Item(status='C ', wc_rev='3'),
> 'F/alpha' : Item(status='D ', wc_rev='3'),
> 'D' : Item(status='C ', wc_rev='3'),
> 'D/D1' : Item(status='D ', wc_rev='3'),
> 'D/D1/delta' : Item(status=' ', wc_rev='3'),
> 'DF' : Item(status='C ', wc_rev='3'),
> 'DF/D1' : Item(status='D ', wc_rev='3'),
> 'DF/D1/beta' : Item(status='D ', wc_rev='3'),
> 'DD' : Item(status='C ', wc_rev='3'),
> 'DD/D1' : Item(status='D ', wc_rev='3'),
> 'DD/D1/D2' : Item(status='D ', wc_rev='3'),
> 'DD/D1/D2/epsilon' : Item(status=' ', wc_rev='3'),
> 'DDF' : Item(status='C ', wc_rev='3'),
> 'DDF/D1' : Item(status='D ', wc_rev='3'),
> 'DDF/D1/D2' : Item(status='D ', wc_rev='3'),
> 'DDF/D1/D2/gamma' : Item(status='D ', wc_rev='3'),
> 'DDD' : Item(status='C ', wc_rev='3'),
> 'DDD/D1' : Item(status='D ', wc_rev='3'),
> 'DDD/D1/D2' : Item(status='D ', wc_rev='3'),
> 'DDD/D1/D2/D3' : Item(status='D ', wc_rev='3'),
> 'DDD/D1/D2/D3/zeta' : Item(status=' ', wc_rev='3'),
> })
> ]]]
> ** BAD! **

Only a little bad. Just a bit naughty.

> All directories should be marked C.

Only the "container" dirs should be marked C, because the update should
skip the scheduled-delete trees inside.

>
> Directories are marked for deletion, even if they contain files that are not
> marked for deletion (delta, epsilon, zeta).

The items not marked for deletion shouldn't have been added by the update.

>
> All files that were edited in the update (alpha, beta, gamma) are
> still marked
> for deletion. Should they be unmarked?

An open question. I propose we don't undo the user's deletion automatically
unless they provide the --accept=theirs option.

The status I would expect:

[[[
   expected_status = wc.State('', {
     '' : Item(status=' ', wc_rev='3'),
     'F' : Item(status='C ', wc_rev='3'),
     'F/alpha' : Item(status='D ', wc_rev='3'),
     'D' : Item(status='C ', wc_rev='3'),
     'D/D1' : Item(status='D ', wc_rev='3'),
     'DF' : Item(status='C ', wc_rev='3'),
     'DF/D1' : Item(status='D ', wc_rev='3'),
     'DD' : Item(status='C ', wc_rev='3'),
     'DD/D1' : Item(status='D ', wc_rev='3'),
     'DD/D1/D2' : Item(status='D ', wc_rev='3'),
     'DDF' : Item(status='C ', wc_rev='3'),
     'DDF/D1' : Item(status='D ', wc_rev='3'),
     'DDF/D1/D2' : Item(status='D ', wc_rev='3'),
     'DDF/D1/D2/gamma' : Item(status='D ', wc_rev='3'),
     'DDD' : Item(status='C ', wc_rev='3'),
     'DDD/D1' : Item(status='D ', wc_rev='3'),
     'DDD/D1/D2' : Item(status='D ', wc_rev='3'),
     'DDD/D1/D2/D3' : Item(status='D ', wc_rev='3'),
     })
]]]

No difference except that no files were added by the update.

Here is the next test scheme:

> tree_conflicts_on_update_1_2
> Local tree delete, incoming leaf delete
>
> [[[
> expected_output = wc.State('', {
> 'F' : Item(status='C '),
> 'F/alpha' : Item(status='D '),
> 'D' : Item(status='C '),
> 'D/D1' : Item(status='D '),
> 'DF' : Item(status='C '),
> 'DF/D1' : Item(status='C '),
> 'DF/D1/beta' : Item(status='D '),
> 'DD' : Item(status='C '),
> 'DD/D1' : Item(status='C '),
> 'DD/D1/D2' : Item(status='D '),
> 'DDF' : Item(status='C '),
> 'DDF/D1' : Item(status='C '),
> 'DDF/D1/D2' : Item(status='C '),
> 'DDF/D1/D2/gamma' : Item(status='D '),
> 'DDD' : Item(status='C '),
> 'DDD/D1' : Item(status='C '),
> 'DDD/D1/D2' : Item(status='C '),
> 'DDD/D1/D2/D3' : Item(status='D '),
> })
> ]]]
> Good: Tries to carry out incoming leaf del markings on top of the local
> markings from tree del, results in conflicts.

Bad: svn should skip the update of each tree scheduled for deletion. Only
the parent of each deleted tree should be marked conflicted, assuming that
the update starts in a non-deleted dir (as is the case here).

[[[
   expected_output = wc.State('', {
     'F' : Item(status='C '),
     'F/alpha' : Item(status='D '),
     'D' : Item(status='C '),
     'DF' : Item(status='C '),
     'DD' : Item(status='C '),
     'DDF' : Item(status='C '),
     'DDD' : Item(status='C '),
     })
]]]

>
> [[[
> expected_disk = state_after_leaf_del
> ]]]
> Good: Files have been removed, but directories remain to be removed by a
> commit. Local working copy unchanged.

Bad: the file alpha (in a non-deleted dir) should be removed, but the other
items should remain.

deep_trees_after_leaf_edit = wc.State('', {
   'F' : Item(),
   'D/D1' : Item(),
   'DF/D1/beta' : Item("This is the file 'beta'.\n"),
   'DD/D1/D2' : Item(),
   'DDF/D1/D2/gamma' : Item("This is the file 'gamma'.\n"),
   'DDD/D1/D2/D3' : Item(),
    })

>
> [[[
> expected_status = wc.State('', {
> '' : Item(status=' ', wc_rev='3'),
> 'F' : Item(status='C ', wc_rev='3'),
> 'D' : Item(status='C ', wc_rev='3'),
> 'DF' : Item(status='C ', wc_rev='3'),
> 'DF/D1' : Item(status='D ', wc_rev='3'),
> 'DD' : Item(status='C ', wc_rev='3'),
> 'DD/D1' : Item(status='D ', wc_rev='3'),
> 'DDF' : Item(status='C ', wc_rev='3'),
> 'DDF/D1' : Item(status='D ', wc_rev='3'),
> 'DDF/D1/D2' : Item(status='D ', wc_rev='3'),
> 'DDD' : Item(status='C ', wc_rev='3'),
> 'DDD/D1' : Item(status='D ', wc_rev='3'),
> 'DDD/D1/D2' : Item(status='D ', wc_rev='3'),
> })
> ]]]
> ** BAD! **
> According to the output tree above, all these directories should be marked C.
> D/D1, DD/D1/D2, DDD/D1/D2/D3 have become unversioned -- is this ok?

There should be no unversioned items, due to the skipped updates.

Should be the same as the 1_1 case except that F/alpha is gone:

[[[
   expected_status = wc.State('', {
     '' : Item(status=' ', wc_rev='3'),
     'F' : Item(status='C ', wc_rev='3'),
     'D' : Item(status='C ', wc_rev='3'),
     'D/D1' : Item(status='D ', wc_rev='3'),
     'DF' : Item(status='C ', wc_rev='3'),
     'DF/D1' : Item(status='D ', wc_rev='3'),
     'DD' : Item(status='C ', wc_rev='3'),
     'DD/D1' : Item(status='D ', wc_rev='3'),
     'DD/D1/D2' : Item(status='D ', wc_rev='3'),
     'DDF' : Item(status='C ', wc_rev='3'),
     'DDF/D1' : Item(status='D ', wc_rev='3'),
     'DDF/D1/D2' : Item(status='D ', wc_rev='3'),
     'DDF/D1/D2/gamma' : Item(status='D ', wc_rev='3'),
     'DDD' : Item(status='C ', wc_rev='3'),
     'DDD/D1' : Item(status='D ', wc_rev='3'),
     'DDD/D1/D2' : Item(status='D ', wc_rev='3'),
     'DDD/D1/D2/D3' : Item(status='D ', wc_rev='3'),
     })
]]]

I suppose the good news is that the expected values for both
test are more similar.

I'll have a look at the other update scenarios in another mail.

Regards,
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_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-01 10:08:29 CEST

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