Re: Property change notifications for "svn:mergeinfo"

On Fri, 06 Apr 2007, Paul Burba wrote:

> I'll try to address both Dan and Mike's comments below
>
> > -----Original Message-----
> > From: Daniel Rall [mailto:dlr@collab.net]
> > Sent: Thursday, April 05, 2007 8:07 PM
> >
> > Handling of the "svn:mergeinfo" property was hacked into
> > svn_wc__merge_props() in all the places that previously
> > resulted in property conflicts, adding intelligent merging of
> > the merge info itself. However, we aren't currently setting
> > the property state output variable to the appropriate value
> > for each merge.
> >
> > Should we announce property changes to the "svn:mergeinfo" property?
> > I think so.
>
> > -----Original Message-----
> > From: C. Michael Pilato [mailto:cmpilato@collab.net]
> > Sent: Friday, April 06, 2007 1:58 AM
> >
> > If, in fact, merge tracking information is merely
> > "application specific book keeping data", then I think the
> > choice to use Subversion properties to hold it was not such a
> > good one.
> >
> > Fortunately, it's *not* just stuff users should never see.
> > It was designed to be human-hackable: merge-tracking.txt
> > lists as a goal the ability "to edit merge information in a
> > human editable form."
> >
> > Now, that's not to say folks won't want some way to
> > optionally silence the mergeinfo diff spew from 'svn diff' --
> > but I think that should be optional behavior. And I don't
> > see any reason to hide the fact that the 'svn:mergeinfo'
> > property has changed in 'svn status', 'svn update', etc.
>
> A little clarification: svn status, update, switch, diff, etc. *do*
> treat svn:mergeinfo just like any other property. Nothing being hidden,
> suppressed, or otherwise skipped there that I know nor was I proposing
> that we should do that.
>
> My original observation of the current trunk behavior had only to do
> with 'svn merge' and its notification of svn:mergeinfo changes. Take
> this simple example starting with (you know what's coming...) the
> standard greek test tree (...I love the greek tree!):
>
> # Make a branch with
> # a URL --> WC copy
> --------------------
> >svn copy %URL%/A A_COPY
> A A_COPY\B
> A A_COPY\B\lambda
> A A_COPY\B\E
> A A_COPY\B\E\alpha
> A A_COPY\B\E\beta
> A A_COPY\B\F
> A A_COPY\mu
> A A_COPY\C
> A A_COPY\D
> A A_COPY\D\gamma
> A A_COPY\D\G
> A A_COPY\D\G\pi
> A A_COPY\D\G\rho
> A A_COPY\D\G\tau
> A A_COPY\D\H
> A A_COPY\D\H\chi
> A A_COPY\D\H\omega
> A A_COPY\D\H\psi
> Checked out revision 1.
> A A_COPY
>
> # The copy results in some
> # mergeinfo on the branch
> --------------------------------
> >svn pl -vR
> Properties on 'A_COPY':
> svn:mergeinfo : /A:1
>
> # Make a change in the branch
> # source and commit it and the
> # branch itself as r2
> -----------------------------
> >echo new content > A\B\E\alpha
>
> >svn ci -m ""
> Sending A\B\E\alpha
> Adding A_COPY
> Transmitting file data .
> Committed revision 2.
>
> # Now merge r2 into the branch
> ------------------------------
> >svn merge %URL%/A/B A_COPY\B -c2
> U A_COPY\B\E\alpha
>
> OK, finally, here is the crux of why svn:mergeinfo seems different to
> me:
>
> %URL%/A/B has no svn:mergeinfo property
>
> and
>
> A_COPY\B has no svn:mergeinfo property
>
> So why would we expect to see 'UU A_COPY\B\E\alpha' as the merge
> result?
>
> Of course after the merge A_COPY\B "magically" has svn:mergeinfo:
>
> >svn pl -vR
> Properties on 'A_COPY':
> svn:mergeinfo : /A:1
> Properties on 'A_COPY\B':
> svn:mergeinfo : /A/B:1-2
>
> So maybe the merge should look like this instead?
> >svn merge %URL%/A/B A_COPY\B -c2
> UU A_COPY\B\E\alpha
> ^
> But doesn't that imply some property existed on the URL and/or WC before
> the merge? Of course if we merged into 'A_COPY' that would be true and
> my argument for svn:mergeinfo specialness kinda goes out the window a
> bit :-\
>
> FWIW, the fact that the mergeinfo has been added is still clear when we
> run svn status or diff:
...

When we apply our merge info-specific property merging logic to
property merges of the "svn:mergeinfo" property, we were failing to
send any property notifications. I added reporting of these
notifications in r24738.

I think we should have a unit test here as well.