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/02 15:33:41 UTC
Re: Help on 1.6-blocker #3334 - tree conflicts in update
On Sat, Jan 31, 2009 at 3:50 AM, Julian Foad <ju...@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 <ju...@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@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
Re: Help on 1.6-blocker #3334 - tree conflicts in update
Posted by Julian Foad <ju...@btopenworld.com>.
On Mon, 2009-02-02 at 11:20 -0500, Mark Phippard wrote:
> On Mon, Feb 2, 2009 at 11:15 AM, Julian Foad <ju...@btopenworld.com> wrote:
>
> >> 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?
> >
> > Yes.
> >
> >> OTOH if the delete is part of a move, then
> >> *maybe* having only the modified portions of the deleted subtree makes
> >> sense.
> >
> > Maybe.
> >
> >> 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.
> >
> > Excessive? It's not as if we're adding stuff that wasn't there. It was
> > all there in the WC just before you ran "update". Why is it excessive if
> > the update says "Hold on, the incoming change wants to delete this huge
> > tree but I'm not going to do that to you just yet, I'll wait until you
> > tell me to resolve as theirs."
>
> The fact that there is no obvious answer we can agree on tells me
> let's go with what we have and see where that leads under real world
> usage.
>
> Julian, the only part that is kind of excessive is the user recovery
> process. If a user wants to preserve that single file in a deep tree
> how many commands do they have to run to unschedule all the other
> files and clean up their WC?
Yes. I don't know which scenarios will be more common, and this one
didn't seem likely to me. But I was just going through your other mail
and basically concluding that yes, this scenario (re-create single file
with parents in deep tree) is real, as well as the other scenarios. And
it doesn't have such easy commands to achieve. A "revert recursively all
schedule-added files except added-and-modified files" is the sort of
command that would help, but we don't have it.
> I tend to think we cannot win no matter what we do so I like the idea
> of going with what we have now and see if it hits the mark for users
> or creates new problems.
OK,
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1091303
Re: Help on 1.6-blocker #3334 - tree conflicts in update
Posted by Mark Phippard <ma...@gmail.com>.
On Mon, Feb 2, 2009 at 11:15 AM, Julian Foad <ju...@btopenworld.com> wrote:
>> 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?
>
> Yes.
>
>> OTOH if the delete is part of a move, then
>> *maybe* having only the modified portions of the deleted subtree makes
>> sense.
>
> Maybe.
>
>> 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.
>
> Excessive? It's not as if we're adding stuff that wasn't there. It was
> all there in the WC just before you ran "update". Why is it excessive if
> the update says "Hold on, the incoming change wants to delete this huge
> tree but I'm not going to do that to you just yet, I'll wait until you
> tell me to resolve as theirs."
The fact that there is no obvious answer we can agree on tells me
let's go with what we have and see where that leads under real world
usage.
Julian, the only part that is kind of excessive is the user recovery
process. If a user wants to preserve that single file in a deep tree
how many commands do they have to run to unschedule all the other
files and clean up their WC?
I tend to think we cannot win no matter what we do so I like the idea
of going with what we have now and see if it hits the mark for users
or creates new problems.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1091281
Re: Help on 1.6-blocker #3334 - tree conflicts in update
Posted by Julian Foad <ju...@btopenworld.com>.
On Mon, 2009-02-02 at 10:33 -0500, Paul Burba wrote:
> On Sat, Jan 31, 2009 at 3:50 AM, Julian Foad <ju...@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 <ju...@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@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?
Yes.
> OTOH if the delete is part of a move, then
> *maybe* having only the modified portions of the deleted subtree makes
> sense.
Maybe.
> 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.
Excessive? It's not as if we're adding stuff that wasn't there. It was
all there in the WC just before you ran "update". Why is it excessive if
the update says "Hold on, the incoming change wants to delete this huge
tree but I'm not going to do that to you just yet, I'll wait until you
tell me to resolve as theirs."
Sure, if it's a move and if the "add" half of the move happens, then
yes, that part of the WC is doubled until you resolve... but that's not
a big deal.
> 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.
Thanks, Paul.
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1091260