Hey guys,
while trying to write a test for my most recent fix ("resolved" with
--depth), I found a most horrendous failure in tree-conflicts reporting
during update.
See r34153 (on branch tc-resolve), inline comments in depth_tests.py's
function make_depth_tree_conflicts(), marked "##". :O
Had no time to investigate yet.
~Neels
attached mail follows:
Author: neels
Date: Tue Nov 11 22:01:26 2008
New Revision: 34153
Log:
Fix the behaviour of "resolved" that descends into unversioned
tree-conflicts even if the depth doesn't allow that.
* subversion/libsvn_wc/entries.c
(visit_tc_too_baton_t): Add TARGET and DEPTH fields.
(visit_tc_too_found_entry):
Use new TARGET and DEPTH fields in the baton to determine whether to
descend into unversioned tree-conflicted children of a directory.
(svn_wc__walk_entries_and_tc): Supply TARGET and DEPTH to the baton.
* subversion/tests/cmdline/tree_conflict_tests.py
(ensure_tree_conflict):
Make a "resolved" test also try run "resolved" on the victim's
parent first, to make sure it doesn't resolve its children.
Remove comment about not supporting "resolved" per victim.
* subversion/tests/cmdline/depth_tests.py
(tree_conflicts_resolved_depth_empty, tree_conflicts_resolved_depth_files,
tree_conflicts_resolved_depth_immediates,
tree_conflicts_resolved_depth_infinity, make_depth_tree_conflicts):
Four new tests and a helper. The tests are XFAIL because the helper
make_depth_tree_conflicts() exposes an error in update code that
is not related to "resolved". Separate trials suggest that these
tests will pass as soon as the update issue is resolved. See
inline comment in make_depth_tree_conflicts() marked "##".
Modified:
branches/tc-resolve/subversion/libsvn_wc/entries.c
branches/tc-resolve/subversion/tests/cmdline/depth_tests.py
branches/tc-resolve/subversion/tests/cmdline/tree_conflict_tests.py
Modified: branches/tc-resolve/subversion/libsvn_wc/entries.c
URL: http://svn.collab.net/viewvc/svn/branches/tc-resolve/subversion/libsvn_wc/entries.c?pathrev=34153&r1=34152&r2=34153
==============================================================================
--- branches/tc-resolve/subversion/libsvn_wc/entries.c Tue Nov 11 21:47:14 2008 (r34152)
+++ branches/tc-resolve/subversion/libsvn_wc/entries.c Tue Nov 11 22:01:26 2008 (r34153)
@@ -3316,6 +3316,8 @@ typedef struct visit_tc_too_baton_t
svn_wc_adm_access_t *adm_access;
const svn_wc_entry_callbacks2_t *callbacks;
void *baton;
+ const char *target;
+ svn_depth_t depth;
} visit_tc_too_baton_t;
/* An svn_wc_entry_callbacks2_t callback function.
@@ -3333,14 +3335,44 @@ visit_tc_too_found_entry(const char *pat
apr_pool_t *pool)
{
struct visit_tc_too_baton_t *baton = walk_baton;
+ svn_boolean_t check_children;
/* Call the entry callback for this entry. */
SVN_ERR(baton->callbacks->found_entry(path, entry, baton->baton, pool));
- /* If this is a directory, also visit any unversioned children that are
- * tree conflict victims. */
- if (entry->kind == svn_node_dir && !entry_is_hidden(entry))
+ if (entry->kind != svn_node_dir || entry_is_hidden(entry))
+ return SVN_NO_ERROR;
+
+ /* If this is a directory, we may need to also visit any unversioned
+ * children that are tree conflict victims. However, that should not
+ * happen when we've already reached the requested depth. */
+
+ switch (baton->depth){
+ case svn_depth_empty:
+ check_children = FALSE;
+ break;
+
+ /* Since svn_depth_files only visits files and this is a directory,
+ * we have to be at the target. Just verify that anyway: */
+ case svn_depth_files:
+ case svn_depth_immediates:
+ /* Check if this already *is* an immediate child, in which
+ * case we shouldn't descend further. */
+ check_children = (strcmp(baton->target, path) == 0);
+ break;
+
+ case svn_depth_infinity:
+ case svn_depth_exclude:
+ case svn_depth_unknown:
+ check_children = TRUE;
+ break;
+ };
+
+ if (check_children)
{
+ /* We're supposed to check the children of this directory. However,
+ * in case of svn_depth_files, don't visit directories. */
+
svn_wc_adm_access_t *adm_access = NULL;
apr_array_header_t *conflicts
= apr_array_make(pool, 0, sizeof(svn_wc_conflict_description_t *));
@@ -3360,6 +3392,10 @@ visit_tc_too_found_entry(const char *pat
= APR_ARRAY_IDX(conflicts, i, svn_wc_conflict_description_t *);
const svn_wc_entry_t *child_entry;
+ if ((conflict->node_kind == svn_node_dir)
+ && (baton->depth == svn_depth_files))
+ continue;
+
/* If this victim is not in this dir's entries ... */
SVN_ERR(svn_wc_entry(&child_entry, conflict->path, adm_access,
TRUE, pool));
@@ -3482,6 +3518,8 @@ svn_wc__walk_entries_and_tc(const char *
visit_tc_too_baton.adm_access = adm_access;
visit_tc_too_baton.callbacks = walk_callbacks;
visit_tc_too_baton.baton = walk_baton;
+ visit_tc_too_baton.target = path;
+ visit_tc_too_baton.depth = depth;
SVN_ERR(svn_wc_walk_entries3(path, path_adm_access,
&visit_tc_too_callbacks, &visit_tc_too_baton,
Modified: branches/tc-resolve/subversion/tests/cmdline/depth_tests.py
URL: http://svn.collab.net/viewvc/svn/branches/tc-resolve/subversion/tests/cmdline/depth_tests.py?pathrev=34153&r1=34152&r2=34153
==============================================================================
--- branches/tc-resolve/subversion/tests/cmdline/depth_tests.py Tue Nov 11 21:47:14 2008 (r34152)
+++ branches/tc-resolve/subversion/tests/cmdline/depth_tests.py Tue Nov 11 22:01:26 2008 (r34153)
@@ -2246,6 +2246,176 @@ def excluded_receive_remote_removal(sbox
#----------------------------------------------------------------------
+# Check that "svn resolved" visits tree-conflicts *on unversioned items*
+# according to the --depth parameter.
+
+def make_depth_tree_conflicts(sbox):
+ "Helper for tree_conflicts_resolved_depth_*"
+
+ sbox.build()
+ wc = sbox.wc_dir
+
+ j = os.path.join
+ A = j(wc, 'A')
+ B = j(A, 'B')
+ l = j(B, 'lambda')
+ E = j(B, 'E')
+ a = j(E, 'alpha')
+
+ # Store node modifications as rev 2
+ svntest.actions.run_and_verify_svn(None, None, [],
+ 'propset', 'foo', 'foo-val', E)
+ svntest.main.file_append(l, "Modified lambda.\n")
+ svntest.main.file_append(a, "Modified alpha.\n")
+
+ expected_output = svntest.wc.State(wc, {
+ 'A/B/E' : Item(verb='Sending'),
+ 'A/B/E/alpha' : Item(verb='Sending'),
+ 'A/B/lambda' : Item(verb='Sending'),
+ })
+
+ expected_status = svntest.actions.get_virginal_state(wc, 1)
+ expected_status.tweak('A/B/lambda', 'A/B/E', 'A/B/E/alpha',
+ wc_rev = 2)
+
+ svntest.actions.run_and_verify_commit(wc,
+ expected_output,
+ expected_status,
+ None,
+ A)
+
+ # Go back to rev 1
+ expected_output = svntest.wc.State(wc, {
+ 'A/B/E' : Item(status=' U'),
+ 'A/B/E/alpha' : Item(status='U '),
+ 'A/B/lambda' : Item(status='U '),
+ })
+ expected_status = svntest.actions.get_virginal_state(wc, 1)
+ expected_disk = svntest.main.greek_state.copy()
+ svntest.actions.run_and_verify_update(wc,
+ expected_output,
+ expected_disk,
+ expected_status,
+ None, None, None, None, None, False,
+ '-r1', A)
+
+ # Perform node deletions so that items become unversioned and
+ # will have tree-conflicts upon update.
+ svntest.actions.run_and_verify_svn(None, None, [],
+ 'rm', l)
+
+ ## Attention:
+ ## This test fails because a tree-conflict isn't recorded.
+ ## Running this test with the -v flag ( ./depth_tests.py -v 35 )
+ ## shows that the node `lambda' is well reported tree-conflicted
+ ## in the output, but the status doesn't show a C on `lambda'!
+ ## Now, if you remove this following command:
+ svntest.actions.run_and_verify_svn(None, None, [],
+ 'rm', E)
+ ## and also uncomment the two lines below that start with `###',
+ ## you will see that the status *does* report the conflict, as
+ ## seen in ./update_tests.py 46 (tree conflicts on update 1.1).
+ ## So persistence of the conflict depends on whether
+ ## siblings are also conflicted!
+
+ # Update so that conflicts appear
+ expected_output = svntest.wc.State(wc, {
+ 'A/B/lambda' : Item(status=' ', treeconflict='C'),
+ 'A/B/E' : Item(status=' ', treeconflict='C'),
+ 'A/B/E/alpha' : Item(status=' ', treeconflict='C'),
+ })
+
+ expected_disk = svntest.main.greek_state.copy()
+ expected_disk.remove('A/B/lambda', 'A/B/E/alpha', 'A/B/E/beta');
+ ### expected_output = None
+ ### expected_disk = None
+
+ expected_status = svntest.actions.get_virginal_state(wc, 2)
+ expected_status.tweak('A/B/E', 'A/B/E/alpha', 'A/B/E/beta', 'A/B/lambda',
+ status='D ')
+ expected_status.tweak('A/B/lambda', 'A/B/E', 'A/B/E/alpha',
+ treeconflict='C')
+ expected_status.tweak('A/B/E/alpha', 'A/B/lambda', wc_rev='1')
+
+ svntest.actions.run_and_verify_update(wc,
+ expected_output,
+ expected_disk,
+ expected_status,
+ None, None, None, None, None, False,
+ wc)
+
+
+
+def tree_conflicts_resolved_depth_empty(sbox):
+ "tree conflicts resolved depth-empty"
+
+ make_depth_tree_conflicts(sbox)
+
+ wc = sbox.wc_dir
+ B = os.path.join(wc, 'A', 'B')
+
+ svntest.actions.run_and_verify_svn(None,
+ [],
+ [],
+ 'resolved', '--depth=empty', B)
+
+
+def tree_conflicts_resolved_depth_files(sbox):
+ "tree conflicts resolved depth-files"
+
+ make_depth_tree_conflicts(sbox)
+
+ wc = sbox.wc_dir
+ j = os.path.join
+ B = j(wc, 'A', 'B')
+ l = j(B, 'lambda')
+
+
+ svntest.actions.run_and_verify_svn(None,
+ ["Resolved conflicted state of '%s'\n" % l],
+ [],
+ 'resolved', '--depth=files', B)
+
+
+def tree_conflicts_resolved_depth_immediates(sbox):
+ "tree conflicts resolved depth-immediates"
+
+ make_depth_tree_conflicts(sbox)
+
+ wc = sbox.wc_dir
+ j = os.path.join
+ B = j(wc, 'A', 'B')
+ l = j(B, 'lambda')
+ E = j(B, 'E')
+
+ svntest.actions.run_and_verify_svn(None,
+ ["Resolved conflicted state of '%s'\n" % l,
+ "Resolved conflicted state of '%s'\n" % E],
+ [],
+ 'resolved', '--depth=immediates', B)
+
+
+def tree_conflicts_resolved_depth_infinity(sbox):
+ "tree conflicts resolved depth-infinity"
+
+ make_depth_tree_conflicts(sbox)
+
+ wc = sbox.wc_dir
+ j = os.path.join
+ B = j(wc, 'A', 'B')
+ l = j(B, 'lambda')
+ E = j(B, 'E')
+ a = j(E, 'alpha')
+
+ svntest.actions.run_and_verify_svn(None,
+ ["Resolved conflicted state of '%s'\n" % l,
+ "Resolved conflicted state of '%s'\n" % E,
+ "Resolved conflicted state of '%s'\n" % a],
+ [],
+ 'resolved', '--depth=infinity', B)
+
+
+#----------------------------------------------------------------------
# list all tests here, starting with None:
test_list = [ None,
depth_empty_checkout,
@@ -2282,6 +2452,10 @@ test_list = [ None,
excluded_path_update_operation,
excluded_path_misc_operation,
excluded_receive_remote_removal,
+ XFail(tree_conflicts_resolved_depth_empty),
+ XFail(tree_conflicts_resolved_depth_files),
+ XFail(tree_conflicts_resolved_depth_immediates),
+ XFail(tree_conflicts_resolved_depth_infinity),
]
if __name__ == "__main__":
Modified: branches/tc-resolve/subversion/tests/cmdline/tree_conflict_tests.py
URL: http://svn.collab.net/viewvc/svn/branches/tc-resolve/subversion/tests/cmdline/tree_conflict_tests.py?pathrev=34153&r1=34152&r2=34153
==============================================================================
--- branches/tc-resolve/subversion/tests/cmdline/tree_conflict_tests.py Tue Nov 11 21:47:14 2008 (r34152)
+++ branches/tc-resolve/subversion/tests/cmdline/tree_conflict_tests.py Tue Nov 11 22:01:26 2008 (r34153)
@@ -472,8 +472,11 @@ def ensure_tree_conflict(sbox, operation
if 'resolve' in test_what:
verbose_print("--- Resolving the conflict")
- # Resolve recursively on the parent, because per-victim resolve is
- # not yet implemented.
+ # Make sure resolving the parent does nothing.
+ run_and_verify_svn(None,
+ [], [],
+ 'resolved', os.path.dirname(victim))
+ # The real resolved call.
run_and_verify_svn(None,
"Resolved .* '" + re.escape(victim) + "'", [],
'resolved', victim)
---------------------------------------------------------------------
To unsubscribe, e-mail: svn-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: svn-help_at_subversion.tigris.org
Received on 2008-11-12 07:09:06 CET