Hi,
Attached is a (work-in-progress) patch to fix issue 3686[1].
For those unfamiliar with the issue: when a binary file (with svn:executable
set) is modified as a result of a merge, the executable flag is missing
post-merge. A commit will restore the flag.
I've looked into the code, and have found the problem. Essentially, in
libsvn_client/merge.c, there is a "Special case" merge for binary files.
Essentially, if the WORKING version is equal to the LEFT version, then we
directly copy the RIGHT over the top. As a result, the file never gets
included in the workqueue, and the sync_file_flags task is never run.
I believe the solution is to move this logic from libsvn_client to
libsvn_wc, and specifically the merge_binary_file function. This has further
effects as well. This function would always raise a conflict for binary
files, whereas now it will attempt to merge (using the above special case
logic), and if this isn't possible, then raise the conflict.
I'm submitting this patch as a "am I on the right track?" submission, rather
than a final job. I'll craft a log message if I'm correct.
Cheers,
Daniel B.
Index: subversion/tests/cmdline/merge_tests.py
===================================================================
--- subversion/tests/cmdline/merge_tests.py (revision 1065385)
+++ subversion/tests/cmdline/merge_tests.py (working copy)
@@ -16593,8 +16593,8 @@
merge_with_os_deleted_subtrees,
no_self_referential_or_nonexistent_inherited_mergeinfo,
XFail(subtree_merges_inherit_invalid_working_mergeinfo),
- XFail(SkipUnless(merge_change_to_file_with_executable,
- svntest.main.is_posix_os)),
+ SkipUnless(merge_change_to_file_with_executable,
+ svntest.main.is_posix_os),
]
if __name__ == '__main__':
Index: subversion/libsvn_wc/merge.c
===================================================================
--- subversion/libsvn_wc/merge.c (revision 1065385)
+++ subversion/libsvn_wc/merge.c (working copy)
@@ -1094,6 +1094,7 @@
const char *left_label,
const char *right_label,
const char *target_label,
+ svn_boolean_t dry_run,
const svn_wc_conflict_version_t *left_version,
const svn_wc_conflict_version_t *right_version,
const char *detranslated_target_abspath,
@@ -1111,13 +1112,30 @@
const char *merge_dirpath, *merge_filename;
const char *conflict_wrk;
svn_skel_t *work_item;
+ svn_boolean_t same_contents = FALSE;
SVN_ERR_ASSERT(svn_dirent_is_absolute(target_abspath));
*work_items = NULL;
svn_dirent_split(&merge_dirpath, &merge_filename, target_abspath, pool);
+
+ /* Attempt to merge the binary file. If this cannot be done, then
+ raise the conflict. */
+ SVN_ERR(svn_io_files_contents_same_p(&same_contents,
+ left_abspath,
+ target_abspath,
+ scratch_pool));
+ if (same_contents)
+ {
+ if (!dry_run)
+ SVN_ERR(svn_io_file_move(right_abspath, target_abspath,
+ scratch_pool));
+ *merge_outcome = svn_wc_merge_merged;
+ return SVN_NO_ERROR;
+ }
+
/* Give the conflict resolution callback a chance to clean
up the conflict before we mark the file 'conflicted' */
if (conflict_func)
@@ -1357,26 +1375,23 @@
if (is_binary)
{
- if (dry_run)
- /* in dry-run mode, binary files always conflict */
- *merge_outcome = svn_wc_merge_conflict;
- else
- SVN_ERR(merge_binary_file(work_items,
- merge_outcome,
- db,
- left_abspath,
- right_abspath,
- target_abspath,
- left_label,
- right_label,
- target_label,
- left_version,
- right_version,
- detranslated_target_abspath,
- mimeprop,
- conflict_func,
- conflict_baton,
- result_pool, scratch_pool));
+ SVN_ERR(merge_binary_file(work_items,
+ merge_outcome,
+ db,
+ left_abspath,
+ right_abspath,
+ target_abspath,
+ left_label,
+ right_label,
+ target_label,
+ dry_run,
+ left_version,
+ right_version,
+ detranslated_target_abspath,
+ mimeprop,
+ conflict_func,
+ conflict_baton,
+ result_pool, scratch_pool));
}
else
{
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 1065385)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -1452,40 +1452,6 @@
SVN_ERR(svn_wc_text_modified_p2(&has_local_mods, merge_b->ctx->wc_ctx,
mine_abspath, FALSE, subpool));
- /* Special case: if a binary file's working file is
- exactly identical to the 'left' side of the merge, then don't
- allow svn_wc_merge to produce a conflict. Instead, just
- overwrite the working file with the 'right' side of the
- merge. Why'd we check for local mods above? Because we want
- to do a different notification depending on whether or not
- the file was locally modified.
-
- Alternately, if the 'left' side of the merge doesn't exist in
- the repository, and the 'right' side of the merge is
- identical to the WC, pretend we did the merge (a no-op). */
- if ((mimetype1 && svn_mime_type_is_binary(mimetype1))
- || (mimetype2 && svn_mime_type_is_binary(mimetype2)))
- {
- /* For adds, the 'left' side of the merge doesn't exist. */
- svn_boolean_t older_revision_exists =
- !merge_b->add_necessitated_merge;
- svn_boolean_t same_contents;
- SVN_ERR(svn_io_files_contents_same_p(&same_contents,
- (older_revision_exists ?
- older_abspath : yours_abspath),
- mine_abspath, subpool));
- if (same_contents)
- {
- if (older_revision_exists && !merge_b->dry_run)
- {
- SVN_ERR(svn_io_file_move(yours_abspath, mine_abspath,
- subpool));
- }
- merge_outcome = svn_wc_merge_merged;
- merge_required = FALSE;
- }
- }
-
if (merge_required)
{
/* xgettext: the '.working', '.merge-left.r%ld' and
