On Fri, Mar 16, 2012 at 1:47 PM, Greg Stein <gstein_at_gmail.com> wrote:
>
> On Mar 16, 2012 8:32 AM, "Stefan Sperling" <stsp_at_elego.de> wrote:
>>
>> On Fri, Mar 16, 2012 at 07:58:18AM -0400, Greg Stein wrote:
>> > On Mar 16, 2012 5:30 AM, <philip_at_apache.org> wrote:
>> > >...
>> > > +++ subversion/trunk/subversion/tests/cmdline/update_tests.py Fri Mar
>> > > 16
>> > 09:29:49 2012
>> > > @@ -5696,6 +5696,101 @@ def update_moved_dir_file_move(sbox):
>> > > None, None, None,
>> > > None, None, 1)
>> > >
>> > > +@XFail()
>> > > +def update_move_text_mod(sbox):
>> > > + "text mod to moved files"
>> > > +
>> > > + sbox.build()
>> > > + wc_dir = sbox.wc_dir
>> > > + svntest.main.file_append(sbox.ospath('A/B/lambda'), "modified\n")
>> > > + svntest.main.file_append(sbox.ospath('A/B/E/beta'), "modified\n")
>> > > + sbox.simple_commit()
>> > > + sbox.simple_update(revision=1)
>> > > +
>> > > + sbox.simple_move("A/B/E", "A/E2")
>> > > + sbox.simple_move("A/B/lambda", "A/lambda2")
>> > > +
>> > > + expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
>> > > + expected_status.tweak('A/B/E', 'A/B/E/alpha', 'A/B/E/beta',
>> > 'A/B/lambda',
>> > > + status='D ')
>> > > + expected_status.add({
>> > > + 'A/E2' : Item(status='A ', copied='+', wc_rev='-'),
>> > > + 'A/E2/alpha' : Item(status=' ', copied='+', wc_rev='-'),
>> > > + 'A/E2/beta' : Item(status=' ', copied='+', wc_rev='-'),
>> > > + 'A/lambda2' : Item(status='A ', copied='+', wc_rev='-'),
>> > > + })
>> > > +
>> > > + svntest.actions.run_and_verify_status(wc_dir, expected_status)
>> > > +
>> > > + expected_output = svntest.wc.State(wc_dir, {
>> > > + 'A/lambda2' : Item(status='U '),
>> > > + 'A/E2/beta' : Item(status='U '),
>> > > + })
>> >
>> > The above output for an 'svn update ' implies that the incoming edits
>> > will
>> > be applied to the moved files.
>>
>> Yes, finally!!!
>>
>> > I think that is wrong. I believe we should
>> > mark a conflict: incoming edit vs local move.
>>
>> IMO you'd really want text conflicts in this case, not a tree conflict.
>>
>> > The default resolution would
>> > be to apply, but I really do not believe we should silently update like
>> > this.
>> >
>> > Consider where I took some code that did X+Y and moved it to a new
>> > location, for use there (but not yet committed). Somebody else edits the
>> > code to remove Y, and commits.
>> >
>> > If I update, then the code I moved *changes* on me. I grabbed the code
>> > expecting X+Y. The removal of Y may have made sense in the old location,
>> > but not the new location.
>>
>> You *moved* the file to "grab the code". A moved file is still the same
>> "container" (for lack of a better term), and changes destined for this
>> container should follow the move.
>>
>> Why didn't you copy the file that contains the code you want to base
>> your changes on? If you don't care about keeping the "same container"
>> semantics, you should copy. And maybe even delete the source if you
>> like. That will still give you a tree conflict when the update brings
>> in changes for the deleted file.
>
> Don't tell me my use case is wrong. I described it, and that is what you now
> should work with.
>
> I thought the code didn't work at A. I wanted features X+Y at B rather than
> A. So I move it.
>
> Somebody else thought A should not implement Y.
>
> That is a conflict. Two different intentions. And I believe a tree conflict.
> I could just as well argue prop changes, and other more explicit changes
> within this overall move (eg. maybe I remove Y and an associated config
> file, yet the other guy adds Z that should not be in the code at path B).
>
> These should be tree conflicts, rather than silent application of edits to
> moved nodes.
>
> And don't tell me to use copy as a user strategy to avoid future changes and
> conflicts.
I do not agree, and I'm really looking forward to the day where
subversion merges text changes into a moved file automatically.
Of course it's a use case that you had another intention, in which
case this automatic resolution doesn't do the right thing for you. But
I think that's far, far less common than wanting these changes to be
merged automatically. If we really need to support both use cases,
we'd need to look into making the auto-resolution strategy
configurable by the user (but with sensible defaults please).
In my eyes, your example isn't really very different from: I moved a
block of code around in a file (or maybe changed the function
signature), and someone else committed the removal of a single line
inside this block. Now, upon update we currently also merge that
one-line change automatically into the moved block of code, right? I
hope we won't start arguing that this should create a text conflict
...
--
Johan
Received on 2012-03-16 13:59:03 CET