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

Re: branch tree-conflicts-notify: merge to trunk?

From: Stephen Butler <sbutler_at_elego.de>
Date: Sat, 25 Oct 2008 14:15:43 +0200

Quoting "Neels J. Hofmeyr" <neels_at_elego.de>:

> Hey Steve! (sbutler)
>
> That's so awesome! Somehow you've magically fixed almost everything on the
> branch! All I had to do was raise and notify a tree-conflict on file
> addition in a conflicted directory.

Thanks!

BTW, I think we should (silently) skip a file-add in a tree-conflicted
tree, rather than flag a new tree conflict. And of course skip all
other changes, too. Otherwise the user might see a whole treeful of
t.c. victims, and not be sure where the problem started. A single
tree conflict, raised at the root of the tree during open_directory(),
should be enough.

>
> With that, one thing remains: There's still some differentiation needed
> between raised and persisted tree-conflicts. But I guess we can treat this
> separately (in need of more tests, see below).
>
> Otherwise, the "only" thing that can lead update_tests.py to perfection is
> the implementation of dirs_same_p() a.k.a. diff_summarize, to be able to see
> whether a subtree we're deleting is really the same one.
>
>
> ...And discussion on how exactly the output should be.
> E.g., I think it is nice like this:
>
> A C alpha
> D C beta
> M C gamma
>
> While, right now, update notifies more like this:
>
> A C alpha
> D C beta
> C gamma
>
> Should we add the M or lose the A and D? I'd say add the M and whatever else
> would have been shown without a tree-conflict, to be able to see what was
> tried by the update. (Maybe there are more cases to this, though.)

I believe the first two columns tell the user what changes the update
has made in their working copy. If a path is skipped due to a tree
conflict, we should leave these columns blank.

Of course, we plan to skip all tree conflict victims (very soon), so
the 'x C' will become ' C' in all cases.

If ' C' is too cryptic, we could add the '(M->!)' later.

>
>
> Either way, I think this branch is ready for reintegration into trunk!!
> What do you think, Steve?
> (still in need of a complete `make check' though)

Yes, if the switch and merge tests aren't badly affected. Looking
into that now.

>
>
> Thanks again for the great improvements I found there today! I really didn't
> know what to do about the test framework, it's great how that just works
> now. The stuff I complained about before also makes sense to me, now.
>
> Oh, I'd just like to note that there was a misconception in some comments in
> update_tests.py:
>
> [[[
> - ### Current behavior (rather messy, isn't it?)
> ]]]
>
> It wasn't reaaally that messy, it just missed the cases of file additions
> into conflicted directories. ;)
>
>
> [[[
> # 1.1) local tree delete, incoming leaf edit
> ...
> - ### Current behavior carries out deletions in conflicted trees.
> - expected_state = state_after_leaf_edit.copy()
> - expected_state.remove('F/alpha', 'DF/D1/beta', 'DDF/D1/D2/gamma')
> + # The directory structure is scheduled for delete but is still
> + # found on disk, because the way directory deletes are handled by svn.
> + # No files are present because alpha, beta, gamma were locally removed
> + # and delta, epsilon, zeta weren't added due to tree-conflicts in their
> + # parent directory.
> + expected_disk = state_after_leaf_edit.copy()
> + expected_disk.remove('F/alpha', 'DF/D1/beta', 'DDF/D1/D2/gamma',
> + 'D/D1/delta', 'DD/D1/D2/epsilon',
> 'DDD/D1/D2/D3/zeta')
> ]]]
>
> The incoming action is a leaf edit, so no deletions are tried. The error was
> that files were being added into a conflicted dir. (Why was this called
> expected_state? It's called expected_disk everywhere else.)
>
> [[[
> - ### Current behavior records redundant conflicts!
> + ### Current behavior fails to show a status on files that would
> + ### have been added if there hadn't been a tree-conflict.
> ]]]
>
> The complete local tree was removed, so if you try to add a file a few
> levels inside a removed dir, all the non-existing path elements are in tree
> conflict. Makes sense to me.

But the whole tree is already tree-conflicted, so raising new tree
conflicts insided it is redundant.

>
>
> The check_tree_conflict() vs. svn_wc_conflicted_p() discussion made me
> think: In the tests, I guess we should also try to run the same operation
> that causes the conflicts again, to check that the persisting conflicts
> aren't re-raised or missed altogether. Is this accounted for yet?

No, currently we test only fresh tree conflicts. Good point.

>
>
> And to squeeze yet another topic into this mail: What's the deal with the
> run_and_verify_unquiet_status tests -- is status not intended to show
> tree-conflicts (on missing nodes) by default?

True. The final status check in every run_and_verify_X() runs with the
-q option, ignoring files and dirs that aren't under version control.

I think I'll change our DeepTreesTestCase class (on trunk) to do the
status check unquietly, because so many t.c. victims don't exist.

Cheers,
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-10-25 14:15:56 CEST

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