Re: valgrind UMRs in "svn merge --record-only"

Paul Burba <[email protected]> Fri, 4 Dec 2009 09:53:10 -0500
Newsgroups gmane.comp.version-control.subversion.devel,gmane.comp.version-control.subversion.rapidsvn.devel
Message-ID <[email protected]>
On Wed, Dec 2, 2009 at 10:41 AM, Philip Martin
<[email protected]> wrote:
> Paul Burba <[email protected]> writes:
>
>>> How is the user supposed to have any
>>> confidence that they have run the command correctly,
>>
>> How would a notification really help them know they ran the command
>> *correctly*?  Like any merge, choose the wrong source, the wrong
>> target, the wrong revision, you get the wrong result.  I don't see how
>> improved notifications avoid the need to check svn status, diff,
>> and/or propget svn:mergeinfo after the merge, to see if the result is
>> what was expected.  Admittedly --record-only merges are particularly
>> problematic because there are no text or tree conflicts to alert us
>> that we might have screwed up, but I still don't see how notifications
>> help users much in confirming correctness.
>
> I could ask why normal merge produces notification, after all the user
> has to check status and diff to ensure that the merge got applied
> correctly.  Does that mean the normal merge notification is of no use?

Point taken.

> The obvious case where record-only notifications help is when the
> merge doesn't change anything, because I got the wrong source URL or
> revision.  If I'm expecting to see a change and I get no notification
> then I know I made a mistake.

Ditto.

>> Possibly I am thinking of too simplistic a notification.  I assume the
>> notification would be something like this in the simple case (i.e. no
>> pre-existing subtree mergeinfo, no incoming mergeinfo diffs):
>>
>>   svn merge ^^/trunk branches\b1.0  -c357 --record-only
>>   --- Recording mergeinfo for r357 into 'branches\b1.0':
>>    U    branches\b1.0
>
> I was thinking just the U line.

I think a "record only" header is necessary.  Now that --record-only
merges may apply svn:mergeinfo diffs, we might also see traditional
merge notification headers.  Without a special header it looks like
all notifications are related to the diff, which IMO is quite
confusing.

For example, we have a WC merge target with no pre-existing mergeinfo:

  >svn pg svn:mergeinfo -vR A2\D\G

We want to merge a rev from some other branch that includes
svn:mergeinfo property changes:

  >svn diff ^^/A_COPY/D/G -c11
  Index: rho
  ===================================================================
  --- rho (revision 10)
  +++ rho (revision 11)
  @@ -1 +1 @@
  -This is the file 'rho'.
  +New content
  \ No newline at end of file

  Property changes on: rho
  ___________________________________________________________________
  Added: svn:mergeinfo
     Merged /A/D/G/rho:r4

Currently in trunk(@887205) the merge produces a notification only for
the mergeinfo change:

  >svn merge --record-only -c11 ^^/A_COPY/D/G A2\D\G
  --- Merging r11 into 'A2\D\G':
   U   A2\D\G\rho

But of course the --record-only logic recorded mergeinfo *describing*
the merge on the target too:

  >svn st
   M      A2\D\G
   M      A2\D\G\rho

  >svn pg svn:mergeinfo -vR A2\D\G
  Properties on 'A2\D\G\rho':
    svn:mergeinfo
      /A/D/G/rho:4
      /A_COPY/D/G/rho:11
  Properties on 'A2\D\G':
    svn:mergeinfo
      /A_COPY/D/G:11

If we add notifications for mergeinfo changes describing the merge it
would look something like this...

  >svn merge --record-only -c11 ^^/A_COPY/D/G A2\D\G
  --- Merging r11 into 'A2\D\G':
   U   A2\D\G\rho
   U   A2\D\G

...Which makes it appear that the change to A2/D/G was part of the
diff.  I think it is a lot clearer if the mergeinfo changes made to
describe the merge have their own special header:

  >svn merge --record-only -c11 ^^/A_COPY/D/G A2\D\G
  --- Merging r11 into 'A2\D\G':
   U   A2\D\G\rho
  --- Recording mergeinfo for r11 into 'A2\D\G':
   U   A2\D\G

This isn't a big deal in simple examples, but if there are mergeinfo
diffs applied to many subtrees *and* many subtrees with pre-existing
mergeinfo, then things get quite ugly.

Paul

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427057

Please start new threads on the <[email protected]> mailing list.
To subscribe to the new list, send an empty e-mail to <[email protected]>.