You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Paul Burba <pt...@gmail.com> on 2009/02/04 14:36:01 UTC
Re: Need to resolve expected behavior of XFailing tests caused by
tree conflicts (Issue #3209)
On Thu, Jan 22, 2009 at 11:20 AM, Stephen Butler <sb...@elego.de> wrote:
> Quoting Paul Burba <pt...@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
> below.
>
> Regards,
> Steve
>
>>
>> =======
>> 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:
>>
>> http://svn.haxx.se/dev/archive-2006-08/0336.shtml
>> http://svn.haxx.se/dev/archive-2006-09/0362.shtml
>> http://svn.haxx.se/dev/archive-2006-10/0708.shtml
>>
>> 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'
>> type.
>>
>> 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
> differences.
>
> 2 is a bit tricky to sort out, if the merge has already been
> attempted by the update. 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.
>
> Assuming that situation 2 isn't too tough for the user, 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
> conflict.
>
>>
>> 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
>> A/D/H
>> to A/D/M, leaving us with (I'm simplifying here, the test does some other
>> stuff
>> 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
>> 'checkout_tests-13/A/D/M'
>
> 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
>
>>
>> http://svn.collab.net/repos/svn/trunk/notes/tree-conflicts/use-cases.txt
>> 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
>> skips:
>>
>> 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
>> notifications.
>>
>> So, is this simply a test for issue #3329 or is there anything else to
>> consider?
>
> Yes, it's simply a regression test for #3329 "Update throws error when
> skipping some tree conflicts".
>
>>
>>
>> =======
>> 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
>>
>> http://svn.collab.net/repos/svn/trunk/notes/tree-conflicts/scratch-pad.txt,
>> but there is no conclusion:
>>
>> 'svn merge' pulls file modification atop missing file
>>
>> NOW: "Skipped missing target" message.
>>
>> NEW:
>>
>> There is also some mention of this in
>>
>> http://svn.collab.net/repos/svn/trunk/notes/tree-conflicts/design-overview.txt
>>
>> * Pre-existing obstructions (status "~") or missing items ("!") should be
>> detected first, before attempting to apply the change, and abort the
>> operation.
>>
>> What is the latest thinking on this scenario?
>>
>> ~~~~~
Hi Steve,
Any progress on any of the aforementioned todos? If not, will you be
able to work on any of them? We still have issue #3209 as a blocker
for 1.6 so we need to figure out what, if anything really needs to be
done before 1.6.
Paul
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1102493
Re: Need to resolve expected behavior of XFailing tests caused by
tree conflicts (Issue #3209)
Posted by Stephen Butler <sb...@elego.de>.
Quoting Mark Phippard <ma...@gmail.com>:
> I am going to top-reply because it is a "meta comment".
>
> This came up as we were working on resolutions for various tree
> conflicts. It just did not feel to us like it was explainable why
> this was not a tree conflict. The situation seems similar to lots of
> the other scenarios where TC's are raised. So ideally, for
> consistency with ourselves, yes, I think this should be a tree
> conflict.
>
> That said, now that we have a tree conflicts mechanism in place, I
> have no problem with shipping a release that has scenarios where we
> are not currently raising tree conflicts, but would like to. IOW, we
> can still improve out detection mechanisms post-1.6. The main thing
> of course, is that in all scenarios we must leavesthe WC in a usable
> state. In this case, I do think the current behavior does do that.
+1
>
> So I would be OK with this coming post 1.6 if it requires a lot of
> work and verification.
I think it will require a fair bit of work, because the locally-
added tree shouldn't be overwritten without checking for text and
prop conflicts. (Details inline below)
>
> Mark
>
> PS - I agree that "ideally" if the file contents are identical, we
> would not raise a conflict and just convert the local Add into a local
> "Normal" state file. I do not see any reason to raise a conflict in
> this scenario if it is possible.
>
>
>
> On Fri, Feb 13, 2009 at 10:49 AM, Paul Burba <pt...@gmail.com> wrote:
>> I think we need to change this. Last night in IRC Mark raised this
>> very issue, specifically the case where a file add is obstructed by a
>> file added without history where the files have text conflicts. He
>> expected a tree conflict in this case. Prior to merging the issue
>> 3334 branch to trunk we did get a tree conflict(1):
>>
>> PRE3334.DBG>svn st -v update_tests-34.backup\A\D
>> 1 1 jrandom update_tests-34.backup\A\D
>> 1 1 jrandom update_tests-34.backup\A\D\gamma
>> 1 1 jrandom update_tests-34.backup\A\D\G
>> 1 1 jrandom update_tests-34.backup\A\D\G\pi
>> 1 1 jrandom update_tests-34.backup\A\D\G\rho
>> 1 1 jrandom update_tests-34.backup\A\D\G\tau
>> 1 1 jrandom update_tests-34.backup\A\D\H
>> 1 1 jrandom update_tests-34.backup\A\D\H\chi
>> 1 1 jrandom update_tests-34.backup\A\D\H\omega
>> 1 1 jrandom update_tests-34.backup\A\D\H\psi
>> A 0 ? ? update_tests-34.backup\A\D\epsilon
>>
>> PRE3334.DBG>svn log -r2 -v -q update_tests-34.backup
>> ------------------------------------------------------------------------
>> r2 | jrandom | 2009-02-13 09:47:36 -0500 (Fri, 13 Feb 2009)
>> Changed paths:
>> A /A/D/epsilon
>> ------------------------------------------------------------------------
>>
>> PRE3334.DBG>svn up update_tests-34.backup\A\D
>> C update_tests-34.backup\A\D\epsilon
>> At revision 2.
>> Summary of conflicts:
>> Tree conflicts: 1
>>
>> ===CASE A===
>>
>> With trunk@35842, as you point out, there is no tree conflict nor any error:
>>
>> trunk.dbg>svn up update_tests-34.backup\A\D
>> Conflict discovered in 'update_tests-34.backup/A/D/epsilon'.
>> Select: (p) postpone, (df) diff-full, (e) edit,
>> (mc) mine-conflict, (tc) theirs-conflict,
>> (s) show all options: p
>> C update_tests-34.backup\A\D\epsilon
>> Updated to revision 2.
>> Summary of conflicts:
>> Text conflicts: 1
>>
>> trunk.dbg>svn st -v update_tests-34.backup\A\D
>> 2 2 jrandom update_tests-34.backup\A\D
>> ?
>> update_tests-34.backup\A\D\epsilon.mine
>> ?
>> update_tests-34.backup\A\D\epsilon.r0
>> ?
>> update_tests-34.backup\A\D\epsilon.r2
>> 2 1 jrandom update_tests-34.backup\A\D\gamma
>> 2 1 jrandom update_tests-34.backup\A\D\G
>> 2 1 jrandom update_tests-34.backup\A\D\G\pi
>> 2 1 jrandom update_tests-34.backup\A\D\G\rho
>> 2 1 jrandom update_tests-34.backup\A\D\G\tau
>> 2 1 jrandom update_tests-34.backup\A\D\H
>> 2 1 jrandom update_tests-34.backup\A\D\H\chi
>> 2 1 jrandom update_tests-34.backup\A\D\H\omega
>> 2 1 jrandom update_tests-34.backup\A\D\H\psi
>> C 2 2 jrandom update_tests-34.backup\A\D\epsilon
>>
>> ===CASE B===
>>
>> If there are no text differences between the two files then we report
>> the added obstruction as 'E'xisting:
>>
>> trunk.dbg>svn up update_tests-34.backup\A\D
>> E update_tests-34.backup\A\D\epsilon
>> Updated to revision 2.
>>
>> trunk.dbg>svn st -v update_tests-34.backup\A\D
>> 2 2 jrandom update_tests-34.backup\A\D
>> 2 1 jrandom update_tests-34.backup\A\D\gamma
>> 2 1 jrandom update_tests-34.backup\A\D\G
>> 2 1 jrandom update_tests-34.backup\A\D\G\pi
>> 2 1 jrandom update_tests-34.backup\A\D\G\rho
>> 2 1 jrandom update_tests-34.backup\A\D\G\tau
>> 2 1 jrandom update_tests-34.backup\A\D\H
>> 2 1 jrandom update_tests-34.backup\A\D\H\chi
>> 2 1 jrandom update_tests-34.backup\A\D\H\omega
>> 2 1 jrandom update_tests-34.backup\A\D\H\psi
>> 2 2 jrandom update_tests-34.backup\A\D\epsilon
>>
>> I agree with Mark that CASE A should be a tree conflict, and the
>> incoming file's text should simply be replaced by the locally added
>> file's text rather than being merged. This essentially maintains the
>> old obstruction tolerance functionality added in r22257 but adds a
>> tree conflict to attract the user's attention that something might be
>> amiss.
Actually, an update's incoming text and prop changes are merged
into a locally-added-without-history tree, possibly raising text
and prop conflicts.
Currently, when we raise a tree conflict, we do one of the
following to the local content:
(local del, incoming edit):
Replace the local content without further conflict-checking,
because the user already scheduled it for deletion.
(local add-w-hist, incoming add):
Skip the local victim tree entirely.
(Note: Other types of tree conflicts have no incoming adds or
edits, so content merging isn't an issue.)
Case A above (local add-wo-hist, incoming add):
I assume we want to keep the content merging. If so...we'll
have to overhaul the notification code to integrate tree
conflict notification into the normal close_{directory,file}
notification. Funny thing is, last fall we had it that way,
until we decided in a moment of temporary insanity to skip all
TC victims. :-) Rearranging the notification code is about
a thousand-line diff, IIRC, which is perhaps more than we
want to tackle before 1.6.0.
>>
>> It's a bit trickier with case B, the existing behavior of not raising
>> a tree conflict is quite nice, but at first glance this does not
>> appear very straightforward...looking a bit deeper right now.
Since we'd print the tree conflict notification when closing the
victim, we could hold off actually writing the tree conflict
description until the close, and write it only if there are text
or property conflicts in the tree. Or, to be strict, if the local
and incoming trees didn't match exactly.
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
Re: Need to resolve expected behavior of XFailing tests caused by
tree conflicts (Issue #3209)
Posted by Mark Phippard <ma...@gmail.com>.
I am going to top-reply because it is a "meta comment".
This came up as we were working on resolutions for various tree
conflicts. It just did not feel to us like it was explainable why
this was not a tree conflict. The situation seems similar to lots of
the other scenarios where TC's are raised. So ideally, for
consistency with ourselves, yes, I think this should be a tree
conflict.
That said, now that we have a tree conflicts mechanism in place, I
have no problem with shipping a release that has scenarios where we
are not currently raising tree conflicts, but would like to. IOW, we
can still improve out detection mechanisms post-1.6. The main thing
of course, is that in all scenarios we must leavesthe WC in a usable
state. In this case, I do think the current behavior does do that.
So I would be OK with this coming post 1.6 if it requires a lot of
work and verification.
Mark
PS - I agree that "ideally" if the file contents are identical, we
would not raise a conflict and just convert the local Add into a local
"Normal" state file. I do not see any reason to raise a conflict in
this scenario if it is possible.
On Fri, Feb 13, 2009 at 10:49 AM, Paul Burba <pt...@gmail.com> wrote:
> I think we need to change this. Last night in IRC Mark raised this
> very issue, specifically the case where a file add is obstructed by a
> file added without history where the files have text conflicts. He
> expected a tree conflict in this case. Prior to merging the issue
> 3334 branch to trunk we did get a tree conflict(1):
>
> PRE3334.DBG>svn st -v update_tests-34.backup\A\D
> 1 1 jrandom update_tests-34.backup\A\D
> 1 1 jrandom update_tests-34.backup\A\D\gamma
> 1 1 jrandom update_tests-34.backup\A\D\G
> 1 1 jrandom update_tests-34.backup\A\D\G\pi
> 1 1 jrandom update_tests-34.backup\A\D\G\rho
> 1 1 jrandom update_tests-34.backup\A\D\G\tau
> 1 1 jrandom update_tests-34.backup\A\D\H
> 1 1 jrandom update_tests-34.backup\A\D\H\chi
> 1 1 jrandom update_tests-34.backup\A\D\H\omega
> 1 1 jrandom update_tests-34.backup\A\D\H\psi
> A 0 ? ? update_tests-34.backup\A\D\epsilon
>
> PRE3334.DBG>svn log -r2 -v -q update_tests-34.backup
> ------------------------------------------------------------------------
> r2 | jrandom | 2009-02-13 09:47:36 -0500 (Fri, 13 Feb 2009)
> Changed paths:
> A /A/D/epsilon
> ------------------------------------------------------------------------
>
> PRE3334.DBG>svn up update_tests-34.backup\A\D
> C update_tests-34.backup\A\D\epsilon
> At revision 2.
> Summary of conflicts:
> Tree conflicts: 1
>
> ===CASE A===
>
> With trunk@35842, as you point out, there is no tree conflict nor any error:
>
> trunk.dbg>svn up update_tests-34.backup\A\D
> Conflict discovered in 'update_tests-34.backup/A/D/epsilon'.
> Select: (p) postpone, (df) diff-full, (e) edit,
> (mc) mine-conflict, (tc) theirs-conflict,
> (s) show all options: p
> C update_tests-34.backup\A\D\epsilon
> Updated to revision 2.
> Summary of conflicts:
> Text conflicts: 1
>
> trunk.dbg>svn st -v update_tests-34.backup\A\D
> 2 2 jrandom update_tests-34.backup\A\D
> ? update_tests-34.backup\A\D\epsilon.mine
> ? update_tests-34.backup\A\D\epsilon.r0
> ? update_tests-34.backup\A\D\epsilon.r2
> 2 1 jrandom update_tests-34.backup\A\D\gamma
> 2 1 jrandom update_tests-34.backup\A\D\G
> 2 1 jrandom update_tests-34.backup\A\D\G\pi
> 2 1 jrandom update_tests-34.backup\A\D\G\rho
> 2 1 jrandom update_tests-34.backup\A\D\G\tau
> 2 1 jrandom update_tests-34.backup\A\D\H
> 2 1 jrandom update_tests-34.backup\A\D\H\chi
> 2 1 jrandom update_tests-34.backup\A\D\H\omega
> 2 1 jrandom update_tests-34.backup\A\D\H\psi
> C 2 2 jrandom update_tests-34.backup\A\D\epsilon
>
> ===CASE B===
>
> If there are no text differences between the two files then we report
> the added obstruction as 'E'xisting:
>
> trunk.dbg>svn up update_tests-34.backup\A\D
> E update_tests-34.backup\A\D\epsilon
> Updated to revision 2.
>
> trunk.dbg>svn st -v update_tests-34.backup\A\D
> 2 2 jrandom update_tests-34.backup\A\D
> 2 1 jrandom update_tests-34.backup\A\D\gamma
> 2 1 jrandom update_tests-34.backup\A\D\G
> 2 1 jrandom update_tests-34.backup\A\D\G\pi
> 2 1 jrandom update_tests-34.backup\A\D\G\rho
> 2 1 jrandom update_tests-34.backup\A\D\G\tau
> 2 1 jrandom update_tests-34.backup\A\D\H
> 2 1 jrandom update_tests-34.backup\A\D\H\chi
> 2 1 jrandom update_tests-34.backup\A\D\H\omega
> 2 1 jrandom update_tests-34.backup\A\D\H\psi
> 2 2 jrandom update_tests-34.backup\A\D\epsilon
>
> I agree with Mark that CASE A should be a tree conflict, and the
> incoming file's text should simply be replaced by the locally added
> file's text rather than being merged. This essentially maintains the
> old obstruction tolerance functionality added in r22257 but adds a
> tree conflict to attract the user's attention that something might be
> amiss.
>
> It's a bit trickier with case B, the existing behavior of not raising
> a tree conflict is quite nice, but at first glance this does not
> appear very straightforward...looking a bit deeper right now.
>
> Does this sound like a reasonable approach?
>
> Paul
>
> ~~~~~
>
> (1) Before the 3334 branch was merged back to trunk things didn't work
> quite so well:
>
> Turns out epsilon is still scheduled for addition, and a revert or
> resolve shows it as unversioned!
>
> PRE3334.DBG>svn st -v update_tests-34.backup\A\D\epsilon
> A C 0 ? ? update_tests-34.backup\A\D\epsilon
> > local add, incoming add upon update
>
> PRE3334.DBG>svn revert -R update_tests-34.backup\A\D\epsilon
> Reverted 'update_tests-34.backup\A\D\epsilon'
>
> PRE3334.DBG>svn st -v update_tests-34.backup\A\D\epsilon
> ? update_tests-34.backup\A\D\epsilon
>
>
> PRE3334.DBG>
>
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1151646
Re: Need to resolve expected behavior of XFailing tests caused by
tree conflicts (Issue #3209)
Posted by Paul Burba <pt...@gmail.com>.
On Fri, Feb 13, 2009 at 9:22 AM, Stephen Butler <sb...@elego.de> wrote:
> Quoting Paul Burba <pt...@gmail.com>:
>
>> On Thu, Feb 5, 2009 at 1:20 PM, Stephen Butler <sb...@elego.de> wrote:
>>>
>>> Quoting Mark Phippard <ma...@gmail.com>:
>
>>> The XFAIL parts of that test are in fairly odd situations where the
>>> error message "A/B/C does not exist" takes precedence,
>>
>> Hi Steve,
>>
>> I don't follow the "not exist" part, is that still the case? All I
>> see failing in update_tests.py 50 is that the second updates don't
>> report that the tree conflicted paths under the update target are
>> skipped...
>
> The "not exist" part is no longer relevant. Since r34456, we don't
> bother testing update deep inside local trees that no longer exist
> in the repo. We may revisit this when wc-ng comes along.
>
>>
>>> or where the
>>> update output is silent. It'd be nice to say "A is tree-conflicted"
>>> in all of those situations, but it isn't a blocker.
>>
>> ...and if that all the problem is, then I agree this is not serious
>> enough to be a blocker.
>
> OK.
>
>>
>>> The skipping behavior is changing. We may have to revise this test
>>> soon anyway.
>
> Actually we didn't change the skipping behavior for already-discovered
> tree conflicts, so no effort is needed here.
>
>>>
>>>>
>>>> You are working on groups 1 & 2. Do you think you'll be able to bring
>>>> these to completion? Any ideas on how much work is left/when it will
>>>> be completed?
>>>
>>> I'll commit the first part of the fix in a bit, if the tests are OK.
>>> Just making the add_file update code consistent with add_directory.
>>> This incidentally fixes some of the issue 3209 problems Paul
>>> mentioned.
>>>
>>> There's a day or two of work to do, so that update/switch will tolerate
>>> unversioned items and items added without history.
>>>
>>> But first I think we should eliminate skipping of newly discovered tree
>>> conflicts. Issue 3334 is more important, and involves the same code,
>>> so I'd like to get it out of the way first.
>>
>> Issue #3334 is now out of the way, so #3209 is the last thing (other
>> than the Ruby bindings) standing between us and branching 1.6. If
>> there is any portion of this I can take on please let me know.
>
> I committed r35840 and r35842 today, which clean up the remaining
> XFAILs noted in #3209.
>
> I think we're out of the woods on this one now. I'm updating issue
> 3209 now. If there are no objections, I'll close it.
>
> The only aspect I'm not 100% sure of is the following:
>
> For "local add *without* history, incoming add via up/sw/co", we
> neither raise a tree conflict nor stop with an error. Any
> objections?
I think we need to change this. Last night in IRC Mark raised this
very issue, specifically the case where a file add is obstructed by a
file added without history where the files have text conflicts. He
expected a tree conflict in this case. Prior to merging the issue
3334 branch to trunk we did get a tree conflict(1):
PRE3334.DBG>svn st -v update_tests-34.backup\A\D
1 1 jrandom update_tests-34.backup\A\D
1 1 jrandom update_tests-34.backup\A\D\gamma
1 1 jrandom update_tests-34.backup\A\D\G
1 1 jrandom update_tests-34.backup\A\D\G\pi
1 1 jrandom update_tests-34.backup\A\D\G\rho
1 1 jrandom update_tests-34.backup\A\D\G\tau
1 1 jrandom update_tests-34.backup\A\D\H
1 1 jrandom update_tests-34.backup\A\D\H\chi
1 1 jrandom update_tests-34.backup\A\D\H\omega
1 1 jrandom update_tests-34.backup\A\D\H\psi
A 0 ? ? update_tests-34.backup\A\D\epsilon
PRE3334.DBG>svn log -r2 -v -q update_tests-34.backup
------------------------------------------------------------------------
r2 | jrandom | 2009-02-13 09:47:36 -0500 (Fri, 13 Feb 2009)
Changed paths:
A /A/D/epsilon
------------------------------------------------------------------------
PRE3334.DBG>svn up update_tests-34.backup\A\D
C update_tests-34.backup\A\D\epsilon
At revision 2.
Summary of conflicts:
Tree conflicts: 1
===CASE A===
With trunk@35842, as you point out, there is no tree conflict nor any error:
trunk.dbg>svn up update_tests-34.backup\A\D
Conflict discovered in 'update_tests-34.backup/A/D/epsilon'.
Select: (p) postpone, (df) diff-full, (e) edit,
(mc) mine-conflict, (tc) theirs-conflict,
(s) show all options: p
C update_tests-34.backup\A\D\epsilon
Updated to revision 2.
Summary of conflicts:
Text conflicts: 1
trunk.dbg>svn st -v update_tests-34.backup\A\D
2 2 jrandom update_tests-34.backup\A\D
? update_tests-34.backup\A\D\epsilon.mine
? update_tests-34.backup\A\D\epsilon.r0
? update_tests-34.backup\A\D\epsilon.r2
2 1 jrandom update_tests-34.backup\A\D\gamma
2 1 jrandom update_tests-34.backup\A\D\G
2 1 jrandom update_tests-34.backup\A\D\G\pi
2 1 jrandom update_tests-34.backup\A\D\G\rho
2 1 jrandom update_tests-34.backup\A\D\G\tau
2 1 jrandom update_tests-34.backup\A\D\H
2 1 jrandom update_tests-34.backup\A\D\H\chi
2 1 jrandom update_tests-34.backup\A\D\H\omega
2 1 jrandom update_tests-34.backup\A\D\H\psi
C 2 2 jrandom update_tests-34.backup\A\D\epsilon
===CASE B===
If there are no text differences between the two files then we report
the added obstruction as 'E'xisting:
trunk.dbg>svn up update_tests-34.backup\A\D
E update_tests-34.backup\A\D\epsilon
Updated to revision 2.
trunk.dbg>svn st -v update_tests-34.backup\A\D
2 2 jrandom update_tests-34.backup\A\D
2 1 jrandom update_tests-34.backup\A\D\gamma
2 1 jrandom update_tests-34.backup\A\D\G
2 1 jrandom update_tests-34.backup\A\D\G\pi
2 1 jrandom update_tests-34.backup\A\D\G\rho
2 1 jrandom update_tests-34.backup\A\D\G\tau
2 1 jrandom update_tests-34.backup\A\D\H
2 1 jrandom update_tests-34.backup\A\D\H\chi
2 1 jrandom update_tests-34.backup\A\D\H\omega
2 1 jrandom update_tests-34.backup\A\D\H\psi
2 2 jrandom update_tests-34.backup\A\D\epsilon
I agree with Mark that CASE A should be a tree conflict, and the
incoming file's text should simply be replaced by the locally added
file's text rather than being merged. This essentially maintains the
old obstruction tolerance functionality added in r22257 but adds a
tree conflict to attract the user's attention that something might be
amiss.
It's a bit trickier with case B, the existing behavior of not raising
a tree conflict is quite nice, but at first glance this does not
appear very straightforward...looking a bit deeper right now.
Does this sound like a reasonable approach?
Paul
~~~~~
(1) Before the 3334 branch was merged back to trunk things didn't work
quite so well:
Turns out epsilon is still scheduled for addition, and a revert or
resolve shows it as unversioned!
PRE3334.DBG>svn st -v update_tests-34.backup\A\D\epsilon
A C 0 ? ? update_tests-34.backup\A\D\epsilon
> local add, incoming add upon update
PRE3334.DBG>svn revert -R update_tests-34.backup\A\D\epsilon
Reverted 'update_tests-34.backup\A\D\epsilon'
PRE3334.DBG>svn st -v update_tests-34.backup\A\D\epsilon
? update_tests-34.backup\A\D\epsilon
PRE3334.DBG>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1151565
Re: Need to resolve expected behavior of XFailing tests caused by
tree conflicts (Issue #3209)
Posted by Stephen Butler <sb...@elego.de>.
Quoting Paul Burba <pt...@gmail.com>:
> On Thu, Feb 5, 2009 at 1:20 PM, Stephen Butler <sb...@elego.de> wrote:
>> Quoting Mark Phippard <ma...@gmail.com>:
>> The XFAIL parts of that test are in fairly odd situations where the
>> error message "A/B/C does not exist" takes precedence,
>
> Hi Steve,
>
> I don't follow the "not exist" part, is that still the case? All I
> see failing in update_tests.py 50 is that the second updates don't
> report that the tree conflicted paths under the update target are
> skipped...
The "not exist" part is no longer relevant. Since r34456, we don't
bother testing update deep inside local trees that no longer exist
in the repo. We may revisit this when wc-ng comes along.
>
>> or where the
>> update output is silent. It'd be nice to say "A is tree-conflicted"
>> in all of those situations, but it isn't a blocker.
>
> ...and if that all the problem is, then I agree this is not serious
> enough to be a blocker.
OK.
>
>> The skipping behavior is changing. We may have to revise this test
>> soon anyway.
Actually we didn't change the skipping behavior for already-discovered
tree conflicts, so no effort is needed here.
>>
>>>
>>> You are working on groups 1 & 2. Do you think you'll be able to bring
>>> these to completion? Any ideas on how much work is left/when it will
>>> be completed?
>>
>> I'll commit the first part of the fix in a bit, if the tests are OK.
>> Just making the add_file update code consistent with add_directory.
>> This incidentally fixes some of the issue 3209 problems Paul
>> mentioned.
>>
>> There's a day or two of work to do, so that update/switch will tolerate
>> unversioned items and items added without history.
>>
>> But first I think we should eliminate skipping of newly discovered tree
>> conflicts. Issue 3334 is more important, and involves the same code,
>> so I'd like to get it out of the way first.
>
> Issue #3334 is now out of the way, so #3209 is the last thing (other
> than the Ruby bindings) standing between us and branching 1.6. If
> there is any portion of this I can take on please let me know.
I committed r35840 and r35842 today, which clean up the remaining
XFAILs noted in #3209.
I think we're out of the woods on this one now. I'm updating issue
3209 now. If there are no objections, I'll close it.
The only aspect I'm not 100% sure of is the following:
For "local add *without* history, incoming add via up/sw/co", we
neither raise a tree conflict nor stop with an error. Any
objections?
>
>>> Group 4 ... what is your take on Paul's comments? Is this just an
>>> area where there could be differences of opinion but it is working as
>>> intended. Or is something broken and this is an item where more work
>>> needs to be done?
>>
>> On looking more closely at merge_tests.py 20, I think the immediate
>> issue Paul raised is a design question: should a missing item be
>> flagged as a tree conflict by merge, or just notify the user that
>> we skipped it? I tend toward the former.
>
> I'm fine with the latter for 1.6 and seeing if users have any problems
> with it. Why? As I said previously in this thread:
>
> "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."
>
> I don't see holding up 1.6 on this particular issue.
>
> Paul
I agree. The commit is already blocked by the missing items, so adding
a tree conflict wouldn't gain much.
>
>> In the source code, a
>> comment suggests we simply update the missing items automatically,
>> then continue the merge.
>>
>> But anyway, I see in libsvn_client/merge.c that our tree conflict
>> detection code for handling missing and obstructing items is
>> inconsistent in handling dirs and files.
>>
>> merge_dir_opened():
>> unversioned or is-file or missing or unknown or deleted:
>> -> "local delete, incoming edit upon merge"
>>
>> merge_file_opened():
>> unversioned or is-dir or missing or unknown:
>> -> "local missing, incoming edit upon merge"
>>
>> Looks like we need to clean up that code! After we sort out
>> issues 3334 and 3209 for update/switch, I guess the next step
>> is to sketch a table of how we handle each of these cases, to
>> make sure the high-level design is implemented like we planned
>> in /notes/tree-conflicts/*.txt.
>>
>> Steve
I agree that the merge issues are not 1.6 blockers.
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
Re: Need to resolve expected behavior of XFailing tests caused by
tree conflicts (Issue #3209)
Posted by Paul Burba <pt...@gmail.com>.
On Thu, Feb 5, 2009 at 1:20 PM, Stephen Butler <sb...@elego.de> wrote:
> Quoting Mark Phippard <ma...@gmail.com>:
>
>> Stephen,
>>
>> To clarify ...
>>
>> Group 3 is "done". Does that mean the test is no longer XFAIL? It
>> seemed like you just changed the description of the test from the
>> comments. But perhaps that was all Paul was pointing out needed to be
>> done. I guess my assumption with this issue is that when we are
>> "done" the tests will no longer be XFAIL.
>
> I just meant I had improved the cryptic output of the tests (in the
> docstrings).
>
> The XFAIL parts of that test are in fairly odd situations where the
> error message "A/B/C does not exist" takes precedence,
Hi Steve,
I don't follow the "not exist" part, is that still the case? All I
see failing in update_tests.py 50 is that the second updates don't
report that the tree conflicted paths under the update target are
skipped...
> or where the
> update output is silent. It'd be nice to say "A is tree-conflicted"
> in all of those situations, but it isn't a blocker.
...and if that all the problem is, then I agree this is not serious
enough to be a blocker.
> The skipping behavior is changing. We may have to revise this test
> soon anyway.
>
>>
>> You are working on groups 1 & 2. Do you think you'll be able to bring
>> these to completion? Any ideas on how much work is left/when it will
>> be completed?
>
> I'll commit the first part of the fix in a bit, if the tests are OK.
> Just making the add_file update code consistent with add_directory.
> This incidentally fixes some of the issue 3209 problems Paul
> mentioned.
>
> There's a day or two of work to do, so that update/switch will tolerate
> unversioned items and items added without history.
>
> But first I think we should eliminate skipping of newly discovered tree
> conflicts. Issue 3334 is more important, and involves the same code,
> so I'd like to get it out of the way first.
Issue #3334 is now out of the way, so #3209 is the last thing (other
than the Ruby bindings) standing between us and branching 1.6. If
there is any portion of this I can take on please let me know.
>> Group 4 ... what is your take on Paul's comments? Is this just an
>> area where there could be differences of opinion but it is working as
>> intended. Or is something broken and this is an item where more work
>> needs to be done?
>
> On looking more closely at merge_tests.py 20, I think the immediate
> issue Paul raised is a design question: should a missing item be
> flagged as a tree conflict by merge, or just notify the user that
> we skipped it? I tend toward the former.
I'm fine with the latter for 1.6 and seeing if users have any problems
with it. Why? As I said previously in this thread:
"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."
I don't see holding up 1.6 on this particular issue.
Paul
> In the source code, a
> comment suggests we simply update the missing items automatically,
> then continue the merge.
>
> But anyway, I see in libsvn_client/merge.c that our tree conflict
> detection code for handling missing and obstructing items is
> inconsistent in handling dirs and files.
>
> merge_dir_opened():
> unversioned or is-file or missing or unknown or deleted:
> -> "local delete, incoming edit upon merge"
>
> merge_file_opened():
> unversioned or is-dir or missing or unknown:
> -> "local missing, incoming edit upon merge"
>
> Looks like we need to clean up that code! After we sort out
> issues 3334 and 3209 for update/switch, I guess the next step
> is to sketch a table of how we handle each of these cases, to
> make sure the high-level design is implemented like we planned
> in /notes/tree-conflicts/*.txt.
>
> Steve
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1146703
Re: Need to resolve expected behavior of XFailing tests caused by
tree conflicts (Issue #3209)
Posted by Stephen Butler <sb...@elego.de>.
Quoting Mark Phippard <ma...@gmail.com>:
> Stephen,
>
> To clarify ...
>
> Group 3 is "done". Does that mean the test is no longer XFAIL? It
> seemed like you just changed the description of the test from the
> comments. But perhaps that was all Paul was pointing out needed to be
> done. I guess my assumption with this issue is that when we are
> "done" the tests will no longer be XFAIL.
I just meant I had improved the cryptic output of the tests (in the
docstrings).
The XFAIL parts of that test are in fairly odd situations where the
error message "A/B/C does not exist" takes precedence, or where the
update output is silent. It'd be nice to say "A is tree-conflicted"
in all of those situations, but it isn't a blocker.
The skipping behavior is changing. We may have to revise this test
soon anyway.
>
> You are working on groups 1 & 2. Do you think you'll be able to bring
> these to completion? Any ideas on how much work is left/when it will
> be completed?
I'll commit the first part of the fix in a bit, if the tests are OK.
Just making the add_file update code consistent with add_directory.
This incidentally fixes some of the issue 3209 problems Paul
mentioned.
There's a day or two of work to do, so that update/switch will tolerate
unversioned items and items added without history.
But first I think we should eliminate skipping of newly discovered tree
conflicts. Issue 3334 is more important, and involves the same code,
so I'd like to get it out of the way first.
>
> Group 4 ... what is your take on Paul's comments? Is this just an
> area where there could be differences of opinion but it is working as
> intended. Or is something broken and this is an item where more work
> needs to be done?
On looking more closely at merge_tests.py 20, I think the immediate
issue Paul raised is a design question: should a missing item be
flagged as a tree conflict by merge, or just notify the user that
we skipped it? I tend toward the former. In the source code, a
comment suggests we simply update the missing items automatically,
then continue the merge.
But anyway, I see in libsvn_client/merge.c that our tree conflict
detection code for handling missing and obstructing items is
inconsistent in handling dirs and files.
merge_dir_opened():
unversioned or is-file or missing or unknown or deleted:
-> "local delete, incoming edit upon merge"
merge_file_opened():
unversioned or is-dir or missing or unknown:
-> "local missing, incoming edit upon merge"
Looks like we need to clean up that code! After we sort out
issues 3334 and 3209 for update/switch, I guess the next step
is to sketch a table of how we handle each of these cases, to
make sure the high-level design is implemented like we planned
in /notes/tree-conflicts/*.txt.
Steve
>
> Thanks
>
> Mark
>
>
>
> On Wed, Feb 4, 2009 at 3:06 PM, Stephen Butler <sb...@elego.de> wrote:
>> Hi Paul,
>>
>> summary: still hacking on groups 1 & 2, 3 is done, 4 is still open
>> for discussion.
>>
>> Some details inline, for anyone interested.
>>
>> Steve
>>
>> Quoting Paul Burba <pt...@gmail.com>:
>>
>>> On Thu, Jan 22, 2009 at 11:20 AM, Stephen Butler <sb...@elego.de> wrote:
>>>>
>>>> Quoting Paul Burba <pt...@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.
>>
>> [...]
>>>>>
>>>>> =======
>>>>> 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.
>>
>> [...]
>>>>
>>>> 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
>>>> conflict.
>>
>> I think I have a fix for this, but am running into OS X "bus error",
>> I think due to SQLite, so I can't run tests and commit yet. :-(
>>
>> [...]
>>>>>
>>>>> trunk>svn up checkout_tests-13
>>>>> ......subversionlibsvn_wcadm_files.c:672: (apr_err=155000)
>>>>> svn: Revision 3 doesn't match existing revision 1 in
>>>>> 'checkout_tests-13/A/D/M'
>>>>
>>>> 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.
>>
>> The fix is part of the fix mentioned above.
>>
>>>>
>>>>>
>>>>>
>>>>> =======
>>>>> 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.
>>
>> [...]
>>>>>
>>>>> 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.
>>
>> Not done yet.
>>
>>>>>
>>>>>
>>>>> =======
>>>>> 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.
>>
>> Better docstrings checked in on trunk as r35683. At least I'm able
>> to finish something today!
>>
>>>>>
>>>>> =======
>>>>> 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.
>>>>>
>>>
>>>
>>> Hi Steve,
>>>
>>> Any progress on any of the aforementioned todos? If not, will you be
>>> able to work on any of them? We still have issue #3209 as a blocker
>>> for 1.6 so we need to figure out what, if anything really needs to be
>>> done before 1.6.
>>>
>>> Paul
>>>
>>
>>
>>
>> --
>> 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
>>
>>
>>
>>
>
>
>
> --
> Thanks
>
> Mark Phippard
> http://markphip.blogspot.com/
>
--
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
Re: Need to resolve expected behavior of XFailing tests caused by
tree conflicts (Issue #3209)
Posted by Mark Phippard <ma...@gmail.com>.
Stephen,
To clarify ...
Group 3 is "done". Does that mean the test is no longer XFAIL? It
seemed like you just changed the description of the test from the
comments. But perhaps that was all Paul was pointing out needed to be
done. I guess my assumption with this issue is that when we are
"done" the tests will no longer be XFAIL.
You are working on groups 1 & 2. Do you think you'll be able to bring
these to completion? Any ideas on how much work is left/when it will
be completed?
Group 4 ... what is your take on Paul's comments? Is this just an
area where there could be differences of opinion but it is working as
intended. Or is something broken and this is an item where more work
needs to be done?
Thanks
Mark
On Wed, Feb 4, 2009 at 3:06 PM, Stephen Butler <sb...@elego.de> wrote:
> Hi Paul,
>
> summary: still hacking on groups 1 & 2, 3 is done, 4 is still open
> for discussion.
>
> Some details inline, for anyone interested.
>
> Steve
>
> Quoting Paul Burba <pt...@gmail.com>:
>
>> On Thu, Jan 22, 2009 at 11:20 AM, Stephen Butler <sb...@elego.de> wrote:
>>>
>>> Quoting Paul Burba <pt...@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.
>
> [...]
>>>>
>>>> =======
>>>> 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.
>
> [...]
>>>
>>> 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
>>> conflict.
>
> I think I have a fix for this, but am running into OS X "bus error",
> I think due to SQLite, so I can't run tests and commit yet. :-(
>
> [...]
>>>>
>>>> trunk>svn up checkout_tests-13
>>>> ......subversionlibsvn_wcadm_files.c:672: (apr_err=155000)
>>>> svn: Revision 3 doesn't match existing revision 1 in
>>>> 'checkout_tests-13/A/D/M'
>>>
>>> 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.
>
> The fix is part of the fix mentioned above.
>
>>>
>>>>
>>>>
>>>> =======
>>>> 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.
>
> [...]
>>>>
>>>> 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.
>
> Not done yet.
>
>>>>
>>>>
>>>> =======
>>>> 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.
>
> Better docstrings checked in on trunk as r35683. At least I'm able
> to finish something today!
>
>>>>
>>>> =======
>>>> 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.
>>>>
>>
>>
>> Hi Steve,
>>
>> Any progress on any of the aforementioned todos? If not, will you be
>> able to work on any of them? We still have issue #3209 as a blocker
>> for 1.6 so we need to figure out what, if anything really needs to be
>> done before 1.6.
>>
>> Paul
>>
>
>
>
> --
> 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
>
>
>
>
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1107759
Re: Need to resolve expected behavior of XFailing tests caused by
tree conflicts (Issue #3209)
Posted by Stephen Butler <sb...@elego.de>.
Hi Paul,
summary: still hacking on groups 1 & 2, 3 is done, 4 is still open
for discussion.
Some details inline, for anyone interested.
Steve
Quoting Paul Burba <pt...@gmail.com>:
> On Thu, Jan 22, 2009 at 11:20 AM, Stephen Butler <sb...@elego.de> wrote:
>> Quoting Paul Burba <pt...@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.
[...]
>>> =======
>>> 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.
[...]
>> 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
>> conflict.
I think I have a fix for this, but am running into OS X "bus error",
I think due to SQLite, so I can't run tests and commit yet. :-(
[...]
>>>
>>> trunk>svn up checkout_tests-13
>>> ......subversionlibsvn_wcadm_files.c:672: (apr_err=155000)
>>> svn: Revision 3 doesn't match existing revision 1 in
>>> 'checkout_tests-13/A/D/M'
>>
>> 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.
The fix is part of the fix mentioned above.
>>
>>>
>>>
>>> =======
>>> 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.
[...]
>>> 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.
Not done yet.
>>>
>>>
>>> =======
>>> 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.
Better docstrings checked in on trunk as r35683. At least I'm able
to finish something today!
>>>
>>> =======
>>> 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.
>>>
>
>
> Hi Steve,
>
> Any progress on any of the aforementioned todos? If not, will you be
> able to work on any of them? We still have issue #3209 as a blocker
> for 1.6 so we need to figure out what, if anything really needs to be
> done before 1.6.
>
> Paul
>
--
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