Bert and Stefan,
The following patch also appears to fix the bug -- all tests pass.
However, see my further comments below.
[[[
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 1687145)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -2791,18 +2791,12 @@ merge_dir_opened(void **new_dir_baton,
SVN_ERR(svn_wc__node_is_added(&added, merge_b->ctx->wc_ctx,
local_abspath, scratch_pool));
db->tree_conflict_reason = added ? svn_wc_conflict_reason_added
:
svn_wc_conflict_reason_obstructed;
-
- if ((merge_b->merge_source.ancestral ||
merge_b->reintegrate_merge)
- && !(pdb && pdb->shadowed))
- {
- store_path(merge_b->skipped_abspaths, local_abspath);
- }
}
}
/* Handle pending conflicts */
SVN_ERR(mark_dir_edited(merge_b, db, local_abspath, scratch_pool));
@@ -5395,20 +5389,12 @@ record_skips_in_mergeinfo(const char *me
merge_b, skipped_abspath,
iterpool));
if (obstruction_state == svn_wc_notify_state_obstructed
|| obstruction_state == svn_wc_notify_state_missing)
continue;
- /* Make sure this is not a tree-conflicted path either. We don't
- * want to add mergeinfo on such nodes since it causes problems down
- * the line (see issue #4582, "reintegrate complains about missing
- * ranges from node unrelated to branch") */
- if (svn_hash_gets(merge_b->tree_conflicted_abspaths,
- skipped_abspath) != NULL)
- continue;
-
/* Add an empty range list for this path.
### TODO: This works fine for a file path skipped because it is
### missing as long as the file's parent directory is present.
### But missing directory paths skipped are not handled yet,
### see issue #2915.
]]]
Rather than adding code to ignore a 'skipped' path if it is also a
tree conflict path, this patch instead removes the code which recorded
an add-vs-add tree conflict victim as 'skipped'. That code block was
added recently (in http://svn.apache.org/r1666690) by Bert. Bert, can
you comment on it?
[[[
------------------------------------------------------------------------
r1666690 | rhuijben | 2015-03-14 15:30:17 +0100 (Sat, 14 Mar 2015) | 26 lines
In the libsvn_client merge code: When reporting and recording that we skip a
node because it is shadowed on an ancestor, properly record that the
ancestor is skipped instead of the descendant itself.
* subversion/libsvn_client/merge.c
(record_skip): Only record a skip, if we really skipped this node, and
not one of its ancestors. To do this handle this add a parent-baton
argument to this function.
(merge_file_changed,
merge_file_added,
merge_file_deleted): Update caller.
(merge_dir_opened): Not only skip the directory add, but also record it
as skipped.
(merge_dir_changed,
merge_dir_added,
merge_dir_deleted,
merge_node_absent): Update caller.
* subversion/tests/cmdline/merge_tree_conflict_tests.py
(merge_obstruction_recording): New function.
(test_list): Add merge_obstruction_recording.
Found by: Pete Harlan <pchpublic88{_AT_}gmail.com>
]]]
However, neither the current code nor the code resulting from this
patch looks entirely correct. Notice the first statement shown in the
patch context:
SVN_ERR(svn_wc__node_is_added(&added,...));
...reason = added ? ...reason_added : ...reason_obstructed;
That looks at the WC schedule to decide whether to call this an
'added' or an 'obstructed' conflict. For a merge, I think if the
directory exists in the working state, then we should report the
reason as 'added' regardless whether the dir was added in a commit on
the checked-out branch or locally added in the WC. And I'm not
entirely sure when we should call it 'obstructed' -- maybe only when
there is a local unversioned object on disk where the WC working state
expects there is nothing?
- Julian
