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

[Fwd: svn commit: r34133 - in branches/tc-resolve/subversion: include libsvn_client libsvn_wc]

From: Neels J. Hofmeyr <neels_at_elego.de>
Date: Tue, 11 Nov 2008 05:26:04 +0100

Hey Julian,

I've gotten as far as none of the tree_conflicts tests failing anymore. But
could you please closely review these two things:

1
[[[
* subversion/libsvn_client/resolved.c
  (svn_client_resolve):
    Get the ADM_ACCESS on the target's parent directory to be able to resolve
    tree-conflicted directories. Extend the ADM_LOCK_LEVEL to still be able
    to lock the target node for text/prop conflicts.

* subversion/include/svn_client.h
  (svn_client_resolve): Comment.
]]]

and

2
[[[
* subversion/libsvn_wc/adm_ops.c
  (resolve_found_entry_callback):
    Completely split tree-conflict resolution from text and prop conflicts,
    because tree-conflicts need a different ADM_ACCESS when handling
    directories. Change use of RESOLVED to already reflect the sanity-check,
    and sanity-check separately for tree-conflicts. Clarify some conditions.
    Also remove the debugging conditions that exited prematurely.
]]]

...come to think of it, why don't we list dir-tree-conflicts in the THISDIR
entry of the victim instead of in the parent directory? That would ease the
locking stuff by preventing locking above the target subtree. Right?

Thanks
~Neels

attached mail follows:


Author: neels
Date: Mon Nov 10 20:04:21 2008
New Revision: 34133

Log:
Fix per-victim "resolved" on directory tree-conflict victims.

* subversion/libsvn_client/resolved.c
  (svn_client_resolve):
    Get the ADM_ACCESS on the target's parent directory to be able to resolve
    tree-conflicted directories. Extend the ADM_LOCK_LEVEL to still be able
    to lock the target node for text/prop conflicts.

* subversion/include/svn_client.h
  (svn_client_resolve): Comment.

* subversion/libsvn_wc/adm_ops.c
  (resolve_found_entry_callback):
    Completely split tree-conflict resolution from text and prop conflicts,
    because tree-conflicts need a different ADM_ACCESS when handling
    directories. Change use of RESOLVED to already reflect the sanity-check,
    and sanity-check separately for tree-conflicts. Clarify some conditions.
    Also remove the debugging conditions that exited prematurely.

* subversion/libsvn_wc/tree_conflicts.c
  (svn_wc__loggy_del_tree_conflict):
    Extend an assertion by a NULL check. Add another assertion to make sure
    the VICTIM_PATH is a child node of the ADM_ACCESS' path.

* subversion/libsvn_wc/tree_conflicts.h
  (svn_wc__loggy_add_tree_conflict,
   svn_wc__loggy_del_tree_conflict,
   svn_wc__del_tree_conflict,
   svn_wc__write_tree_conflicts_to_entry,
   svn_wc__tree_conflict_exists):
    Comments, add "@since New in 1.6.".

Modified:
   branches/tc-resolve/subversion/include/svn_client.h
   branches/tc-resolve/subversion/libsvn_client/resolved.c
   branches/tc-resolve/subversion/libsvn_wc/adm_ops.c
   branches/tc-resolve/subversion/libsvn_wc/tree_conflicts.c
   branches/tc-resolve/subversion/libsvn_wc/tree_conflicts.h

