[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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

From: Paul Burba <ptburba_at_gmail.com>
Date: Mon, 2 Aug 2010 09:59:15 -0400

On Sat, Jul 31, 2010 at 7:13 PM, Daniel Shahaf <d.s_at_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:%
> ]]]
>
Received on 2010-08-02 15:59:57 CEST

This is an archived mail posted to the Subversion Dev mailing list.