On Sat, May 1, 2010 at 22:42, Neels J Hofmeyr <ne...@elego.de> wrote:
> I have this patch in progress which tries to get rid of one {entry->deleted
> || (entry->schedule one of _delete, _replace)} use in harvest_committables().
>
> It seems to pass now (complete make check still running, will verify
> tomorrow), but it definitely needs polish. Even if this patch passes make
> check it's a sad twisty bastard.
>
> Thoughts welcome!
>
> ~Neels
>
>
>
> * subversion/include/private/svn_wc_private.h
> (svn_wc__node_is_status_deleted):
Just fix this function to check for obstructed_delete, too. I've
mentioned this before, but have forgotten to fix it. Do not introduce
a new function.
>
> * subversion/libsvn_client/commit_util.c
> (harvest_committables):
>
> * subversion/libsvn_wc/node.c
> (svn_wc__node_is_replaced):
Do not change this function. It was designed/written to *specifically*
mean svn_wc_schedule_replace. It should not be changed.
>
> * subversion/tests/cmdline/copy_tests.py
> (mixed_wc_to_url):
>
> --This line, and those below, will be ignored--
> conf: # dub:/home/neels/pat/.pat-base/config-default
> conf: http://archive.apache.org/dist/apr/apr-1.3.9.tar.bz2
> conf: http://archive.apache.org/dist/apr/apr-util-1.3.9.tar.bz2
> conf: http://www.sqlite.org/sqlite-3.6.22.tar.gz
> conf: http://www.webdav.org/neon/neon-0.29.3.tar.gz
> conf: fsfs
> conf: local
> Index: subversion/include/private/svn_wc_private.h
> ===================================================================
> --- subversion/include/private/svn_wc_private.h (revision 940121)
> +++ subversion/include/private/svn_wc_private.h (working copy)
> @@ -414,6 +414,19 @@ svn_wc__node_is_status_deleted(svn_boole
> apr_pool_t *scratch_pool);
>
> /**
> + * Set @a *is_obstructed_delete to TRUE if @a local_abspath is
> + * "obstructed_deleted", using @a wc_ctx. If @a local_abspath is not in the
> + * working copy, return @c SVN_ERR_WC_PATH_NOT_FOUND. Use @a scratch_pool
> for
> + * all temporary allocations.
> + */
> +svn_error_t *
> +svn_wc__node_is_status_deleted_or_obstructed_del(
> + svn_boolean_t *is_obstructed_delete,
> + svn_wc_context_t *wc_ctx,
> + const char *local_abspath,
> + apr_pool_t *scratch_pool);
> +
> +/**
> * Set @a *is_obstructed to whether @a local_abspath is obstructed, using
> * @a wc_ctx. If @a local_abspath is not in the working copy, return
> * @c SVN_ERR_WC_PATH_NOT_FOUND. Use @a scratch_pool for all temporary
> Index: subversion/libsvn_client/commit_util.c
> ===================================================================
> --- subversion/libsvn_client/commit_util.c (revision 940121)
> +++ subversion/libsvn_client/commit_util.c (working copy)
> @@ -493,6 +493,11 @@ harvest_committables(apr_hash_t *committ
>
> /* If the entry is deleted with "--keep-local", we can skip checking
> for conflicts inside. */
> + /* ### Since wc-ng won't store whether a node was deleted with --keep-local
> + ### or not, a 'svn delete --keep-local' should instead clear the
> + ### tree-conflicts found in the targets. This code here should simply
> + ### always check for tree-conflicts and assume ignorable conflicts to be
> + ### gone already. */
> if (entry->keep_local)
> SVN_ERR_ASSERT(entry->schedule == svn_wc_schedule_delete);
> else
> @@ -519,12 +524,23 @@ harvest_committables(apr_hash_t *committ
> - The entry is scheduled for deletion or replacement, which case
> we need to send a delete either way.
> */
> - if ((! adds_only)
> - && ((entry->deleted && entry->schedule == svn_wc_schedule_normal)
> - || (entry->schedule == svn_wc_schedule_delete)
> - || (entry->schedule == svn_wc_schedule_replace)))
> + if (! adds_only)
> {
> - state_flags |= SVN_CLIENT_COMMIT_ITEM_DELETE;
> + svn_boolean_t is_replaced;
> + svn_boolean_t is_status_deleted;
> + svn_boolean_t is_present;
> +
> + /* ### There is room for optimization here, but let's keep it plain
> + * while this function is in flux. */
> + SVN_ERR(svn_wc__node_is_status_deleted_or_obstructed_del(
> + &is_status_deleted, ctx->wc_ctx, local_abspath,
> + scratch_pool));
> + SVN_ERR(svn_wc__node_is_replaced(&is_replaced, ctx->wc_ctx,
> + local_abspath, scratch_pool));
> + SVN_ERR(svn_wc__node_is_status_present(&is_present, ctx->wc_ctx,
> + local_abspath, scratch_pool));
> + if (is_status_deleted || is_replaced || ! is_present)
> + state_flags |= SVN_CLIENT_COMMIT_ITEM_DELETE;
> }
>
> /* Check for the trivial addition case. Adds can be explicit
> Index: subversion/libsvn_wc/node.c
> ===================================================================
> --- subversion/libsvn_wc/node.c (revision 940121)
> +++ subversion/libsvn_wc/node.c (working copy)
> @@ -675,6 +675,28 @@ svn_wc__node_is_status_deleted(svn_boole
> }
>
> svn_error_t *
> +svn_wc__node_is_status_deleted_or_obstructed_del(svn_boolean_t *is_deleted,
> + svn_wc_context_t *wc_ctx,
> + const char *local_abspath,
> + apr_pool_t *scratch_pool)
> +{
> + svn_wc__db_status_t status;
> +
> + SVN_ERR(svn_wc__db_read_info(&status,
> + NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL,
> + wc_ctx->db, local_abspath,
> + scratch_pool, scratch_pool));
> +
> + *is_deleted = (status == svn_wc__db_status_deleted) ||
> + (status == svn_wc__db_status_obstructed_delete);
> +
> + return SVN_NO_ERROR;
> +}
> +
> +svn_error_t *
> svn_wc__node_is_status_obstructed(svn_boolean_t *is_obstructed,
> svn_wc_context_t *wc_ctx,
> const char *local_abspath,
> @@ -803,9 +825,35 @@ svn_wc__node_is_replaced(svn_boolean_t *
> const char *local_abspath,
> apr_pool_t *scratch_pool)
> {
> - return svn_error_return(svn_wc__internal_is_replaced(replaced, wc_ctx->db,
> - local_abspath,
> - scratch_pool));
> + SVN_ERR(svn_wc__internal_is_replaced(replaced, wc_ctx->db, local_abspath,
> + scratch_pool));
> +
> + if (*replaced)
> + {
> + /* ### Catch a mixed-rev copy that replaces. The mixed-rev children are
> + * each regarded as op-roots of the replace and result in currently
> + * unexpected behaviour. Model wc-1 behaviour via
> + * svn_wc__node_get_copyfrom_info(). */
> + svn_wc__db_status_t status;
> +
> + SVN_ERR(svn_wc__db_scan_addition(&status, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL,
> + wc_ctx->db, local_abspath,
> + scratch_pool, scratch_pool));
> +
> + if (status == svn_wc__db_status_copied
> + || status == svn_wc__db_status_moved_here)
> + {
> + svn_boolean_t is_copy_target;
> +
> + SVN_ERR(svn_wc__node_get_copyfrom_info(NULL, NULL, &is_copy_target,
> + wc_ctx, local_abspath,
> + scratch_pool,
> scratch_pool));
> + if (! is_copy_target)
> + *replaced = FALSE;
> + }
> + }
Instead of this, have the commit function fetch the copyfrom/target
data. That stuff will be null if the node is not a copy.
>...
I also found a bug in your recent change to nde_get_copyfrom_info. You
should get the parent's status, ensure it is status_added (it can't be
obstructed_add, or the child info could not have been fetched!), and
then use scan_addition() to get the copyfrom data (IF
parent_original_abspath is NULL).
Basically, the requested node could have copyfrom info supporting a
mixed-rev copy, the parent could have NULL info (inherit), and the
grandparent could have copyfrom info.
Cheers,
-g
