Re: [mergeinfo or wc-ng] 'svn merge -c-N; svn merge -cN' leaving changes behind?

[Daniel Shahaf <d....@daniel.shahaf.name>]

Paul,

Thanks for the extremely detailed technical+historical explanation and for
the suggested workarounds.  I admit I was a bit surprised to find out the
sequence in the subject isn't a noop, but at least now I know why :-)

Thanks again,

Daniel

Paul Burba wrote on Mon, Aug 02, 2010 at 09:59:15 -0400:
> On Sat, Jul 31, 2010 at 7:13 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > Reverse-merging a change, and then re-merging it, falsely causes 'status' and
> > 'diff' to think the affected files are still changed:
> 
> Hi Daniel,
> 
> This is issue #2881 'Support negated mergeinfo revision ranges'
> (http://subversion.tigris.org/issues/show_bug.cgi?id=2881)
> 
> The problem is that we currently have no way of representing reversed
> merges from a path's own history.
> 
> Here is what is happening in your example:
> 
> 1) A reverse merge from a path's "natural" history:
> 
> svn merge -c -979045 trunk-WC-at-some-rev-younger-than-r979045
> 
> The merge tracking logic asks 'should this merge be allowed?' and it
> answers this question this way:
> 
>   a) Is '/subversion/trunk:979045' represented in the explicit
>   mergeinfo on the WC target?
> 
>   b) If the WC target has no explicit mergeinfo, is  '/subversion/trunk:979045'
>   represented in the inherited mergeinfo of the WC target?
> 
>   c) Is '/subversion/trunk:979045' represented in the history of
>   the WC target?
> 
> During the development of merge tracking we made the decision that the
> answer to one of these questions has to be yes to allow the merge to
> proceed.  Otherwise some very basic use cases won't work, e.g. sync
> merges to a branch.
> 
> Now the trunk WC target has explicit mergeinfo, but
> '/subversion/trunk:979045' isn't in it.  And since the path has
> explicit mergeinfo, it can't inherit any.  But,
> '/subversion/trunk:979045' is part of trunk's own history at this
> point, so the merge is allowed:
> 
>   C:\SVN\src-trunk-3-WCNG>svn merge -c -979045 .
>   --- Reverse-merging r979045 into '.':
>   U    subversion\mod_dav_svn\reports\update.c
>   U    subversion\libsvn_ra_neon\fetch.c
>   --- Recording mergeinfo for reverse merge of r979045 into '.':
>    U   .
> 
> But notice that after the merge, there is no change in the mergeinfo:
> 
>   C:\SVN\src-trunk-3-WCNG>svn st
>   M       subversion\mod_dav_svn\reports\update.c
>   M       subversion\libsvn_ra_neon\fetch.c
> 
> This *is* issue 2881, there is no way to record this merge, there is
> no way to say '/subversion/trunk:-979045' (note the minus sign).
> 
> Sidebar: Yes, the notification "--- Recording mergeinfo for reverse
> merge of r979045 into '.':" is a bit misleading here, because there is
> no net change to the mergeinfo.
> 
> Anyhow, now you reverse the reverse merge, but it is a noop:
> 
>   C:\SVN\src-trunk-3-WCNG>svn merge -c 979045 .
> 
>   C:\SVN\src-trunk-3-WCNG>
> 
> Why?  Because the merge tracking logic asks the same three questions:
> 
> Is '/subversion/trunk:979045' represented in the explicit/inherited
> mergeinfo or is it part of the target's natural history?
> 
> Again, as far as it can tell, r979045 is part of the target's history,
> so merge tracking does nothing, since as one of it's most basic
> functions, it doesn't want to repeat "merges" that have already been
> performed.
> 
> Does that make sense?
> 
> If we had a way of representing the first reverse merge in the
> target's mergeinfo, then the subsequent forward merge would see that
> r979045 is not a repeat merge and would permit it.  This has never
> been fixed because:
> 
> 1) There are easy work-arounds (e.g. use --ignore-ancestry or assuming
> the first reverse merge was committed in rN, reverse merge rN)
> 
> 2) Nobody is clamoring for it
> 
> 3) The law of unintended consequences and merge tracking go together
> like chocolate chip cookies and milk.  Ok, admittedly this isn't a
> very good reason, but I'm hesitant to add a whole new layer of
> complexity to MT without a more compelling use case.
> 
> Paul
> 
> > [[[
> > 0:% $svn st
> > (clean; nothing relevant)
> > 0:% $svn merge -c -979045 .
> > --- Reverse-merging r979045 into '.':
> > U    subversion/mod_dav_svn/reports/update.c
> > U    subversion/libsvn_ra_neon/fetch.c
> > --- Recording mergeinfo for reverse merge of r979045 into '.':
> >  U   .
> > 0:% $svn merge -c 979045 .
> > 0:% $svn st --depth=empty ./ ; $svn st subversion/*neon subversion/mod*
> > M       subversion/libsvn_ra_neon/fetch.c
> > M       subversion/mod_dav_svn/reports/update.c
> >
> > --- Changelist 'helpers':
> >        subversion/libsvn_ra_neon/props.c
> >
> > --- Changelist 'cap':
> >        subversion/libsvn_ra_neon/options.c
> > 0:% $svn di subversion/libsvn_ra_neon
> > Index: subversion/libsvn_ra_neon/fetch.c
> > ===================================================================
> > --- subversion/libsvn_ra_neon/fetch.c   (revision 981102)
> > +++ subversion/libsvn_ra_neon/fetch.c   (working copy)
> > @@ -1713,10 +1713,8 @@
> >       if (! rb->receiving_all)
> >         break;
> >
> > -      base_checksum = svn_xml_get_attr_value("base-checksum", atts);
> > -
> >       SVN_ERR((*rb->editor->apply_textdelta)(rb->file_baton,
> > -                                             base_checksum,
> > +                                             NULL, /* ### base_checksum */
> >                                              rb->file_pool,
> >                                              &(rb->whandler),
> >                                              &(rb->whandler_baton)));
> > 0:% ./tools/client-side/wc-format.sh
> > 18
> > 0:% ./subversion/svnversion/svnversion
> > 981102M
> > 0:%
> > ]]]
> >