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