On Thu, Jan 22, 2009 at 11:20 AM, Stephen Butler <sbutler_at_elego.de> wrote:
> Quoting Paul Burba <ptburba_at_gmail.com>:
>> While reviewing issue #3209 I went through all the XFailing tests in
>> checkout_tests.py, update_tests.py, switch_tests.py, and
>> merge_tests.py looking for tests that fail because their expectations
>> were not adjusted to account for new tree conflict behavior.
>> Ignoring tests that fail for reasons unrelated to TCs, those that are
>> failing because they test unimplemented TC behavior, those that are
>> actively being worked on by others, and those I was able to fix, there
>> are six tests remaining that started failing with the advent of tree
>> conflict handling. These fall into four groups:
> Hi Paul,
> thanks for this report! I wrote some comments and todo-items inline
I'm often mocked for my overly-long e-mails, so thanks for wading
>> GROUP 1
>> checkout_tests.py 13 'co handles obstructing paths scheduled for add'
>> update_tests.py 34 'update handles obstructing paths scheduled for add'
>> switch_tests.py 21 'forced switch detects tree conflicts'
>> These three tests all cover behavior added in r22257:
>> Enable up/sw/co to tolerate obstructions scheduled
>> for addition without history.
>> It's no longer an error when an update, switch, or
>> checkout attempts to add a path that is already
>> scheduled for addition *without* history in the WC.
>> In the case of dirs, the directory is tolerated and
>> reported as 'E'xisting.
>> For files, if the obstruction is identical textually
>> to the addition from the repos it is reported as
>> 'E'xisting, if it differs it's treated as a conflict.
>> Property merges for both files and dirs are reported
>> normally, i.e. ' ', 'C', 'G'.
>> Obstructions scheduled for addition *with* history
>> still result in an error.
>> See these three threads for more on this change:
>> All these tests start with two identical working copies, WC1 and WC2.
>> A set of files and directories are added to the repository via WC1.
>> Then the test adds the same set of paths *without* history to WC2.
>> These local adds in WC2 fall into 3 categories:
>> A) Files textually identical to the files added to WC1.
>> Previously we expected these obstructions to be
>> tolerated, reported as 'E'xisting, and leave no trace
>> after the checkout. Now however this produces a tree
>> conflict of the 'local add, incoming add upon update'
>> B) Files which differ textually from the files added
>> to WC1.
>> Prior to TC we expected these to be conflicts and they
>> still are, but they are tree conflicts of the 'local
>> add, incoming add upon update' type instead.
>> C) Added directories that have no properties, the same
>> properties, or conflicting properties with the the
>> directories added to WC1.
>> Prior to TC we expected all of these to be tolerated and
>> reported as 'E'xisting, though their properties might mer'G'e
>> or 'C'onflict. That is still the case.
>> The desired behavior in cases A) & B) need to be resolved. Do we want
>> to support the behavior introduced in r22257 that tolerates
>> obstructing adds with history or remove it? The behavior in case C)
>> should follow the decision on A) and B), files and directories are
>> treated consistently I think.
> To see this from the user's point of view, let's refer to
> notes/tree-conflicts/resolution.txt. In section "up/sw: Add onto
> Add", there are 3 sample use cases. The local and incoming changes...
> 1. are the same patch (local change can be reverted, or merged).
> 2. are different, but share a name (local change can be renamed).
> 3. are similar, and should be merged.
> 1 and 3 are safe for reporting the existing paths and merging any
> 2 is a bit tricky to sort out, if the merge has already been
> attempted by the update.
You're quite right; assuming we return to the pre-TC behavior of
tolerating the existing file add-without-history, then we'll get a
traditional text conflict and the user will have to resort to some
non-obvious trickery to keep both the incoming file and their
completely unrelated local addition. But that is also true if '2' is
treated as a tree conflict right? So I'm not sure they are any worse
off either way. Though *maybe* it is more obvious what the problem is
if we flag a tree conflict. Though maybe not, if the files really
have nothing to do with each other, then the text conflict is quite
likely to be of this variety...
*The entire text of my locally added file*
*The entire text of the incoming added file*
...which seems pretty obvious too.
> Maybe the user should update back to the
> old BASE revision, move the files, and add them again? I'll have to
> try it myself.
Or, resolve --accept mine-full, OS copy, revert, add, commit (RORAC?):
# Oops, nu.r2 landed on my local add-without-history nu.mine,
# but these two file have nothing to do with each other, I
# want to keep both.
>svn st merge_tests-111.other
# Keep nu.mine
>svn resolve --accept mine-full merge_tests-111.other\A\C\nu
Resolved conflicted state of 'merge_tests-111.other\A\C\nu'
>svn st merge_tests-111.other
# Kludgey copy outside of Subversion.
>copy merge_tests-111.other\A\C\nu merge_tests-111.other\A\C\new_nu
1 file(s) copied.
# Now, keep the incoming nu.
>svn revert merge_tests-111.other\A\C\nu
# And lastly add and commit "my" nu.
>svn add merge_tests-111.other\A\C\new_nu
>svn ci -m "New nus" merge_tests-111.other
Transmitting file data .
Committed revision 3.
Yup, that is capital 'U' Ugly, but again, if this is treated as a tree
conflict we are left in quite a similar situation.
> Assuming that situation 2 isn't too tough for the user,
FWIW, I searched through the users mailing list, back to the release
of 1.5.0, looking for reports of use case '2', but didn't find any.
> I'd prefer to
> tolerate the local-add-without-history (i.e., not raise a tree
> conflict). To do this, I'll make the add_file() callback act like
> add_directory(), checking for the local-add before checking for a tree
>> There is also a regression in these tests. Taking checkout_tests.py
>> 13 for example, after the above cases are covered, the test does a
>> URL-to-URL copy of A/D/G to A/D/M and then does a WC-to-WC copy of
>> to A/D/M, leaving us with (I'm simplifying here, the test does some other
>> as well):
>> trunk>svn st checkout_tests-13 -u
>> * checkout_tests-13\A\D\M\pi
>> * checkout_tests-13\A\D\M\rho
>> * checkout_tests-13\A\D\M\tau
>> A + * - checkout_tests-13\A\D\M
>> * 1 checkout_tests-13\A\D
>> Status against revision: 3
>> Then when we try to update (the test does a checkout but the effect is
>> the same)
>> we get this error:
>> trunk>svn up checkout_tests-13
>> ..\..\..\subversion\libsvn_wc\adm_files.c:672: (apr_err=155000)
>> svn: Revision 3 doesn't match existing revision 1 in
> You're right, the update editor is being too picky here. Calling
> svn_wc_ensure_adm3() is overkill anyway, it includes code that creates
> the admin dir if it doesn't exist. All we want to do here is a
> read-only sanity check. I'll change it to compare only the incoming
> and existing URLs. If they don't match, there will be an error.
>> GROUP 2
>> update_tests.py 31 'forced up fails with some types of obstructions'
>> This test fails when we a (--forced) update tries to add a directory
>> when a versioned directory of the same name already exists.
>> Admittedly this is quite a contrived situation: The test adds a
>> directory A/C/I in r2, updates A/C to r1, then checks out %URL%/A/C/I
>> directly to A/C/I in the WC. This leaves us with a situation where
>> A/C thinks A/C/I is some unversioned item:
>> >svn st update_tests-31.backup\A\C -v
>> 1 1 jrandom update_tests-31.backup\A\C
>> ? update_tests-31.backup\A\C\I
>> >svn st update_tests-31.backup\A\C\I -v
>> 2 2 jrandom update_tests-31.backup\A\C\I
>> Prior to TC we expected this to fail with an error:
>> svn: Failed to add directory 'update_tests-31.backup\A\C\I':
>> a versioned directory of the same name already exists
>> This is now a tree conflict:
>> >svn up --force update_tests-31.backup
>> C update_tests-31.backup\A\C\I
>> At revision 2.
>> Summary of conflicts:
>> Tree conflicts: 1
>> >svn st update_tests-31.backup
>> ? C update_tests-31.backup\A\C\I
>> > local add, incoming add upon update
>> It strikes me that *both* of these behaviors are probably wrong.
>> Shouldn't the update simply tolerate A/C/I's presence and simply bring
>> A/C up to r2? Though if A/C/I pointed to a different location then
>> there should be an error, but what?
> I agree, it would be nice to subsume the existing A/C/I working copy
> when checking out a new working copy in the parent dir, and simply to
> bring A/C/I up to date, raising new conflicts within it as necessary.
> If the URLs don't match, I'll raise an error, because it's the same
> situation as an existing added-without-history item (see end of "GROUP
> 1", above).
>> FWIW this doesn't seem a very common use case. The old behavior was
>> added by me back with the r20945 'takeover' functionality, and IIRC it
>> was simply because this was a possibility and the code had to do
>> *something* in this case.
>> GROUP 3
>> update_tests.py 50 'tree conflicts on update 2.3'
>> This tests that updates of tree conflicted paths report the TCs as
>> skipped, but what exactly does 2.3 refer to?
> To no other document. The names 2.1 and 2.2 are arbitrary, too.
> Descriptive docstrings might help.
> tree conflicts 2.1 leaf edit, tree del on update
> tree conflicts 2.2 leaf del, tree del on update
> tree conflicts 2.3 skip TCs on 2nd update
Ok, just wanted to be sure I wasn't missing some secret TC docs
printed on the back of the Constitution :-)
>> has use case 2, but there are no subsections. This appears to be an
>> test for issue #3329,
>> http://subversion.tigris.org/issues/show_bug.cgi?id=3329#desc2, and as
>> I noted there, many of the cases this test covers now work and report
>> a) Directory tree conflict victim is updated
>> b) File tree conflict victim is updated
>> c) Directory within a tree conflict victim is updated
>> d) Fle within a tree conflict victim is updated
>> (file doesn't exist in the repos)
>> What doesn't work is:
>> e) A file within a tree conflict victim is updated
>> (file doesn't exist in the repos but shows as
>> locally added)
>> f) Directory update target is not tree conflicted
>> but has tree conflicted file child.
>> g) Directory update target is not tree conflicted
>> but has tree conflicted dir child.
>> In all three of these cases the update prints no skip notifications,
>> the 'At revision N.' is the only output. The status of the WC is the
>> same in each case, so no harm is done outside of the missing skip
>> So, is this simply a test for issue #3329 or is there anything else to
> Yes, it's simply a regression test for #3329 "Update throws error when
> skipping some tree conflicts".
Is anyone working on e), f), and g)? I'm glad to take a look if not.
>> GROUP 4
>> merge_tests.py 20 'merge into missing must not break working copy'
>> In r31326 (on the tree-conflicts branch) this test was changed to
>> expect a tree conflict when merging into a directory that has a
>> missing subtree (to be clear, this is missing in the '!' removed by a
>> non-svn command sense of missing). The current (and pre-TC?) behavior
>> was to report the missing subtrees as skipped.
>> Why is a tree conflict expected here? We report the files as skipped
>> during the merge and they show as missing in status. You can't even
>> commit the change without updating first to get the missing items
>> back. Yes, at that point you could commit and most likely you won't
>> have what you want in the repos, but you must willfully ignore a lot
>> of warnings that something is not quite right.
>> This case mentioned in the 'SCENARIO PLAYGROUND' in
>> but there is no conclusion:
>> 'svn merge' pulls file modification atop missing file
>> NOW: "Skipped missing target" message.
>> There is also some mention of this in
>> * Pre-existing obstructions (status "~") or missing items ("!") should be
>> detected first, before attempting to apply the change, and abort the
>> What is the latest thinking on this scenario?
I know, people lose energy by the end of my mails, did you have any
thoughts on this last item?
Received on 2009-01-23 01:11:40 CET