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

The purpose of prepare_merge_props_changed()

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Fri, 4 Jan 2013 17:47:00 +0000 (GMT)

I'm trying to understand prepare_merge_props_changed().  I don't get why the stripping
of self-referential and foreign-repos mergeingo is *not* performed when
doing a reverse merge, as I highlight below.  Paul, can you shed any light on it?

Index: subversion/libsvn_client/merge.c

 /* Prepare a set of property changes PROPCHANGES to be used for a merge
-   operation on LOCAL_ABSPATH. Store the result in *PROP_UPDATES.
+   operation on LOCAL_ABSPATH.

+   Remove all non-regular prop-changes (entry-props and WC-props).
+   Remove all non-mergeinfo prop-changes if it's a record-only merge.
+   Remove self-referential mergeinfo (### in some cases...)
+   Remove foreign-repository mergeinfo (### in some cases...)
+   Store the filtered property changes in *PROP_UPDATES.
    Store information on where mergeinfo is updated in MERGE_B.

    Used for both file and directory property merges. */
 static svn_error_t *
 prepare_merge_props_changed(const apr_array_header_t **prop_updates,
                             const char *local_abspath,
                             const apr_array_header_t *propchanges,
                             merge_cmd_baton_t *merge_b,
                             apr_pool_t *result_pool,
                             apr_pool_t *scratch_pool)
   apr_array_header_t *props;


   if (props->nelts)
       /* If this is a forward merge then don't add new mergeinfo to
          PATH that is already part of PATH's own history, see
http://svn.haxx.se/dev/archive-2008-09/0006.shtml.  If the
          merge sources are not ancestral then there is no concept of a
          'forward' or 'reverse' merge and we filter unconditionally. */
+      /*
+       * ### Whereas, if this is a reverse merge, this implies it's OK
+       *    to add new mergeinfo that is already part of PATH's own
+       *    history (and OK to add foreign-repository mergeinfo) ???
+       *
+       *    This doesn't sound right.  Why are these two kinds of
+       *    filtering conditional at all?
+       */
       if (merge_b->merge_source.loc1->rev < merge_b->merge_source.loc2->rev
           || !merge_b->merge_source.ancestral)
           /* Issue #3383: We don't want mergeinfo from a foreign repos.

              If this is a merge from a foreign repository we must strip all
              incoming mergeinfo (including mergeinfo deletions).  Otherwise if
              this property isn't mergeinfo or is NULL valued (i.e. prop removal)
              or empty mergeinfo it does not require any special handling.  There
              is nothing to filter out of empty mergeinfo and the concept of
              filtering doesn't apply if we are trying to remove mergeinfo
              entirely.  */
           if (! merge_b->same_repos)
             SVN_ERR(omit_mergeinfo_changes(&props, props, result_pool));
           else if (HONOR_MERGEINFO(merge_b) || merge_b->reintegrate_merge)

It looks like if I were to try a foreign-repos reverse merge, where the change in the foreign repos included a mergeinfo change, that mergeinfo change would (wrongly) propagate into the target repos.
The following snippet isn't a full repro recipe but shows that a foreign mergeinfo change is indeed being applied in this case:

$ svn merge -c-5 file:///home/julianfoad/tmp/svn/frm/repo/branches/A foo/
--- Reverse-merging (from foreign repository) r5 into 'foo':
 U   foo/A
 C   foo
Summary of conflicts:
  Property conflicts: 1
Conflict for property 'svn:mergeinfo' discovered on 'foo'.
Trying to delete property 'svn:mergeinfo'
but the local property value is different.
<<<<<<< (local property value)
>>>>>>> (incoming property value)

The discussion in <http://subversion.tigris.org/issues/show_bug.cgi?id=3383> doesn't say anything about the filtering of foreign-repos mergeinfo being conditional.

I haven't investigated/tested the self-ref filtering part of this.

- Julian
Received on 2013-01-04 18:47:35 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.