Hi, Attached is a completed patch to resolve issue 3686[1], where the executable bit is not maintained during the merge of a binary file.
I thought about making this change more generic, and applying it to text files as well (there was discussion with performance of simple-case merging a month ago on users@), but thought I'd leave that for later. I didn't want to worry about (potential) line-endings, keywords, etc. problems. [[[ Fix issue #3686 - executable bit not set during merge. The cause was the special case in libsvn_client, which bypassed the use of the workqueue. This logic has now been moved into libsvn_wc. Additionally, this change allows the status of binary files (during a dry-run merge) to be reported correctly (previously, all binary files were reported as conflicted). * subversion/libsvn_client/merge.c (merge_file_changed): Removed binary-merge special case (now in libsvn_wc). Removed merge_required variable (resulting in indentation changes). * subversion/libsvn_wc/merge.c (merge_binary_file): Added dry_run parameter. Add the special case merging of binary files. * subversion/tests/cmdline/merge_tests.py (merge_change_to_file_with_executable): Remove @XFail decorator. ]]] [1] http://subversion.tigris.org/ds/viewMessage.do?dsForumId=463&dsMessageId=2635024 Cheers, Daniel B. --- Daniel Becroft
Index: subversion/tests/cmdline/merge_tests.py =================================================================== --- subversion/tests/cmdline/merge_tests.py (revision 1068136) +++ subversion/tests/cmdline/merge_tests.py (working copy) @@ -16469,7 +16469,6 @@ #---------------------------------------------------------------------- # Test for issue #3686 'executable flag not correctly set on merge' # See http://subversion.tigris.org/issues/show_bug.cgi?id=3686 -@XFail() @Issue(3686) @SkipUnless(svntest.main.is_posix_os) def merge_change_to_file_with_executable(sbox): Index: subversion/libsvn_wc/merge.c =================================================================== --- subversion/libsvn_wc/merge.c (revision 1068136) +++ 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,31 @@ 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. At the moment, we can only + handle the special case: if the LEFT side of the merge is equal + to WORKING, then we can copy RIGHT directly. */ + 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 +1376,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 1068136) +++ subversion/libsvn_client/merge.c (working copy) @@ -1300,7 +1300,6 @@ { merge_cmd_baton_t *merge_b = baton; apr_pool_t *subpool = svn_pool_create(merge_b->pool); - svn_boolean_t merge_required = TRUE; enum svn_wc_merge_outcome_t merge_outcome; SVN_ERR_ASSERT(mine_abspath && svn_dirent_is_absolute(mine_abspath)); @@ -1452,75 +1451,38 @@ 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. + /* xgettext: the '.working', '.merge-left.r%ld' and + '.merge-right.r%ld' strings are used to tag onto a file + name in case of a merge conflict */ + const char *target_label = _(".working"); + const char *left_label = apr_psprintf(subpool, + _(".merge-left.r%ld"), + older_rev); + const char *right_label = apr_psprintf(subpool, + _(".merge-right.r%ld"), + yours_rev); + conflict_resolver_baton_t conflict_baton = { 0 }; + const svn_wc_conflict_version_t *left; + const svn_wc_conflict_version_t *right; - 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; - } - } + conflict_baton.wrapped_func = merge_b->ctx->conflict_func; + conflict_baton.wrapped_baton = merge_b->ctx->conflict_baton; + conflict_baton.conflicted_paths = &merge_b->conflicted_paths; + conflict_baton.pool = merge_b->pool; - if (merge_required) - { - /* xgettext: the '.working', '.merge-left.r%ld' and - '.merge-right.r%ld' strings are used to tag onto a file - name in case of a merge conflict */ - const char *target_label = _(".working"); - const char *left_label = apr_psprintf(subpool, - _(".merge-left.r%ld"), - older_rev); - const char *right_label = apr_psprintf(subpool, - _(".merge-right.r%ld"), - yours_rev); - conflict_resolver_baton_t conflict_baton = { 0 }; - const svn_wc_conflict_version_t *left; - const svn_wc_conflict_version_t *right; + SVN_ERR(make_conflict_versions(&left, &right, mine_abspath, + svn_node_file, merge_b)); + SVN_ERR(svn_wc_merge4(&merge_outcome, merge_b->ctx->wc_ctx, + older_abspath, yours_abspath, mine_abspath, + left_label, right_label, target_label, + left, right, + merge_b->dry_run, merge_b->diff3_cmd, + merge_b->merge_options, prop_changes, + conflict_resolver, &conflict_baton, + merge_b->ctx->cancel_func, + merge_b->ctx->cancel_baton, + subpool)); - conflict_baton.wrapped_func = merge_b->ctx->conflict_func; - conflict_baton.wrapped_baton = merge_b->ctx->conflict_baton; - conflict_baton.conflicted_paths = &merge_b->conflicted_paths; - conflict_baton.pool = merge_b->pool; - - SVN_ERR(make_conflict_versions(&left, &right, mine_abspath, - svn_node_file, merge_b)); - SVN_ERR(svn_wc_merge4(&merge_outcome, merge_b->ctx->wc_ctx, - older_abspath, yours_abspath, mine_abspath, - left_label, right_label, target_label, - left, right, - merge_b->dry_run, merge_b->diff3_cmd, - merge_b->merge_options, prop_changes, - conflict_resolver, &conflict_baton, - merge_b->ctx->cancel_func, - merge_b->ctx->cancel_baton, - subpool)); - } - if (content_state) { if (merge_outcome == svn_wc_merge_conflict)
