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 (dirs_same_baton_t): New structure. (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. * 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/tests/cmdline/merge_tests.py =================================================================== --- subversion/tests/cmdline/merge_tests.py (revision 33536) +++ subversion/tests/cmdline/merge_tests.py (working copy) @@ -12086,6 +12086,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) @@ -12204,10 +12211,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") @@ -14051,6 +14057,110 @@ 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") + # 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 @@ -14252,6 +14362,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__': Index: subversion/libsvn_client/merge.c =================================================================== --- subversion/libsvn_client/merge.c (revision 33536) +++ subversion/libsvn_client/merge.c (working copy) @@ -1682,6 +1682,94 @@ 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; +} dirs_same_baton_t; + +/* helpful for debugging +static char +kind_to_char(svn_client_diff_summarize_kind_t kind) +{ + switch (kind) + { + case svn_client_diff_summarize_kind_modified: + return 'M'; + + case svn_client_diff_summarize_kind_added: + return 'A'; + + case svn_client_diff_summarize_kind_deleted: + return 'D'; + + default: + return ' '; + } +} +*/ + +/* Collect change information, implements the + * svn_client_diff_summarize_func_t interface. */ +static svn_error_t * +dirs_same_summarize(const svn_client_diff_summarize_t *summary, + void *baton, + apr_pool_t *pool) +{ + dirs_same_baton_t *b = (dirs_same_baton_t*)baton; + + if (summary->summarize_kind != svn_client_diff_summarize_kind_normal || + summary->prop_changed){ + b->was_modified = TRUE; + /* helpful for debugging + printf("REASON: %c%c %s\n", kind_to_char(summary->summarize_kind), + summary->prop_changed ? 'M' : ' ', summary->path); + */ + } + + return SVN_NO_ERROR; +} + + +/* Compare the directory trees SOURCE_URL@SOURCE_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 char *source_url, + const svn_revnum_t source_rev, + const char *mine, + svn_ra_session_t *ra_session, + svn_wc_adm_access_t *adm_access, + svn_client_ctx_t *ctx, + apr_pool_t *pool) +{ + dirs_same_baton_t summarize_baton; + svn_opt_revision_t rev1, rev2; + + rev1.kind = svn_opt_revision_number; + rev1.value.number = source_rev; + + rev2.kind = svn_opt_revision_working; + + summarize_baton.was_modified = FALSE; + + SVN_ERR(svn_client_diff_summarize2(source_url, &rev1, + mine, &rev2, + svn_depth_infinity, + TRUE /* ignore ancestry */, + NULL /* no changelist */, + dirs_same_summarize, &summarize_baton, + ctx, pool)); + + *same = !summarize_baton.was_modified; + + 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, @@ -1693,9 +1781,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. @@ -1720,32 +1805,51 @@ 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, child_source.url1, + merge_b->merge_source.rev1, path, + merge_b->ra_session1, 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 Index: subversion/libsvn_client/diff.c =================================================================== --- subversion/libsvn_client/diff.c (revision 33536) +++ subversion/libsvn_client/diff.c (working copy) @@ -1452,6 +1452,119 @@ diff_summarize_repos_repos(const struct diff_param return reporter->finish_report(report_baton, pool); } +/* Perform a diff summary between a repository path and a working copy. */ +static svn_error_t * +diff_summarize_repos_wc(const struct diff_parameters *diff_param, + svn_boolean_t reverse, + svn_client_diff_summarize_func_t summarize_func, + void *summarize_baton, + svn_client_ctx_t *ctx, + apr_pool_t *pool) +{ + const char *url1, *anchor, *anchor_url, *target; + svn_wc_adm_access_t *adm_access, *dir_access; + const svn_wc_entry_t *entry; + svn_revnum_t rev; + svn_ra_session_t *ra_session; + const svn_ra_reporter3_t *reporter; + void *report_baton; + const svn_delta_editor_t *diff_editor; + void *diff_edit_baton; + int levels_to_lock = SVN_WC__LEVELS_TO_LOCK_FROM_DEPTH(diff_param->depth); + svn_boolean_t server_supports_depth; + + SVN_ERR_ASSERT(! svn_path_is_url(diff_param->path2)); + + /* Convert path1 to a URL to feed to do_diff. */ + SVN_ERR(convert_to_url(&url1, diff_param->path1, pool)); + + SVN_ERR(svn_wc_adm_open_anchor(&adm_access, &dir_access, &target, + diff_param->path2, FALSE, levels_to_lock, + ctx->cancel_func, ctx->cancel_baton, + pool)); + 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); + + /* If we are performing a pegged diff, we need to find out what our + actual URLs will be. */ + if (diff_param->peg_revision->kind != svn_opt_revision_unspecified) + { + svn_opt_revision_t *start_ignore, *end_ignore, end; + const char *url_ignore; + + end.kind = svn_opt_revision_unspecified; + + SVN_ERR(svn_client__repos_locations(&url1, &start_ignore, + &url_ignore, &end_ignore, + NULL, + diff_param->path1, + diff_param->peg_revision, + diff_param->revision1, &end, + ctx, pool)); + } + + /* Establish RA session to path2's anchor */ + SVN_ERR(svn_client__open_ra_session_internal(&ra_session, anchor_url, + NULL, NULL, NULL, FALSE, TRUE, + ctx, pool)); + + /* Get a revision number from the svn_opt_revision_t */ + SVN_ERR(svn_client__get_revision_number + (&rev, NULL, ra_session, diff_param->revision1, + (diff_param->path1 == url1) ? NULL : diff_param->path1, pool)); + + /* Set up the repos_diff editor. */ + SVN_ERR(svn_client__get_diff_summarize_editor + (target, summarize_func, + summarize_baton, ra_session, rev, + ctx->cancel_func, ctx->cancel_baton, + &diff_editor, &diff_edit_baton, pool)); + /* + SVN_ERR(svn_wc_get_diff_editor5(adm_access, target, + callbacks, callback_baton, + diff_param->depth, + diff_param->ignore_ancestry, + rev2_is_base, + reverse, + ctx->cancel_func, ctx->cancel_baton, + diff_param->changelists, + &diff_editor, &diff_edit_baton, + pool)); +*/ + /* Tell the RA layer we want a delta to change our txn to URL1 */ + + SVN_ERR(svn_ra_do_diff3(ra_session, + &reporter, &report_baton, + rev, + target ? svn_path_uri_decode(target, pool) : NULL, + diff_param->depth, + diff_param->ignore_ancestry, + FALSE, /* do not produce text deltas */ + url1, + diff_editor, diff_edit_baton, pool)); + + SVN_ERR(svn_ra_has_capability(ra_session, &server_supports_depth, + SVN_RA_CAPABILITY_DEPTH, pool)); + + /* Create a txn mirror of path2; the diff editor will print + diffs in reverse. :-) */ + SVN_ERR(svn_wc_crawl_revisions3(diff_param->path2, dir_access, + reporter, report_baton, + FALSE, diff_param->depth, + (! server_supports_depth), + FALSE, NULL, NULL, /* notification is N/A */ + NULL, pool)); + + return svn_wc_adm_close(adm_access); +} + /* This is basically just the guts of svn_client_diff_summarize[_peg](). */ static svn_error_t * do_diff_summarize(const struct diff_parameters *diff_param, @@ -1465,13 +1578,39 @@ do_diff_summarize(const struct diff_parameters *di /* Check if paths/revisions are urls/local. */ SVN_ERR(check_paths(diff_param, &diff_paths)); - if (diff_paths.is_repos1 && diff_paths.is_repos2) - return diff_summarize_repos_repos(diff_param, summarize_func, - summarize_baton, ctx, pool); + if (diff_paths.is_repos1) + { + if (diff_paths.is_repos2) + return diff_summarize_repos_repos(diff_param, summarize_func, + summarize_baton, ctx, pool); + else + /* path 2 is a working copy */ + return diff_summarize_repos_wc(diff_param, FALSE, summarize_func, + summarize_baton, ctx, pool); + } else + if (diff_paths.is_repos2) + { + /* path 1 is a working copy, path 2 is a repos url. + * Things need to be reversed, because only the second path + * argument is allowed to be a working copy. + */ + struct diff_parameters *reversed_diff_param = + apr_palloc(pool, sizeof(*diff_param)); + memcpy(reversed_diff_param, diff_param, sizeof(*diff_param)); + + reversed_diff_param->path1 = diff_param->path2; + reversed_diff_param->revision1 = diff_param->revision2; + reversed_diff_param->path2 = diff_param->path1; + reversed_diff_param->revision2 = diff_param->revision1; + + return diff_summarize_repos_wc(reversed_diff_param, TRUE, summarize_func, + summarize_baton, ctx, pool); + } + else return svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, NULL, - _("Summarizing diff can only compare repository " - "to repository")); + _("Sorry, summarizing diff was called in a way " + "that is not yet supported")); } Index: subversion/libsvn_client/repos_diff_summarize.c =================================================================== --- subversion/libsvn_client/repos_diff_summarize.c (revision 33536) +++ subversion/libsvn_client/repos_diff_summarize.c (working copy) @@ -294,6 +294,9 @@ change_prop(void *entry_baton, { ensure_summarize(ib); ib->summarize->prop_changed = TRUE; + /* helpful for debugging + printf("REASON prop %s = %s\n", name, value->data); + */ } return SVN_NO_ERROR;