On Sat, Jan 31, 2009 at 3:50 AM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> On Fri, 2009-01-30 at 17:58 -0500, Paul Burba wrote:
>> On Wed, Jan 28, 2009 at 3:48 PM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
>> > Blocker:
>> >>
>> >> * #3334: Tree conflicts "merry-go-round" about update updating the base.
>> >> Julian Foad is working on this. Done for when victim is a file, still
>> >> doing for when victim is a directory. [julianfoad]
>> >> See: <http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1019712>.
>> >
>> > I'm struggling with this and could use some help. I have rather little
>> > time to look at it these days.
>> >
>> > On the issue-3334-dirs branch, the remaining problem is "just" a matter
>> > of scheduling a directory tree to be re-added as a copy of what it was
>> > before.
>> >
>> > The function "schedule_existing_item_for_re_add()" tries to do this, but
>> > doesn't get the result quite right.
>> >
>> > On the branch I added a new "test" (update_tests.py 53). This "test"
>> > doesn't actually test anything, but just runs the tree-conflict
>> > scenario, and also runs a manual schedule-as-re-add command sequence, so
>> > that we can (manually) compare the resulting "entries" files.
>> >
>> > In test 53, the tree conflict victim is directory A/ and A's THIS_DIR
>> > entry needs a "revision" of 1, but it gets a revision of 2. That's all
>> > that is wrong with its THIS_DIR entry. There is nothing wrong with its
>> > children, I think. The only other problem is A's entry in its parent,
>> > which gets several fields wrong (different from the "manual copy" WC).
>> >
>> > Please could someone have a look at the differences between the entries
>> > files in test 53's two WCs, and see how to make
>> > "schedule_existing_item_for_re_add()" create that state.
>>
>> Julian (and/or any other TCers),
>>
>> I've got this test working,
>
> Phew! Thanks, Paul.
>
>> but I'm wondering about a slightly
>> different case than the one covered in the test.
>
> I forgot that the new test 53 only currently makes a prop-mod on the
> victim dir. It was intended to cover any kind of modification within the
> tree. You might (or I will) update it like this:
>
> [[[
> Index: subversion/tests/cmdline/update_tests.py
> ===================================================================
> --- subversion/tests/cmdline/update_tests.py (revision 35544)
> +++ subversion/tests/cmdline/update_tests.py (working copy)
> @@ -4311,6 +4311,16 @@
> run_and_verify_svn(None, AnyOutput, [],
> 'propset', 'p', 'v', dir)
>
> + path = os.path.join(dir, 'new_file')
> + svntest.main.file_write(path, "This is the file 'new_file'.\n")
> + svntest.actions.run_and_verify_svn(None, None, [], 'add', path)
> +
> + path = os.path.join(dir, 'B', 'lambda')
> + svntest.actions.run_and_verify_svn(None, None, [], 'delete', path)
> +
> + path = os.path.join(dir, 'B', 'E', 'alpha')
> + svntest.main.file_append(path, "An extra line.\n")
> +
> # Prepare the repos so that a later 'update' has an incoming deletion:
> # Delete the dir in the repos, making r2
> run_and_verify_svn(None, AnyOutput, [],
> ]]]
>
>
>> What if, instead of having a modification directly to the directory
>> that the update will attempt to delete, we instead have a local mod
>> only to a subtree of that directory? For example, say we start with
>> this vanilla Greek WC:
>>
>> issue3334.dgb>svn st -v
>> 1 1 jrandom .
>> 1 1 jrandom A
>> 1 1 jrandom A\B
>> 1 1 jrandom A\B\lambda
>> 1 1 jrandom A\B\E
>> 1 1 jrandom A\B\E\alpha
>> 1 1 jrandom A\B\E\beta
>> 1 1 jrandom A\B\F
>> 1 1 jrandom A\mu
>> 1 1 jrandom A\C
>> 1 1 jrandom A\D
>> 1 1 jrandom A\D\gamma
>> 1 1 jrandom A\D\G
>> 1 1 jrandom A\D\G\pi
>> 1 1 jrandom A\D\G\rho
>> 1 1 jrandom A\D\G\tau
>> 1 1 jrandom A\D\H
>> 1 1 jrandom A\D\H\chi
>> 1 1 jrandom A\D\H\omega
>> 1 1 jrandom A\D\H\psi
>> 1 1 jrandom iota
>>
>> Then we delete 'A' directly in the repos:
>>
>> issue3334.dgb>svn del %url53%/A -m ""
>>
>> Committed revision 2.
>>
>> Then we make a text mod in our WC to a subtree of 'A':
>>
>> issue3334.dgb>echo new content > A\D\G\pi
>>
>> issue3334.dgb>svn up
>> C A
>> At revision 2.
>> Summary of conflicts:
>> Tree conflicts: 1
>>
>> Ok, the tree conflict is correct, but what should be scheduled for
>> addition? Should the entire subtree rooted at 'A' be schedule for
>> addition as a copy from A_at_1?
>>
>> issue3334.dgb>svn st -v
>> 2 2 pburba .
>> A + C - 1 jrandom A
>> > local edit, incoming delete upon update
>> + - 1 jrandom A\B
>> + - 1 jrandom A\B\lambda
>> + - 1 jrandom A\B\E
>> + - 1 jrandom A\B\E\alpha
>> + - 1 jrandom A\B\E\beta
>> + - 1 jrandom A\B\F
>> + - 1 jrandom A\mu
>> + - 1 jrandom A\C
>> + - 1 jrandom A\D
>> + - 1 jrandom A\D\gamma
>> + - 1 jrandom A\D\G
>> M + - 1 jrandom A\D\G\pi
>> + - 1 jrandom A\D\G\rho
>> + - 1 jrandom A\D\G\tau
>> + - 1 jrandom A\D\H
>> + - 1 jrandom A\D\H\chi
>> + - 1 jrandom A\D\H\omega
>> + - 1 jrandom A\D\H\psi
>> 2 1 jrandom iota
>>
>> *OR* should only the modified subtree 'A/D/G/pi' and its path-wise
>> ancestors be present?
>>
>> issue3334.dgb>svn st -v
>> 2 2 pburba .
>> A + C - 1 jrandom A
>> > local edit, incoming delete upon update
>> + - 1 jrandom A\D
>> + - 1 jrandom A\D\G
>> M + - 1 jrandom A\D\G\pi
>>
>> 2 1 jrandom iota
>>
>> I've got the former working*, but I wonder if the latter is not
>> actually the correct behavior.
>>
>> Opinions appreciated,
>
> The former has a logical elegance that I like: "if the incoming change
> wants to delete a tree that has local mods, then don't delete it yet".
> The latter has a kind of minimalism, which can also be a good thing in
> itself. In either case the user can very easily choose "theirs" (revert
> -R).
>
> However, if the user wants to keep "mine", even if only temporarily to
> examine/test/review his changes, then the former provides the context in
> which he can do that before deciding how to resolve it. The latter
> doesn't and I'm not sure there's any simple way to bring that context
> back. Certainly if the user decides that the tree should not have been
> deleted, it is not as simple as "svn resolved --accept=mine".
>
> So I think the former is what we need.
Another thought, the ideal behavior probably depends a lot on if the
incoming delete is part of a move or not. If it is solely a delete,
having the context of entire subtree in addition to our local mods
makes a lot of sense no? OTOH if the delete is part of a move, then
*maybe* having only the modified portions of the deleted subtree makes
sense.
Also, the nature of the local mods matters. At one extreme consider a
modification that consists solely of a text change to an immediate
file child of a large the deleted subtree. Scheduling the entire
subtree for addition does seem a bit excessive. But what if your
local mod consists of changes to dozens of paths scattered around the
subtree, then having the context Julian speaks of would be quite
desirable.
Regardless, I still need to address several other test failures. For
now I will proceed with the former option (as that is what the code
already does). Once I determine that the test failures are all a
matter of changing expectations, rather than new bugs, then we can
make a final determination on this question.
Paul
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1091117
Received on 2009-02-02 16:35:36 CET