On Thu, Nov 13, 2008 at 10:10:16PM -0800, neels_at_tigris.org wrote:
> Author: neels
> Date: Thu Nov 13 22:10:16 2008
> New Revision: 34186
>
> Log:
> Fix some behaviour around "resolved" and tree-conflicts.
>
> Take stsp's patch and fix it: do not try to read entries from the parent
> directory of a working copy root that was supplied as a target.
Thanks for fixing, there was quite a things I had missed (e.g.
checking whether 'entry' is really valid before passing it to
a function.
> Also fix missing resolving of deleted and committed files that became
> tree-conflict victims during a merge.
>
> * subversion/libsvn_client/resolved.c (svn_client_resolve),
> * subversion/libsvn_wc/adm_ops.c (resolve_found_entry_callback):
> Ensure we do not end up trying to find tree conflict info in the
> parent directory of the working copy root.
>
> * subversion/libsvn_wc/entries.c
> (visit_tc_too_found_entry): Also visit tree-conflicts of deleted items.
>
> * subversion/tests/cmdline/resolved_tests.py
> (resolved_on_wc_root, resolved_on_deleted_item):
> New tests file with two new tests for the issues fixed by this commit.
Yay, tests! Should have thought of adding a test for that myself...
One more question:
> Modified: trunk/subversion/libsvn_client/resolved.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/resolved.c?pathrev=34186&r1=34185&r2=34186
> ==============================================================================
> --- trunk/subversion/libsvn_client/resolved.c Thu Nov 13 15:03:26 2008 (r34185)
> +++ trunk/subversion/libsvn_client/resolved.c Thu Nov 13 22:10:16 2008 (r34186)
> @@ -53,19 +53,59 @@ 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_ERR(svn_wc_entry(&entry, path, adm_access, TRUE, pool));
> +
> + if (entry->kind == svn_node_dir)
> + {
> + svn_error_t *err;
> + svn_boolean_t switched;
> +
> + err = svn_wc__path_switched(path, &switched, entry, pool);
> +
> + if (err && (err->apr_err == SVN_ERR_ENTRY_MISSING_URL))
> + svn_error_clear(err);
Could you add a comment explaining what condition you are catching
by ignoring this error? In both places -- there's another instance of
this, see below. I guess it might have to do with unversioned
directories in the working copy, but I'm not sure if my guess is correct.
> + else
> + {
> + SVN_ERR(err);
> + 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,
>
> Modified: trunk/subversion/libsvn_wc/adm_ops.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/adm_ops.c?pathrev=34186&r1=34185&r2=34186
> ==============================================================================
> --- trunk/subversion/libsvn_wc/adm_ops.c Thu Nov 13 15:03:26 2008 (r34185)
> +++ trunk/subversion/libsvn_wc/adm_ops.c Thu Nov 13 22:10:16 2008 (r34186)
> @@ -2871,6 +2871,7 @@ resolve_found_entry_callback(const char
> {
> struct resolve_callback_baton *baton = walk_baton;
> svn_boolean_t resolved = FALSE;
> + svn_boolean_t wc_root = FALSE;
>
> /* 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,8 +2881,35 @@ resolve_found_entry_callback(const char
> return SVN_NO_ERROR;
>
>
> - /* If asked to, clear any tree conflict on the path. */
> - if (baton->resolve_tree)
> + /* Make sure we do not end up looking for tree conflict
> + * info above the working copy root. */
> +
> + if (entry && (entry->kind == svn_node_dir))
> + {
> + 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_error_t *err;
> + err = svn_wc__path_switched(path, &switched, entry, pool);
> + if (err && (err->apr_err == SVN_ERR_ENTRY_MISSING_URL))
> + svn_error_clear(err);
.... here.
Thanks,
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 12:54:10 CET