Neels J. Hofmeyr wrote:
> Hey Julian,
>
> I'm investigating your preliminary patch for directory comparison (I've
> attached the patch for dev@'s reference). Here's what I found:
So, some more.
In merge_tests.py 122
Second merge operation
[[[
# 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 ')
]]]
This section fails because the merge sources are found to differ. There are
two things:
1) dirs_same_p() says that the directories differ because it finds E/newfile
is missing. Because of 2), I guess it is not comparing to the local mods but
only to the previously committed state.
Question: Was that commit missing or is it intentionally omitted / Should
svn treat local mods like the committed ones here?
2) If I add another commit after svn_moddir(...), it still says the
directories differ, also because of E/newfile, but now it says that *files
differ*. Tracing around reveals that it's actually prop changes.
Propchanges are reported on:
"svn:entry:last-author"
"svn:entry:uuid"
"svn:entry:committed-rev"
"svn:entry:committed-date"
So I'm pasting your condition that categorically ignores svn:entry props
from dirs_same_dir_props_changed() over to dirs_same_file_changed().
Fixing these makes merge_tests.py 122 pass. 123 also passes. 124 XFAILS
(wasn't that XPASS before?).
...attaching a new diff that also contains my changes so far, and a diff
between your diff and my diff. Hm, a microbranch would have been useful.
Maybe next time...
NOTE: I updated trunk for this diff, and tests 122 123 124 are now 123 124 125.
--
Neels Hofmeyr -- elego Software Solutions GmbH
Gustav-Meyer-Allee 25 / Gebäude 12, 13355 Berlin, Germany
phone: +49 30 23458696 mobile: +49 177 2345869 fax: +49 30 23458695
http://www.elegosoft.com | Geschäftsführer: Olaf Wagner | Sitz: Berlin
Handelsreg: Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
--- /arch/hg/svn-tc/dirs_same_p-2.patch 2008-09-25 00:20:03.000000000 +0200
+++ /arch/hg/svn-tc/dirs_same_p-3.patch 2008-09-26 04:26:48.000000000 +0200
@@ -26,9 +26,9 @@
Index: subversion/libsvn_client/merge.c
===================================================================
---- subversion/libsvn_client/merge.c (revision 33244)
+--- subversion/libsvn_client/merge.c (revision 33305)
+++ subversion/libsvn_client/merge.c (working copy)
-@@ -1388,33 +1388,59 @@ properties_same_p(svn_boolean_t *same,
+@@ -1385,33 +1385,59 @@ properties_same_p(svn_boolean_t *same,
return SVN_NO_ERROR;
}
@@ -97,16 +97,16 @@
*same = !modified;
}
-@@ -1455,10 +1481,15 @@ merge_file_deleted(svn_wc_adm_access_t *
+@@ -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));
@@ -115,7 +115,7 @@
if (same || merge_b->force || merge_b->record_only /* ### why? */)
{
/* Passing NULL for the notify_func and notify_baton because
-@@ -1676,6 +1707,256 @@ merge_dir_added(svn_wc_adm_access_t *adm
+@@ -1673,8 +1704,278 @@ merge_dir_added(svn_wc_adm_access_t *adm_access,
return SVN_NO_ERROR;
}
@@ -126,8 +126,8 @@
+ apr_pool_t *pool;
+} dirs_same_baton_t;
+
-+/* An svn_wc_diff_callbacks3_t function. */
-+static svn_error_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,
@@ -144,7 +144,26 @@
+{
+ dirs_same_baton_t *baton = diff_baton;
+
-+ baton->was_modified = TRUE;
++ /* 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;
+}
+
@@ -222,10 +241,11 @@
+ dirs_same_baton_t *baton = diff_baton;
+ apr_array_header_t *props;
+
-+ /* Ignore entry-props and wc-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;
+}
+
@@ -267,7 +287,7 @@
+ /* 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 char *anchor, *anchor_url, *target = "";
+ const svn_wc_entry_t *entry;
+ svn_ra_session_t *ra_session2;
+ const svn_wc_diff_callbacks3_t callbacks =
@@ -369,10 +389,12 @@
+ return SVN_NO_ERROR;
+}
+
- /* An svn_wc_diff_callbacks3_t function. */
- static svn_error_t *
++/* An svn_wc_diff_callbacks3_t function. */
++static svn_error_t *
merge_dir_deleted(svn_wc_adm_access_t *adm_access,
-@@ -1687,9 +1968,6 @@ merge_dir_deleted(svn_wc_adm_access_t *a
+ 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;
@@ -382,7 +404,7 @@
/* Easy out: if we have no adm_access for the parent directory,
then this portion of the tree-delta "patch" must be inapplicable.
-@@ -1714,32 +1992,49 @@ merge_dir_deleted(svn_wc_adm_access_t *a
+@@ -1711,32 +2009,50 @@ merge_dir_deleted(svn_wc_adm_access_t *adm_access,
{
if (entry && (entry->schedule != svn_wc_schedule_delete))
{
@@ -429,11 +451,12 @@
+ 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));
++ 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,
++ SVN_ERR(svn_client__wc_delete(path, parent_access,
++ merge_b->force,
+ merge_b->dry_run, FALSE,
+ NULL, NULL,
+ merge_b->ctx, subpool));
@@ -451,7 +474,7 @@
}
}
else
-@@ -3820,7 +4115,7 @@ mark_mergeinfo_as_inheritable_for_a_rang
+@@ -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.
@@ -460,7 +483,7 @@
and REV is the revision to get.
The new temporary file will be created as a sibling of WC_TARGET.
-@@ -3836,6 +4131,7 @@ mark_mergeinfo_as_inheritable_for_a_rang
+@@ -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,
@@ -468,7 +491,7 @@
apr_hash_t **props,
svn_revnum_t rev,
const char *wc_target,
-@@ -3848,7 +4144,7 @@ single_file_merge_get_file(const char **
+@@ -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);
@@ -477,7 +500,7 @@
stream, NULL, props, pool));
return svn_stream_close(stream);
}
-@@ -5328,10 +5624,10 @@ do_file_merge(const char *url1,
+@@ -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. */
@@ -492,7 +515,7 @@
Index: subversion/tests/cmdline/merge_tests.py
===================================================================
---- subversion/tests/cmdline/merge_tests.py (revision 33244)
+--- 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',
@@ -521,10 +544,11 @@
svn_delete(source+"/tau")
s_rev_tau = svn_commit(source)
svn_delete(source+"/pi")
-@@ -13937,6 +13943,109 @@ def tree_conflicts_on_merge_no_local_ci_
-
-
+@@ -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"
+
@@ -556,8 +580,8 @@
+ svn_copy(s_rev_orig, source, target)
+ svn_commit(target)
+ svn_moddir(target+"/E")
++ svn_commit(target)
+ # Should be deleted quietly.
-+ raise ###
+ svn_merge(s_rev_del, source, target, '--- Merging|D ')
+
+ os.chdir(saved_cwd)
@@ -628,13 +652,14 @@
+
+ os.chdir(saved_cwd)
+
-
++
########################################################################
# Run the tests
-@@ -14137,6 +14246,9 @@ test_list = [ None,
- tree_conflicts_on_merge_no_local_ci_5_1,
- tree_conflicts_on_merge_no_local_ci_5_2,
+
+@@ -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),
Received on 2008-09-26 04:30:21 CEST