When a merge deletes a file, check whether the file content is the same, delete it if so, and skip it if not. This improves the decision whether to skip, which was previously based only on whether the file had uncommitted modifications. The comparison is not yet implemented for directories. This changes the meaning of the "force" flag to all of the svn_client "merge" APIs. ### Incomplete. See '###'. merge_tests.py 77 is failing. * subversion/libsvn_client/merge.c (files_contents_same_p, properties_same_p, files_same_p): New functions. (merge_file_deleted): Delete the file if it's identical, otherwise skip it. (do_file_merge): Fix the call to merge_file_deleted() so it can see the file being deleted. This is required by its API although it didn't previously care. * subversion/tests/cmdline/merge_tests.py (svn_mkdir, svn_mkfile, svn_copy, svn_delete, svn_commit, svn_move): New helper functions. (del_identical_file, del_sched_add_hist_file, del_differing_file): New test functions. (test_list): Add the new tests. * subversion/include/svn_client.h (svn_client_merge3): Update documentation for the "force" parameter accordingly. Index: subversion/include/svn_client.h =================================================================== --- subversion/include/svn_client.h (revision 32373) +++ subversion/include/svn_client.h (working copy) @@ -2492,10 +2492,17 @@ svn_client_diff_summarize_peg(const char * and the addition of another, but if this flag is TRUE, unrelated * items will be diffed as if they were related. * + * ### OLD: * If @a force is not set and the merge involves deleting locally modified or * unversioned items the operation will fail. If @a force is set such items * will be deleted. * + * ### NEW: + * If @a force is false and the merge involves deleting a file whose + * content differs from the source-left version, or a locally modified + * directory, or an unversioned item, then the operation will fail. If + * @a force is true then all such items will be deleted. + * * @a merge_options (an array of const char *), if non-NULL, * is used to pass additional command line arguments to the merge * processes (internal or external). @see Index: subversion/libsvn_client/merge.c =================================================================== --- subversion/libsvn_client/merge.c (revision 32373) +++ subversion/libsvn_client/merge.c (working copy) @@ -49,6 +49,7 @@ #include "private/svn_wc_private.h" #include "private/svn_mergeinfo_private.h" +#include "private/svn_debug_private.h" #include "svn_private_config.h" @@ -974,6 +975,84 @@ merge_file_added(svn_wc_adm_access_t *ad return SVN_NO_ERROR; } +/* Compare the contents of the files OLDER and MINE. Set *SAME to true if + * they are identical, false otherwise. + * ### This is presently a raw comparison: no translation of keywords or EOL style. + */ +static svn_error_t * +files_contents_same_p(svn_boolean_t *same, + const char *older, + const char *mine, + apr_pool_t *pool) +{ + SVN_ERR_ASSERT(older); + SVN_ERR_ASSERT(mine); + + SVN_ERR(svn_io_files_contents_same_p(same, older, mine, pool)); + + return SVN_NO_ERROR; +} + +/* Compare the two sets of properties PROPS1 and PROPS2, ignoring the + * "svn:mergeinfo" property, and noticing only "normal" props. Set *SAME to + * true if the rest of the properties are identical or false if they differ. + */ +static svn_error_t * +properties_same_p(svn_boolean_t *same, + apr_hash_t *props1, + apr_hash_t *props2, + apr_pool_t *pool) +{ + apr_array_header_t *prop_changes; + int i, diffs; + + /* Examine the properties that differ */ + SVN_ERR(svn_prop_diffs(&prop_changes, props1, props2, pool)); + diffs = 0; + for (i = 0; i < prop_changes->nelts; i++) + { + const char *pname = APR_ARRAY_IDX(prop_changes, i, svn_prop_t).name; + + /* Count the properties we're interested in; ignore the rest */ + if (svn_wc_is_normal_prop(pname) + && strcmp(pname, SVN_PROP_MERGEINFO) != 0) + diffs++; + } + *same = (diffs == 0); + return SVN_NO_ERROR; +} + +/* Compare the file OLDER (together with its normal properties in + * ORIGINAL_PROPS which may also contain WC props and entry props) and MINE + * (with its properties obtained from the WC admin area ADM_ACCESS). Set + * *SAME to true if they are the same or false if they differ, ignoring + * the "svn:mergeinfo" property. */ +/* ### Need to account for keywords and EOL style. */ +static svn_error_t * +files_same_p(svn_boolean_t *same, + const char *older, + apr_hash_t *original_props, + const char *mine, + svn_wc_adm_access_t *adm_access, + apr_pool_t *pool) +{ + apr_hash_t *working_props; + svn_boolean_t same_text, same_props; + + /* ### Optimisations: Could maybe use WC entry to quickly determine it's + * identical in easy cases e.g. where base-rev(mine)==rev(older) and not + * locally modified. Could avoid second comparison if first one fails + * ("short-circuit boolean evaluation"). */ + + SVN_ERR(svn_wc_prop_list(&working_props, mine, adm_access, pool)); + + SVN_ERR(files_contents_same_p(&same_text, older, mine, pool)); + SVN_ERR(properties_same_p(&same_props, original_props, working_props, pool)); + + *same = (same_text && same_props); + return SVN_NO_ERROR; +} + /* An svn_wc_diff_callbacks3_t function. */ static svn_error_t * merge_file_deleted(svn_wc_adm_access_t *adm_access, @@ -1005,28 +1084,52 @@ merge_file_deleted(svn_wc_adm_access_t * return SVN_NO_ERROR; } + dbg_printf("## merge_file_deleted(mine ='%s'\n" + " older ='%s'\n" + " yours ='%s'\n" + " target='%s'\n" + " url ='%s')\n", + mine, older, yours, merge_b->target, merge_b->url); + SVN_ERR(svn_io_check_path(mine, &kind, subpool)); switch (kind) { case svn_node_file: - svn_path_split(mine, &parent_path, NULL, subpool); - SVN_ERR(svn_wc_adm_retrieve(&parent_access, adm_access, parent_path, - subpool)); - /* Passing NULL for the notify_func and notify_baton because - repos_diff.c:delete_entry() will do it for us. */ - err = svn_client__wc_delete(mine, parent_access, merge_b->force, - merge_b->dry_run, FALSE, NULL, NULL, - merge_b->ctx, subpool); - if (err) - { - if (state) - *state = svn_wc_notify_state_obstructed; - svn_error_clear(err); - } - else if (state) - { - *state = svn_wc_notify_state_changed; - } + { + svn_boolean_t same; + + /* If the files are identical, attempt deletion */ + SVN_ERR(files_same_p(&same, older, original_props, mine, adm_access, + subpool)); + if (same || merge_b->force) + { + svn_path_split(mine, &parent_path, NULL, subpool); + SVN_ERR(svn_wc_adm_retrieve(&parent_access, adm_access, parent_path, + subpool)); + + /* Passing NULL for the notify_func and notify_baton because + repos_diff.c:delete_entry() will do it for us. */ + err = svn_client__wc_delete(mine, parent_access, TRUE, + merge_b->dry_run, FALSE, NULL, NULL, + merge_b->ctx, subpool); + if (err) + { + if (state) + *state = svn_wc_notify_state_obstructed; + svn_error_clear(err); + } + else if (state) + { + *state = svn_wc_notify_state_changed; + } + } + else + { + /* The files differ, so skip instead of deleting */ + if (state) + *state = svn_wc_notify_state_obstructed; + } + } break; case svn_node_dir: if (state) @@ -4662,8 +4766,8 @@ do_file_merge(const char *url1, SVN_ERR(merge_file_deleted(adm_access, &text_state, target_wcpath, - NULL, - NULL, + tmpfile1, + tmpfile2, mimetype1, mimetype2, props1, merge_b)); Index: subversion/tests/cmdline/merge_tests.py =================================================================== --- subversion/tests/cmdline/merge_tests.py (revision 32373) +++ subversion/tests/cmdline/merge_tests.py (working copy) @@ -12617,6 +12617,147 @@ def commit_to_subtree_added_by_merge(sbo svntest.actions.run_and_verify_svn(None, ["At revision 5.\n"], [], 'up', wc_dir) + +def svn_mkdir(path): + "Make and add a directory with some default props." + svntest.actions.run_and_verify_svn(None, None, [], 'mkdir', '--parents', path) + +def svn_mkfile(path): + "Make and add a file with some default content." + dirname, filename = os.path.split(path) + svntest.main.file_write(path, "This is the file '" + filename + "'.\n" + + "Original second line.\n") + svntest.actions.run_and_verify_svn(None, None, [], 'add', path) + svntest.actions.run_and_verify_svn(None, None, [], 'propset', + 'p1', 'v1', path) + #expected_output = wc.State(wc_dir, + # {path : Item(verb='Adding')}) + #wc_status.add({path : Item(status=' ', wc_rev=3)}) + #svntest.actions.run_and_verify_commit(wc_dir, expected_output, + # wc_status, None, wc_dir) + +def svn_copy(path1, path2): + "Copy a WC path locally." + svntest.actions.run_and_verify_svn(None, None, [], 'copy', '--parents', path1, path2) + +def svn_delete(path): + "Delete a WC path locally." + svntest.actions.run_and_verify_svn(None, None, [], 'delete', path) + +def svn_commit(path): + "Commit a WC path and return the new revision number." + svntest.actions.run_and_verify_svn(None, svntest.verify.AnyOutput, [], + 'commit', '-m', '', path) + svn_commit.repo_rev += 1 + print "## repo_rev is now", svn_commit.repo_rev + return svn_commit.repo_rev + +def svn_merge(src_change_num, source, target, exp_out=svntest.verify.AnyOutput): + "Merge a single change from path 'source' to path 'target'" + svntest.actions.run_and_verify_svn(None, exp_out, [], + 'merge', '-c', src_change_num, + source, target) + +def del_identical_file(sbox): + "merge tries to delete a file of identical content" + + # Setup a standard greek tree in r1. + sbox.build() + svn_commit.repo_rev = 1 + + saved_cwd = os.getcwd() + os.chdir(sbox.wc_dir) + + source_d = 'source' + target_d = 'target' + source_file = os.path.join(source_d, 'file') + target_file = os.path.join(target_d, 'file') + + # Make an identical copy, and merge a deletion to it. + svn_mkdir(source_d) + svn_mkfile(source_file) + s_rev = svn_commit('.') + svn_copy(source_file, target_file) + svn_delete(source_file) + s_rev = svn_commit('.') + svn_merge(s_rev, source_d, target_d, '--- Merging|D ') + # should be deleted quietly + +def del_sched_add_hist_file(sbox): + "merge tries to delete identical sched-add file" + + # Setup a standard greek tree in r1. + sbox.build() + svn_commit.repo_rev = 1 + + saved_cwd = os.getcwd() + os.chdir(sbox.wc_dir) + + source_d = 'source' + target_d = 'target' + source_file = os.path.join(source_d, 'file') + target_file = os.path.join(target_d, 'file') + + # Merge a creation, and delete by reverse-merging into uncommitted WC. + svn_mkdir(source_d) + svn_copy(source_d, target_d) + s_rev = svn_commit('.') + svn_mkfile(source_file) + s_rev = svn_commit('.') + svn_merge(s_rev, source_d, target_d, '--- Merging|A ') + svn_merge(-s_rev, source_d, target_d, '--- Reverse-merging|D ') + # should be deleted quietly + + os.chdir(saved_cwd) + +def del_differing_file(sbox): + "merge tries to delete a file of different content" + + # Setup a standard greek tree in r1. + sbox.build() + svn_commit.repo_rev = 1 + + saved_cwd = os.getcwd() + os.chdir(sbox.wc_dir) + + source_d = 'source' + target_d = 'target' + source_file = os.path.join(source_d, 'file') + target_file = os.path.join(target_d, 'file') + + # Copy a file, text-modify it, and merge a deletion to it. + svn_mkdir(source_d) + svn_mkfile(source_file) + s_rev = svn_commit('.') + svn_copy(source_file, target_file) + svntest.main.file_append(target_file, "An extra line in the target.\n") + svn_delete(source_file) + s_rev = svn_commit(source_d) + svn_merge(s_rev, source_d, target_d, 'Skipped') + # should complain and "skip" it + + source_d = 'source2' + target_d = 'target2' + source_file = os.path.join(source_d, 'file') + target_file = os.path.join(target_d, 'file') + + # Copy a file, text-modify it, commit, and merge a deletion to it. + svn_mkdir(source_d) + svn_mkfile(source_file) + s_rev = svn_commit('.') + svn_copy(source_file, target_file) + svntest.main.file_append(target_file, "An extra line in the target.\n") + svn_delete(source_file) + s_rev = svn_commit('.') + svn_merge(s_rev, source_d, target_d, 'Skipped') + # should complain and "skip" it + + # Make a prop-modified copy, and merge a deletion to it. + # ... + + os.chdir(saved_cwd) + + ######################################################################## # Run the tests @@ -12791,6 +12932,9 @@ test_list = [ None, SkipUnless(subtrees_with_empty_mergeinfo, server_has_mergeinfo), SkipUnless(commit_to_subtree_added_by_merge, svntest.main.is_ra_type_dav), + del_identical_file, + del_sched_add_hist_file, + del_differing_file, ] if __name__ == '__main__':