[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Tree conflict bugs I've just been finding

From: Stefan Sperling <stsp_at_elego.de>
Date: Fri, 14 Nov 2008 00:36:46 +0000

On Fri, Nov 14, 2008 at 12:25:02AM +0000, Stefan Sperling wrote:
> On Thu, Nov 13, 2008 at 07:49:20PM +0000, Julian Foad wrote:
> > Bug: "resolved" complains if the target's parent isn't a WC:
> > [[[
> > $ svn resolved wc2
> > svn: warning: '.' is not a working copy
> >
> > $ svn st wc2
> > M C wc2/foo
> > ]]]
>
> Hey Julian,
>
> I've tried to take a shot at this one. My current patch is below.
>
> 4 tests in tree_conflicts_tests.py are failing with this patch:
> FAIL: tree_conflict_tests.py 9: merge file: modify onto not-file
> FAIL: tree_conflict_tests.py 11: merge file: del/rpl/mv onto not-file
> FAIL: tree_conflict_tests.py 13: merge dir: modify onto not-dir
> FAIL: tree_conflict_tests.py 15: merge dir: del/rpl/mv onto not-dir
>
> I don't exactly know why. I may have an error in my patch.
> Maybe you can investigate?

> + if (! wc_root) /* but possibly a switched subdir */
> + {
> + /* In order to resolve tree-conflicts on the target PATH, we need an
> + * adm_access on its parent directory. The lock level then needs to
> + * extend at least onto the immediate children. */
> + SVN_ERR(svn_wc_adm_close2(adm_access, pool));
> + adm_lock_level++;

There's a bug here---^

adm_lock_level should only be increased if it's already
greater or equal 0 (as done in the original code).

The tests still fail even with that fixed though.

New diff with fix:

Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c (revision 34183)
+++ subversion/libsvn_wc/adm_ops.c (working copy)
@@ -2871,6 +2871,7 @@ resolve_found_entry_callback(const char *path,
 {
   struct resolve_callback_baton *baton = walk_baton;
   svn_boolean_t resolved = FALSE;
+ svn_boolean_t wc_root;
 
   /* We're going to receive dirents twice; we want to ignore the
      first one (where it's a child of a parent dir), and only process
@@ -2880,9 +2881,23 @@ resolve_found_entry_callback(const char *path,
     return SVN_NO_ERROR;
 
 
- /* If asked to, clear any tree conflict on the path. */
- if (baton->resolve_tree)
+ /* If asked to, clear any tree conflict on the path.
+ * But make sure we do not end up looking for tree conflict
+ * info above the working copy root. */
+ SVN_ERR(svn_wc_is_wc_root(&wc_root, path, baton->adm_access, pool));
+ if (wc_root)
     {
+ /* Switched subtrees are considered working copy roots by
+ * svn_wc_is_wc_root(). But it's OK to check for tree conflict
+ * info in the parent of a switched subtree, because the
+ * subtree itself might be a tree conflict victim. */
+ svn_boolean_t switched;
+ SVN_ERR(svn_wc__path_switched(path, &switched, entry, pool));
+ wc_root = switched ? FALSE : TRUE;
+ }
+
+ if (baton->resolve_tree && ! wc_root) /* but possibly a switched subdir */
+ {
       const char *conflict_dir, *base_name = NULL;
       svn_wc_adm_access_t *parent_adm_access;
       svn_wc_conflict_description_t *conflict;
Index: subversion/libsvn_client/resolved.c
===================================================================
--- subversion/libsvn_client/resolved.c (revision 34183)
+++ subversion/libsvn_client/resolved.c (working copy)
@@ -53,19 +53,46 @@ svn_client_resolve(const char *path,
 {
   svn_wc_adm_access_t *adm_access;
   int adm_lock_level = SVN_WC__LEVELS_TO_LOCK_FROM_DEPTH(depth);
+ svn_boolean_t wc_root;
+ const svn_wc_entry_t *entry;
 
- /* In order to resolve tree-conflicts on the target PATH, we need an
- * adm_access on its parent directory. The lock level then needs to extend
- * at least onto the immediate children. */
- if (adm_lock_level >= 0)
- adm_lock_level++;
   SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL,
- svn_path_dirname(path, pool),
+ path,
                                  TRUE,
                                  adm_lock_level,
                                  ctx->cancel_func, ctx->cancel_baton,
                                  pool));
 
+ /* Make sure we do not end up looking for tree conflict info
+ * above the working copy root. */
+ SVN_ERR(svn_wc_is_wc_root(&wc_root, path, adm_access, pool));
+ if (wc_root)
+ {
+ /* Switched subtrees are considered working copy roots by
+ * svn_wc_is_wc_root(). But it's OK to check for tree conflict
+ * info in the parent of a switched subtree, because the
+ * subtree itself might be a tree conflict victim. */
+ svn_boolean_t switched;
+ SVN_ERR(svn_wc_entry(&entry, path, adm_access, TRUE, pool));
+ SVN_ERR(svn_wc__path_switched(path, &switched, entry, pool));
+ wc_root = switched ? FALSE : TRUE;
+ }
+
+ if (! wc_root) /* but possibly a switched subdir */
+ {
+ /* In order to resolve tree-conflicts on the target PATH, we need an
+ * adm_access on its parent directory. The lock level then needs to
+ * extend at least onto the immediate children. */
+ SVN_ERR(svn_wc_adm_close2(adm_access, pool));
+ if (adm_lock_level >= 0)
+ adm_lock_level++;
+ SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL,
+ svn_path_dirname(path, pool),
+ TRUE, adm_lock_level,
+ ctx->cancel_func, ctx->cancel_baton,
+ pool));
+ }
+
   SVN_ERR(svn_wc_resolved_conflict4(path, adm_access, TRUE, TRUE, TRUE,
                                     depth, conflict_choice,
                                     ctx->notify_func2, ctx->notify_baton2,

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-11-14 01:37:02 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.