Attempt to detect whether the directory about to be deleted by a merge is equal to the directory in the repository at merge-left source, and do not perform this deletion if it is not. * subversion/libsvn_client/merge.c (files_same_p): Change to an implementation more like we're going to use for directories - one that compares against the repository rather than a local temporary file - to help me get up to speed on how to do this. (merge_file_deleted): Use the new version of files_same_p(). (dirs_same_baton_t): New structure. (dirs_same_file_changed, etc.): New svn_wc_diff_callbacks3_t functions. Each will set a flag in the baton if any difference at all is indicated. (dirs_same_p): New function, to determine whether a WC dir is equal to a repos dir. (merge_dir_deleted): Use dirs_same_p() to decide whether to delete a dir. (single_file_merge_get_file): Add a new parameter for the relative URL of the file to get. (do_file_merge): Adjust the call to (single_file_merge_get_file) accordingly. * subversion/tests/cmdline/merge_tests.py (svn_moddir): New function. (del_differing_file): Cosmetic change only. (del_identical_dir, del_sched_add_hist_dir, del_differing_dir): New test functions. (test_list): Add the new tests. Index: subversion/libsvn_client/merge.c =================================================================== --- subversion/libsvn_client/merge.c (revision 33305) +++ subversion/libsvn_client/merge.c (working copy) @@ -1385,33 +1385,59 @@ properties_same_p(svn_boolean_t *same, 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 its WC admin area ADM_ACCESS). Set - * *SAME to true if they are the same or false if they differ, ignoring - * the "svn:mergeinfo" property, and ignoring differences in keyword +static svn_error_t * +single_file_merge_get_file(const char **filename, + svn_ra_session_t *ra_session, + const char *rel_url, + apr_hash_t **props, + svn_revnum_t rev, + const char *wc_target, + apr_pool_t *pool); + +/* Compare the files OLDER (together with its normal properties in + * ORIGINAL_PROPS which may also contain WC props and entry props) which is + * in repository-normal form, and MINE (with its properties obtained from + * its WC admin area ADM_ACCESS) which is in working-copy form. + * Set *SAME to true if they are the same or false if they differ, ignoring + * the "svn:mergeinfo" property, and allowing for differences due to keyword * expansion and end-of-line style. */ +/* ### This implementation contacts the repository rather than using a + * provided copy of the remote file, and is modelled on the operation of + * svn_client_diff() (?). + */ static svn_error_t * files_same_p(svn_boolean_t *same, - const char *older, - apr_hash_t *original_props, + const char *older_url, + svn_revnum_t older_rev, + svn_ra_session_t *ra_session, const char *mine, svn_wc_adm_access_t *adm_access, apr_pool_t *pool) { + const char *session_url; + const char *rel_url; + const char *older_path; + apr_hash_t *older_props; apr_hash_t *working_props; + SVN_ERR(svn_ra_get_session_url(ra_session, &session_url, pool)); + rel_url = svn_path_is_child(session_url, older_url, pool); + SVN_ERR_ASSERT(rel_url); + SVN_ERR(single_file_merge_get_file(&older_path, ra_session, rel_url, + &older_props, older_rev, mine, + pool)); + SVN_ERR(svn_wc_prop_list(&working_props, mine, adm_access, pool)); /* Compare the properties */ - SVN_ERR(properties_same_p(same, original_props, working_props, pool)); + SVN_ERR(properties_same_p(same, older_props, working_props, pool)); if (*same) { svn_boolean_t modified; /* Compare the file content, translating 'mine' to 'normal' form. */ SVN_ERR(svn_wc__versioned_file_modcheck(&modified, mine, adm_access, - older, TRUE, pool)); + older_path, TRUE, pool)); *same = !modified; } @@ -1452,10 +1478,15 @@ merge_file_deleted(svn_wc_adm_access_t *adm_access case svn_node_file: { svn_boolean_t same; + const char *child = svn_path_is_child(merge_b->target, mine/*?*/, subpool); + const char *url1; + url1 = svn_path_url_add_component(merge_b->merge_source.url1, child, + subpool); + /* If the files are identical, attempt deletion */ - SVN_ERR(files_same_p(&same, older, original_props, mine, adm_access, - subpool)); + SVN_ERR(files_same_p(&same, url1, merge_b->merge_source.rev1, + merge_b->ra_session1, mine, adm_access, subpool)); if (same || merge_b->force || merge_b->record_only /* ### why? */) { /* Passing NULL for the notify_func and notify_baton because @@ -1673,8 +1704,278 @@ merge_dir_added(svn_wc_adm_access_t *adm_access, return SVN_NO_ERROR; } +/* ====================== Support for dirs_same_p ====================== */ + +typedef struct dirs_same_baton_t { + svn_boolean_t was_modified; + apr_pool_t *pool; +} dirs_same_baton_t; + /* An svn_wc_diff_callbacks3_t function. */ static svn_error_t * +dirs_same_file_changed(svn_wc_adm_access_t *adm_access, + svn_wc_notify_state_t *contentstate, + svn_wc_notify_state_t *propstate, + const char *path, + const char *tmpfile1, + const char *tmpfile2, + svn_revnum_t rev1, + svn_revnum_t rev2, + const char *mimetype1, + const char *mimetype2, + const apr_array_header_t *propchanges, + apr_hash_t *originalprops, + void *diff_baton) +{ + dirs_same_baton_t *baton = diff_baton; + + /* Set modified if a text change is reported. */ + if (tmpfile1 != NULL) + { + baton->was_modified = TRUE; + return SVN_NO_ERROR; + } + + /* File props: ignore entry-props and wc-props. */ + if (propchanges != NULL) + { + apr_array_header_t *props; + SVN_ERR(svn_categorize_props(propchanges, NULL, NULL, &props, + baton->pool)); + if (props->nelts > 0) + { + baton->was_modified = TRUE; + return SVN_NO_ERROR; + } + } + + return SVN_NO_ERROR; +} + +static svn_error_t * +dirs_same_file_added(svn_wc_adm_access_t *adm_access, + svn_wc_notify_state_t *contentstate, + svn_wc_notify_state_t *propstate, + const char *path, + const char *tmpfile1, + const char *tmpfile2, + svn_revnum_t rev1, + svn_revnum_t rev2, + const char *mimetype1, + const char *mimetype2, + const apr_array_header_t *propchanges, + apr_hash_t *originalprops, + void *diff_baton) +{ + dirs_same_baton_t *baton = diff_baton; + + baton->was_modified = TRUE; + return SVN_NO_ERROR; +} + +static svn_error_t * +dirs_same_file_deleted(svn_wc_adm_access_t *adm_access, + svn_wc_notify_state_t *state, + const char *path, + const char *tmpfile1, + const char *tmpfile2, + const char *mimetype1, + const char *mimetype2, + apr_hash_t *originalprops, + void *diff_baton) +{ + dirs_same_baton_t *baton = diff_baton; + + baton->was_modified = TRUE; + return SVN_NO_ERROR; +} + +static svn_error_t * +dirs_same_dir_added(svn_wc_adm_access_t *adm_access, + svn_wc_notify_state_t *state, + const char *path, + svn_revnum_t rev, + void *diff_baton) +{ + dirs_same_baton_t *baton = diff_baton; + + baton->was_modified = TRUE; + return SVN_NO_ERROR; +} + +static svn_error_t * +dirs_same_dir_deleted(svn_wc_adm_access_t *adm_access, + svn_wc_notify_state_t *state, + const char *path, + void *diff_baton) +{ + dirs_same_baton_t *baton = diff_baton; + + baton->was_modified = TRUE; + return SVN_NO_ERROR; +} + +static svn_error_t * +dirs_same_dir_props_changed(svn_wc_adm_access_t *adm_access, + svn_wc_notify_state_t *propstate, + const char *path, + const apr_array_header_t *propchanges, + apr_hash_t *original_props, + void *diff_baton) +{ + dirs_same_baton_t *baton = diff_baton; + apr_array_header_t *props; + + /* Dir props: ignore entry-props and wc-props. */ + SVN_ERR(svn_categorize_props(propchanges, NULL, NULL, &props, baton->pool)); + if (props->nelts > 0) + baton->was_modified = TRUE; + + return SVN_NO_ERROR; +} + +static svn_error_t * +dirs_same_dir_opened(svn_wc_adm_access_t *adm_access, + const char *path, + svn_revnum_t rev, + void *diff_baton) +{ + return SVN_NO_ERROR; +} + +static svn_error_t * +dirs_same_dir_closed(svn_wc_adm_access_t *adm_access, + svn_wc_notify_state_t *state, + const char *path, + void *diff_baton) +{ + return SVN_NO_ERROR; +} + + +/* Compare the directory trees OLDER_URL@OLDER_REV and MINE + * (with its properties obtained from its WC admin area ADM_ACCESS). Set + * *SAME to true if they are the same or false if they differ, ignoring + * the "svn:mergeinfo" property, and allowing for differences due to keyword + * expansion and end-of-line style. + * Use *CTX for authentication if necessary. */ +static svn_error_t * +dirs_same_p(svn_boolean_t *same, + const merge_source_t *merge_source, + svn_ra_session_t *ra_session, + const char *mine, + svn_wc_adm_access_t *adm_access, + svn_client_ctx_t *ctx, + apr_pool_t *pool) +{ + /* Approach 1: */ + /* Do a WC-to-repos diff. (The following is adapted from + * libsvn_client/diff.c:diff_repos_wc().) */ + { + const char *anchor, *anchor_url, *target = ""; + const svn_wc_entry_t *entry; + svn_ra_session_t *ra_session2; + const svn_wc_diff_callbacks3_t callbacks = + { dirs_same_file_changed, dirs_same_file_added, dirs_same_file_deleted, + dirs_same_dir_added, dirs_same_dir_deleted, + dirs_same_dir_props_changed, dirs_same_dir_opened, dirs_same_dir_closed + }; + dirs_same_baton_t callback_baton; + const svn_delta_editor_t *diff_editor; + void *diff_edit_baton; + const svn_ra_reporter3_t *reporter; + void *report_baton; + svn_boolean_t server_supports_depth; + + anchor = svn_wc_adm_access_path(adm_access); + + /* Fetch the URL of the anchor directory. */ + SVN_ERR(svn_wc__entry_versioned(&entry, anchor, adm_access, FALSE, pool)); + if (! entry->url) + return svn_error_createf(SVN_ERR_ENTRY_MISSING_URL, NULL, + _("Directory '%s' has no URL"), + svn_path_local_style(anchor, pool)); + anchor_url = apr_pstrdup(pool, entry->url); + + /* Establish RA session to the anchor of MINE */ + SVN_ERR(svn_client__open_ra_session_internal(&ra_session2, anchor_url, + NULL, NULL, NULL, FALSE, TRUE, + ctx, pool)); + + callback_baton.was_modified = FALSE; + callback_baton.pool = pool; + SVN_ERR(svn_wc_get_diff_editor5(adm_access, target, + &callbacks, &callback_baton, + svn_depth_infinity, + TRUE /* ignore_ancestry */, + FALSE /* use_text_base */, + FALSE /* reverse */, + ctx->cancel_func, ctx->cancel_baton, + NULL /* changelists */, + &diff_editor, &diff_edit_baton, pool)); + + SVN_ERR(svn_ra_do_diff3(ra_session2, &reporter, &report_baton, + merge_source->rev1, + target ? svn_path_uri_decode(target, pool) : NULL, + svn_depth_infinity, TRUE /* ignore_ancestry */, + TRUE /* text_deltas */, + merge_source->url1, diff_editor, diff_edit_baton, + pool)); + + SVN_ERR(svn_ra_has_capability(ra_session2, &server_supports_depth, + SVN_RA_CAPABILITY_DEPTH, pool)); + + /* Create a txn mirror of MINE; the diff editor will print + diffs in reverse. :-) */ + SVN_ERR(svn_wc_crawl_revisions3(mine, adm_access, reporter, report_baton, + FALSE, svn_depth_infinity, (! server_supports_depth), + FALSE, NULL, NULL, /* notification is N/A */ + NULL, pool)); + + *same = !callback_baton.was_modified; + } + + /* Approach 2: */ + /* Read this directory from repos and compare it with WC. */ +#if 0 + + apr_hash_t *older_dirents; + apr_array_header_t *older_props; + apr_hash_t *working_props; + + /* Read directory from repos into older_dirents and older_props */ + { + const char *session_url; + const char *rel_url; + apr_hash_t *prop_hash; + apr_array_header_t *prop_array; + + SVN_ERR(svn_ra_get_session_url(ra_session, &session_url, pool)); + rel_url = svn_path_is_child(session_url, older_url, pool); + SVN_ERR_ASSERT(rel_url); + + SVN_ERR(svn_ra_get_dir2(ra_session, &older_dirents, NULL, &prop_hash, + rel_url, older_rev, SVN_DIRENT_ALL, pool)); + /* Purge non-regular props such as entry-props. */ + prop_array = svn_prop_hash_to_array(prop_hash, pool); + SVN_ERR(svn_categorize_props(prop_array, NULL, NULL, &prop_array, pool)); + } + + /* Compare the props with WC's props */ + SVN_ERR(svn_wc_prop_list(&working_props, mine, adm_access, pool)); + SVN_ERR(properties_same_p(same, older_props, working_props, pool)); + + /* ### TODO: Compare dir entries with WC's dir entries */ + + /* ### TODO: Recurse */ + +#endif + + return SVN_NO_ERROR; +} + +/* An svn_wc_diff_callbacks3_t function. */ +static svn_error_t * merge_dir_deleted(svn_wc_adm_access_t *adm_access, svn_wc_notify_state_t *state, const char *path, @@ -1684,9 +1985,6 @@ merge_dir_deleted(svn_wc_adm_access_t *adm_access, apr_pool_t *subpool = svn_pool_create(merge_b->pool); svn_node_kind_t kind; const svn_wc_entry_t *entry; - svn_wc_adm_access_t *parent_access; - const char *parent_path; - svn_error_t *err; /* Easy out: if we have no adm_access for the parent directory, then this portion of the tree-delta "patch" must be inapplicable. @@ -1711,32 +2009,50 @@ merge_dir_deleted(svn_wc_adm_access_t *adm_access, { if (entry && (entry->schedule != svn_wc_schedule_delete)) { - /* ### TODO: Before deleting, we should ensure that this dir - tree is equal to the one we're being asked to delete. - If not, mark this directory as a tree conflict victim, - because this could be use case 5 as described in - notes/tree-conflicts/detection.txt. - */ + /* If the dirs are identical, attempt deletion */ - svn_path_split(path, &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(path, parent_access, merge_b->force, - merge_b->dry_run, FALSE, - NULL, NULL, - merge_b->ctx, subpool); - if (err) + svn_boolean_t same; + const char *child = svn_path_is_child(merge_b->target, path, subpool); + merge_source_t child_source; + + child_source.url1 = svn_path_url_add_component( + merge_b->merge_source.url1, child, subpool); + child_source.url2 = svn_path_url_add_component( + merge_b->merge_source.url2, child, subpool); + + SVN_ERR(dirs_same_p(&same, &merge_b->merge_source, + merge_b->ra_session1, + path, adm_access, merge_b->ctx, subpool)); + if (same || merge_b->force) { + /* Before deleting, ensure that this dir + tree is equal to the one we're being asked to delete. + If not, mark this directory as a tree conflict victim, + because this could be use case 5 as described in + notes/tree-conflicts/detection.txt. + */ + + svn_wc_adm_access_t *parent_access; + const char *parent_path; + + svn_path_split(path, &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. */ + SVN_ERR(svn_client__wc_delete(path, parent_access, + merge_b->force, + merge_b->dry_run, FALSE, + NULL, NULL, + merge_b->ctx, subpool)); if (state) - *state = svn_wc_notify_state_obstructed; - svn_error_clear(err); + *state = svn_wc_notify_state_changed; } else { + /* The dirs differ, so skip instead of deleting */ if (state) - *state = svn_wc_notify_state_changed; + *state = svn_wc_notify_state_obstructed; } } else @@ -3817,7 +4133,7 @@ mark_mergeinfo_as_inheritable_for_a_range( Set *FILENAME to the local path to a new temporary file holding its text, and set *PROPS to a new hash of its properties. - RA_SESSION is a session whose current root is the URL of the file itself, + The file to get is at REL_URL relative to the current root of RA_SESSION, and REV is the revision to get. The new temporary file will be created as a sibling of WC_TARGET. @@ -3833,6 +4149,7 @@ mark_mergeinfo_as_inheritable_for_a_range( static svn_error_t * single_file_merge_get_file(const char **filename, svn_ra_session_t *ra_session, + const char *rel_url, apr_hash_t **props, svn_revnum_t rev, const char *wc_target, @@ -3845,7 +4162,7 @@ single_file_merge_get_file(const char **filename, wc_target, ".tmp", svn_io_file_del_none, pool)); stream = svn_stream_from_aprfile2(fp, FALSE, pool); - SVN_ERR(svn_ra_get_file(ra_session, "", rev, + SVN_ERR(svn_ra_get_file(ra_session, rel_url, rev, stream, NULL, props, pool)); return svn_stream_close(stream); } @@ -5225,10 +5542,10 @@ do_file_merge(const char *url1, /* While we currently don't allow it, in theory we could be fetching two fulltexts from two different repositories here. */ - SVN_ERR(single_file_merge_get_file(&tmpfile1, ra_session1, + SVN_ERR(single_file_merge_get_file(&tmpfile1, ra_session1, "", &props1, r->start, target_wcpath, subpool)); - SVN_ERR(single_file_merge_get_file(&tmpfile2, ra_session2, + SVN_ERR(single_file_merge_get_file(&tmpfile2, ra_session2, "", &props2, r->end, target_wcpath, subpool)); Index: subversion/tests/cmdline/merge_tests.py =================================================================== --- subversion/tests/cmdline/merge_tests.py (revision 33305) +++ subversion/tests/cmdline/merge_tests.py (working copy) @@ -12092,6 +12092,13 @@ def svn_modfile(path): svntest.actions.run_and_verify_svn(None, None, [], 'propset', 'newprop', 'v', path) +def svn_moddir(path): + "Make content and property mods to a WC dir." + svn_mkfile(path+"/newfile") + path = local_path(path) + svntest.actions.run_and_verify_svn(None, None, [], 'propset', + 'newprop', 'v', path) + def svn_copy(s_rev, path1, path2): "Copy a WC path locally." path1 = local_path(path1) @@ -12210,10 +12217,9 @@ def del_differing_file(sbox): saved_cwd = os.getcwd() os.chdir(sbox.wc_dir) + # Set up deletions in the source. source = 'A/D/G' - s_rev_orig = 1 - - # Delete files in the source + s_rev_orig = svn_commit.repo_rev svn_delete(source+"/tau") s_rev_tau = svn_commit(source) svn_delete(source+"/pi") @@ -14057,6 +14063,111 @@ def subtree_gets_changes_even_if_ultimately_delete expected_status, expected_skip, None, None, None, None, None, 1) + +def del_identical_dir(sbox): + "merge tries to delete a dir of identical content" + + # Set up a standard greek tree in r1. + sbox.build() + svn_commit.repo_rev = 1 + + saved_cwd = os.getcwd() + os.chdir(sbox.wc_dir) + + # Set up a modification and deletion in the source branch. + source = 'A/B' + s_rev_orig = svn_commit.repo_rev + svn_moddir(source+"/E") + s_rev_mod = svn_commit(source) + svn_delete(source+"/E") + s_rev_del = svn_commit(source) + + # Make an identical copy, and merge a deletion to it. + target = 'A/B2' + svn_copy(s_rev_mod, source, target) + svn_commit(target) + # Should be deleted quietly. + svn_merge(s_rev_del, source, target, '--- Merging|D ') + + # Make a differing copy, locally modify it so it's the same, + # and merge a deletion to it. + target = 'A/B3' + svn_copy(s_rev_orig, source, target) + svn_commit(target) + svn_moddir(target+"/E") + svn_commit(target) + # Should be deleted quietly. + svn_merge(s_rev_del, source, target, '--- Merging|D ') + + os.chdir(saved_cwd) + +def del_sched_add_hist_dir(sbox): + "merge tries to delete identical sched-add dir" + + # Setup a standard greek tree in r1. + sbox.build() + svn_commit.repo_rev = 1 + + saved_cwd = os.getcwd() + os.chdir(sbox.wc_dir) + + source = 'A/B' + s_rev_orig = svn_commit.repo_rev + + # Merge a creation, and delete by reverse-merging into uncommitted WC. + target = 'A/B2' + svn_copy(s_rev_orig, source, target) + s_rev = svn_commit('.') + svn_mkfile(source+"/dir") + s_rev = svn_commit('.') + svn_merge(s_rev, source, target, '--- Merging|A ') + # Should be deleted quietly. + svn_merge(-s_rev, source, target, '--- Reverse-merging|D ') + + os.chdir(saved_cwd) + +def del_differing_dir(sbox): + "merge tries to delete a dir 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 = 'A/B' + s_rev_orig = svn_commit.repo_rev + + # Delete dirs in the source: one with content change, one with prop change + svn_delete(source+"/E") # will have content change + s_rev_e = svn_commit(source) + svn_delete(source+"/F") # will have prop change + s_rev_f = svn_commit(source) + + # Copy a dir, modify it, and merge a deletion to it. + target = 'A/B2' + svn_copy(s_rev_orig, source, target) + svn_mkfile(target+"/E/newfile") + svntest.actions.run_and_verify_svn(None, None, [], 'propset', + 'newprop', 'v', target+"/F") + svn_merge(s_rev_e, source, target, 'Skipped.*E') + svn_merge(s_rev_f, source, target, 'Skipped.*F') + + # Copy a dir, modify it, commit, and merge a deletion to it. + target = 'A/B3' + svn_copy(s_rev_orig, source, target) + svn_mkfile(target+"/E/newfile") + svntest.actions.run_and_verify_svn(None, None, [], 'propset', + 'newprop', 'v', target+"/F") + svn_commit(target) + # Should complain and "skip" it. + svn_merge(s_rev_e, source, target, 'Skipped.*E') + svn_merge(s_rev_f, source, target, 'Skipped.*F') + + os.chdir(saved_cwd) + + ######################################################################## # Run the tests @@ -14258,6 +14369,9 @@ test_list = [ None, tree_conflicts_on_merge_no_local_ci_6, XFail(SkipUnless(subtree_gets_changes_even_if_ultimately_deleted, server_has_mergeinfo)), + del_identical_dir, + del_sched_add_hist_dir, + XFail(del_differing_dir), ] if __name__ == '__main__':