Modified: branches/tc-resolve/subversion/include/svn_client.h
URL: http://svn.collab.net/viewvc/svn/branches/tc-resolve/subversion/include/svn_client.h?pathrev=34133&r1=34132&r2=34133
==============================================================================
--- branches/tc-resolve/subversion/include/svn_client.h Mon Nov 10 09:14:36 2008 (r34132)
+++ branches/tc-resolve/subversion/include/svn_client.h Mon Nov 10 20:04:21 2008 (r34133)
@@ -3007,6 +3007,8 @@ svn_client_resolved(const char *path,
  * all its immediate conflicted children (both files and directories,
  * if any); if @c svn_depth_infinity, resolve @a path and every
  * conflicted file or directory anywhere beneath it.
+ * Note that this operation will try to lock the parent directory of
+ * @a path in order to be able to resolve tree-conflicts on @a path.
  *
  * If @a conflict_choice is @c svn_wc_conflict_choose_base, resolve the
  * conflict with the old file contents; if

Modified: branches/tc-resolve/subversion/libsvn_client/resolved.c
URL: http://svn.collab.net/viewvc/svn/branches/tc-resolve/subversion/libsvn_client/resolved.c?pathrev=34133&r1=34132&r2=34133
==============================================================================
--- branches/tc-resolve/subversion/libsvn_client/resolved.c Mon Nov 10 09:14:36 2008 (r34132)
+++ branches/tc-resolve/subversion/libsvn_client/resolved.c Mon Nov 10 20:04:21 2008 (r34133)
@@ -53,7 +53,14 @@ 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_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, path, TRUE,
+ /* 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 = 1;
+ 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));

Modified: branches/tc-resolve/subversion/libsvn_wc/adm_ops.c
URL: http://svn.collab.net/viewvc/svn/branches/tc-resolve/subversion/libsvn_wc/adm_ops.c?pathrev=34133&r1=34132&r2=34133
==============================================================================
--- branches/tc-resolve/subversion/libsvn_wc/adm_ops.c Mon Nov 10 09:14:36 2008 (r34132)
+++ branches/tc-resolve/subversion/libsvn_wc/adm_ops.c Mon Nov 10 20:04:21 2008 (r34133)
@@ -2870,78 +2870,96 @@ resolve_found_entry_callback(const char
                              apr_pool_t *pool)
 {
   struct resolve_callback_baton *baton = walk_baton;
- const char *conflict_dir, *base_name = NULL;
- svn_wc_adm_access_t *adm_access;
   svn_boolean_t resolved = FALSE;
 
- if (!entry)
- {
- printf("### Unversioned... woo-hoo! resolve_f_e_cb('%s')\n", path);
- return SVN_NO_ERROR;
- }
- if (entry->deleted || entry->absent)
- {
- printf("### resolve_f_e_cb('%s'): Deleted or Absent\n", path);
- return SVN_NO_ERROR;
- }
- if (entry->schedule == svn_wc_schedule_delete)
- {
- printf("### Sched-delete: resolve_f_e_cb('%s')\n", path);
- }
-
   /* 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
      the second one (where we're looking at THIS_DIR). */
   if (entry && (entry->kind == svn_node_dir)
- && (strcmp(entry->name, SVN_WC_ENTRY_THIS_DIR)) != 0)
+ && (strcmp(entry->name, SVN_WC_ENTRY_THIS_DIR) != 0))
     return SVN_NO_ERROR;
 
- /* Figger out the directory in which the conflict resides. */
- if (entry && entry->kind == svn_node_dir)
- conflict_dir = path;
- else
- svn_path_split(path, &conflict_dir, &base_name, pool);
- SVN_ERR(svn_wc_adm_probe_retrieve(&adm_access, baton->adm_access,
- conflict_dir, pool));
 
- /* Clear any tree conflict on the path. */
+ /* If asked to, clear any tree conflict on the path. */
   if (baton->resolve_tree)
     {
+ const char *conflict_dir, *base_name = NULL;
+ svn_wc_adm_access_t *parent_adm_access;
       svn_wc_conflict_description_t *conflict;
+ svn_boolean_t tree_conflict;
 
- SVN_ERR(svn_wc_get_tree_conflict(&conflict, path, adm_access, pool));
+ /* For tree-conflicts, we want the *parent* directory's adm_access,
+ * even for directories. */
+ svn_path_split(path, &conflict_dir, &base_name, pool);
+ SVN_ERR(svn_wc_adm_probe_retrieve(&parent_adm_access, baton->adm_access,
+ conflict_dir, pool));
+
+ SVN_ERR(svn_wc_get_tree_conflict(&conflict, path, parent_adm_access,
+ pool));
       if (conflict)
         {
- SVN_ERR(svn_wc__del_tree_conflict(path, adm_access, pool));
- resolved = TRUE;
+ SVN_ERR(svn_wc__del_tree_conflict(path, parent_adm_access, pool));
+
+ /* Sanity check: see if libsvn_wc *still* thinks this item is in a
+ state of conflict that we have asked to resolve. If so,
+ don't flag RESOLVED_TREE after all. */
+
+ SVN_ERR(svn_wc_conflicted_p2(NULL, NULL, &tree_conflict, path,
+ parent_adm_access, pool));
+ if (!tree_conflict)
+ resolved = TRUE;
         }
     }
 
