You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stephen Butler <sb...@elego.de> on 2009/02/16 14:15:42 UTC
Issue #3209: merge into missing
Hi tree conflict fans, and 1.6 fans,
my thoughts, this time regarding a subject on which I'm not
completely ignorant... :-\
The status of 1.6-blocker #3209 (see r35877):
[[[
Tackle the last holdout from issue #3209: merge encounters missing
files and dirs. Create tests that try to merge changes into missing
trees. Make them pass, given the current behavior.
The current behavior is inconsistent. A missing file is not treated
as a merge tree conflict. A missing dir, that the merge wants to
delete, is also not treated as a tree conflict.
The next steps are to determine the desired behavior, adjust the test
expectations, and set the tests to XFAIL. Then fix it!
* subversion/tests/cmdline/svntest/actions.py
(deep_trees_rmtree): New helper function.
* subversion/tests/cmdline/merge_tests.py
(tree_conflicts_merge_edit_onto_missing,
tree_conflicts_merge_del_onto_missing): New tests.
(test_list): Add new tests.
]]]
It's been suggested that we shouldn't raise any tree conflicts on
missing items.
http://svn.haxx.se/dev/archive-2009-02/0264.shtml
I went ahead and implemented this behavior (still testing before
commit). The tree conflicts due to local deletions are still
raised. The current change affects only missing items.
IMHO, it's OK to have no tree conflict here. The user gets a
summary at the end of the merge output, which is hard to miss.
Summary of conflicts:
Skipped paths: 1
And of course the user can't commit without updating to restore
the missing items.
It's a little weird that a missing file is given empty mergeinfo,
but a missing dir is not. Is that the way it's supposed to be?
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: Issue #3209: merge into missing
Posted by Stephen Butler <sb...@elego.de>.
Quoting Stefan Sperling <st...@elego.de>:
> On Mon, Feb 16, 2009 at 05:05:27PM +0100, Stephen Butler wrote:
>> Quoting Stephen Butler <sb...@elego.de>:
>>
>>> The status of 1.6-blocker #3209 (see r35877):
>>
>> The good news: this isn't really a blocker. The current inconsistency
>> in tree conflict detection is a bug, but the working copy is left in
>> a usable state.
>>
>> The bad news: the bug might not be fixable in the current design of
>> the client.
>>
>> Background: the merge code runs in a second set of callbacks (in
>> libsvn_wc/repos_diff.c) on top of the usual delta editor callbacks.
>>
>> The current state: within a repos_diff callback, we can't tell
>> whether a nonexistent dir is really nonexistent, or just missing.
>> The former is a tree conflict, the latter should not be a TC.
>>
>> The repos_diff callbacks do a great job of filtering out the info
>> we really need for tree conflict detection! :-\ But that's been
>> discussed at length last year. In r33989, Neels & co. added a
>> tree_conflicted boolean arg to all the relevant callbacks, so that
>> all of the tree conflict detection can be done in
>> libsvn_client/merge.c, where it's easy to follow. This lets us
>> avoid raising a tree conflict inside a TC victim tree, but it's
>> not enough to tell whether an item is really nonexistent or is
>> just missing.
>
> Would passing an enum instead of a boolean help here?
I think so. We could maintain the "deep" tree status in repos_diff.c
and pass an svn_wc_notify_state_t to the merge callbacks instead of
the boolean. I'll give it a try.
These callbacks are private. So I don't need to rev an API, right?
>
> Is this related to the problem of needing to "reimplement diff"
> in order to handle use case 5 for directories properly?
No, this is for use case 4 "local delete, incoming edit", where we'd
like to distinguish between locally deleted and locally missing.
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: Issue #3209: merge into missing
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Feb 16, 2009 at 05:05:27PM +0100, Stephen Butler wrote:
> Quoting Stephen Butler <sb...@elego.de>:
>
>> The status of 1.6-blocker #3209 (see r35877):
>
> The good news: this isn't really a blocker. The current inconsistency
> in tree conflict detection is a bug, but the working copy is left in
> a usable state.
>
> The bad news: the bug might not be fixable in the current design of
> the client.
>
> Background: the merge code runs in a second set of callbacks (in
> libsvn_wc/repos_diff.c) on top of the usual delta editor callbacks.
>
> The current state: within a repos_diff callback, we can't tell
> whether a nonexistent dir is really nonexistent, or just missing.
> The former is a tree conflict, the latter should not be a TC.
>
> The repos_diff callbacks do a great job of filtering out the info
> we really need for tree conflict detection! :-\ But that's been
> discussed at length last year. In r33989, Neels & co. added a
> tree_conflicted boolean arg to all the relevant callbacks, so that
> all of the tree conflict detection can be done in
> libsvn_client/merge.c, where it's easy to follow. This lets us
> avoid raising a tree conflict inside a TC victim tree, but it's
> not enough to tell whether an item is really nonexistent or is
> just missing.
Would passing an enum instead of a boolean help here?
Is this related to the problem of needing to "reimplement diff"
in order to handle use case 5 for directories properly?
Stefan
Re: Issue #3209: merge into missing
Posted by Stephen Butler <sb...@elego.de>.
Quoting Stephen Butler <sb...@elego.de>:
> The status of 1.6-blocker #3209 (see r35877):
The good news: this isn't really a blocker. The current inconsistency
in tree conflict detection is a bug, but the working copy is left in
a usable state.
The bad news: the bug might not be fixable in the current design of
the client.
Background: the merge code runs in a second set of callbacks (in
libsvn_wc/repos_diff.c) on top of the usual delta editor callbacks.
The current state: within a repos_diff callback, we can't tell
whether a nonexistent dir is really nonexistent, or just missing.
The former is a tree conflict, the latter should not be a TC.
The repos_diff callbacks do a great job of filtering out the info
we really need for tree conflict detection! :-\ But that's been
discussed at length last year. In r33989, Neels & co. added a
tree_conflicted boolean arg to all the relevant callbacks, so that
all of the tree conflict detection can be done in
libsvn_client/merge.c, where it's easy to follow. This lets us
avoid raising a tree conflict inside a TC victim tree, but it's
not enough to tell whether an item is really nonexistent or is
just missing.
>
> [[[
> Tackle the last holdout from issue #3209: merge encounters missing
> files and dirs. Create tests that try to merge changes into missing
> trees. Make them pass, given the current behavior.
>
> The current behavior is inconsistent. A missing file is not treated
> as a merge tree conflict. A missing dir, that the merge wants to
> delete, is also not treated as a tree conflict.
>
> The next steps are to determine the desired behavior, adjust the test
> expectations, and set the tests to XFAIL. Then fix it!
>
> * subversion/tests/cmdline/svntest/actions.py
> (deep_trees_rmtree): New helper function.
>
> * subversion/tests/cmdline/merge_tests.py
> (tree_conflicts_merge_edit_onto_missing,
> tree_conflicts_merge_del_onto_missing): New tests.
> (test_list): Add new tests.
> ]]]
>
> It's been suggested that we shouldn't raise any tree conflicts on
> missing items.
>
> http://svn.haxx.se/dev/archive-2009-02/0264.shtml
>
> I went ahead and implemented this behavior (still testing before
> commit).
I won't commit my new hack, because it causes us to miss a lot of
tree conflicts in use case 4, "local (committed) delete, incoming
edit upon merge".
> The tree conflicts due to local deletions are still
> raised.
Oops, my quick hack raises them only for uncommitted deletions.
See above.
> The current change affects only missing items.
Not true, see above. It prevents the detection of nonexistent
items, too.
>
> IMHO, it's OK to have no tree conflict here. The user gets a
> summary at the end of the merge output, which is hard to miss.
>
> Summary of conflicts:
> Skipped paths: 1
>
> And of course the user can't commit without updating to restore
> the missing items.
>
> It's a little weird that a missing file is given empty mergeinfo,
> but a missing dir is not. Is that the way it's supposed to be?
Still an open question.
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