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)
SVN_ERR(filter_self_referential_mergeinfo(&props,
local_abspath,
merge_b->ra_session2,
merge_b->ctx,
result_pool));
}
}
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)
/trunk:1-999=======
>>>>>>> (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