+
   /* If this is a versioned entry, resolve its other conflicts, if any. */
- if (entry)
- SVN_ERR(resolve_conflict_on_entry(path, entry, adm_access, base_name,
- baton->resolve_text, baton->resolve_props,
- baton->conflict_choice, &resolved,
- pool));
+ if (entry && (baton->resolve_text || baton->resolve_props))
+ {
+ const char *conflict_dir, *base_name = NULL;
+ svn_wc_adm_access_t *adm_access;
+ svn_boolean_t did_resolve = FALSE;
+
+ SVN_ERR(svn_wc_adm_probe_retrieve(&adm_access, baton->adm_access,
+ path, pool));
+
+ SVN_ERR(resolve_conflict_on_entry(path, entry, adm_access, base_name,
+ baton->resolve_text,
+ baton->resolve_props,
+ baton->conflict_choice,
+ &did_resolve,
+ pool));
+
+ /* Sanity check: see if libsvn_wc *still* thinks this item is in a
+ state of conflict that we have asked to resolve. If so,
+ don't flag RESOLVED after all. */
+ if (did_resolve)
+ {
+ svn_boolean_t text_conflict, prop_conflict;
+
+ SVN_ERR(svn_wc_conflicted_p2(&text_conflict, &prop_conflict,
+ NULL, path, adm_access,
+ pool));
+
+ if ((baton->resolve_text && text_conflict)
+ || (baton->resolve_props && prop_conflict))
+ /* Explicitly overwrite a possible TRUE from tree-conflict
+ * resolution. If this part failed, it shouldn't be notified
+ * as resolved. (Keeping in an "if...else" for clarity.)
+ * (Actually, we defined that a node can't have other conflicts
+ * when it is tree-conflicted. This here can't hurt though.)*/
+ resolved = FALSE;
+ else
+ resolved = TRUE;
+ }
+ }
 
   /* Notify */
   if (baton->notify_func && resolved)
- {
- /* Sanity check: see if libsvn_wc *still* thinks this item is in a
- state of conflict that we have asked to resolve. If not, report
- the successful resolution. */
- svn_boolean_t text_conflict, prop_conflict, tree_conflict;
- SVN_ERR(svn_wc_conflicted_p2(&text_conflict, &prop_conflict,
- &tree_conflict, path, adm_access,
- pool));
- /*printf("### resolve_found_entry cb ('%s'):%d,%d,%d\n", path, text_conflict, prop_conflict, tree_conflict);*/
- if ((! (baton->resolve_text && text_conflict))
- && (! (baton->resolve_props && prop_conflict))
- && (! (baton->resolve_tree && tree_conflict)))
- (*baton->notify_func)(baton->notify_baton,
- svn_wc_create_notify(path, svn_wc_notify_resolved,
- pool), pool);
- }
+ (*baton->notify_func)(baton->notify_baton,
+ svn_wc_create_notify(path, svn_wc_notify_resolved,
+ pool),
+ pool);
+
   return SVN_NO_ERROR;
 }
 

Modified: branches/tc-resolve/subversion/libsvn_wc/tree_conflicts.c
URL: http://svn.collab.net/viewvc/svn/branches/tc-resolve/subversion/libsvn_wc/tree_conflicts.c?pathrev=34133&r1=34132&r2=34133
==============================================================================
--- branches/tc-resolve/subversion/libsvn_wc/tree_conflicts.c Mon Nov 10 09:14:36 2008 (r34132)
+++ branches/tc-resolve/subversion/libsvn_wc/tree_conflicts.c Mon Nov 10 20:04:21 2008 (r34133)
@@ -540,7 +540,11 @@ svn_wc__loggy_del_tree_conflict(svn_stri
    * Otherwise we should not have been called. */
   dir_path = svn_wc_adm_access_path(adm_access);
   SVN_ERR(svn_wc_entry(&entry, dir_path, adm_access, TRUE, pool));
- SVN_ERR_ASSERT(entry->kind == svn_node_dir);
+ SVN_ERR_ASSERT((entry != NULL) && (entry->kind == svn_node_dir));
+
+ /* Make sure that VICTIM_PATH is a child node of DIR_PATH.
+ * Anything else is a bug. */
+ SVN_ERR_ASSERT(strcmp(dir_path, svn_path_dirname(victim_path, pool)) == 0);
 
   conflicts = apr_array_make(pool, 0,
                              sizeof(svn_wc_conflict_description_t *));

Modified: branches/tc-resolve/subversion/libsvn_wc/tree_conflicts.h
URL: http://svn.collab.net/viewvc/svn/branches/tc-resolve/subversion/libsvn_wc/tree_conflicts.h?pathrev=34133&r1=34132&r2=34133
==============================================================================
--- branches/tc-resolve/subversion/libsvn_wc/tree_conflicts.h Mon Nov 10 09:14:36 2008 (r34132)
+++ branches/tc-resolve/subversion/libsvn_wc/tree_conflicts.h Mon Nov 10 20:04:21 2008 (r34133)
@@ -148,6 +148,8 @@
  *
  * If *LOG_ACCUM is NULL then set *LOG_ACCUM to a new stringbug allocated in
  * POOL, else append to the existing stringbuf there.
+ *
+ * @since New in 1.6.
  */
 svn_error_t *
 svn_wc__loggy_add_tree_conflict(svn_stringbuf_t **log_accum,
@@ -162,6 +164,8 @@ svn_wc__loggy_add_tree_conflict(svn_stri
  *
  * If *LOG_ACCUM is NULL then set *LOG_ACCUM to a new stringbug allocated in
  * POOL, else append to the existing stringbuf there.
+ *
+ * @since New in 1.6.
  */
 svn_error_t *
 svn_wc__loggy_del_tree_conflict(svn_stringbuf_t **log_accum,
@@ -170,14 +174,17 @@ svn_wc__loggy_del_tree_conflict(svn_stri
                                 apr_pool_t *pool);
 
 /* Remove any tree conflict on victim VICTIM_PATH from the directory entry
- * belonging to ADM_ACCESS. (If there is no such conflict recorded. do
+ * belonging to ADM_ACCESS. (If there is no such conflict recorded, do
  * nothing and return success.) ADM_ACCESS must be an access baton for the
  * parent directory of VICTIM_PATH.
  *
  * Warning: This function updates the entry on disk but not the cached entry
  * in ADM_ACCESS.
  *
- * Do all allocations in POOL. */
+ * Do all allocations in POOL.
+ *
+ * @since New in 1.6.
+ */
 svn_error_t *
 svn_wc__del_tree_conflict(const char *victim_path,
                           svn_wc_adm_access_t *adm_access,
@@ -203,6 +210,8 @@ svn_wc__read_tree_conflicts_from_entry(a
  * in CONFLICTS to DIR_ENTRY.
  *
  * This function is used in a unit test in tests/libsvn_wc.
+ *
+ * @since New in 1.6.
  */
 svn_error_t *
 svn_wc__write_tree_conflicts_to_entry(apr_array_header_t *conflicts,
@@ -214,6 +223,8 @@ svn_wc__write_tree_conflicts_to_entry(ap
  * conflicts) for a conflict with the given VICTIM_BASENAME.
  *
  * This function is used in a unit test in tests/libsvn_wc.
+ *
+ * @since New in 1.6.
  */
 svn_boolean_t
 svn_wc__tree_conflict_exists(apr_array_header_t *conflicts,

---------------------------------------------------------------------
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-11 05:26:36 CET

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