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

Re: svn commit: r1021795 - introducing svn_wc_add_from_disk()

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Wed, 13 Oct 2010 12:16:34 +0100

On Tue, 2010-10-12, julianfoad_at_apache.org wrote:
> URL: http://svn.apache.org/viewvc?rev=1021795&view=rev
> Log:
> Introduce svn_wc_add_from_disk() as a new API to replace one task that was
> previously performed by svn_wc_add4(). There are no callers yet.

I updated the libsvn_client callers in r1021800, and those appear to be
fine.

When I update the svn_wc_add4() calls in
entries-compat.c:test_access_baton_like_locking(), the test fails.
Which is wrong, the test or my code?

Note that the test calls first svn_wc_add3() and then svn_wc_add4(). In
svn_wc_add_from_disk(), I intentionally omitted the "legacy locking"
code that still exists in svn_wc_add4() line 1033, where it says "If
using the legacy 1.6 interface ... add is expected to lock the new dir."

It sounded to me like that behaviour should not be required for WC-NG
callers.

I wonder if the test should be using svn_wc_add3() throughout, not
svn_wc_add4().

Anyone?

> * subversion/include/svn_wc.h
> (svn_wc_add_from_disk): New function.
>
> * subversion/libsvn_wc/adm_ops.c
> (add_from_disk): New function.
> (check_can_add_node): New function, extracted from svn_wc_add4.
> (svn_wc_add4): Use check_can_add_node() and add_from_disk().
> (svn_wc_add_from_disk): New function.

My intention is to remove the new-in-1.7 svn_wc_add4() from the public
API, and leave svn_wc_add3() deprecated, once we have alternative API
functions for the other tasks that it currently performs.

> Modified: subversion/trunk/subversion/include/svn_wc.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc.h?rev=1021795&r1=1021794&r2=1021795&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_wc.h (original)
> +++ subversion/trunk/subversion/include/svn_wc.h Tue Oct 12 15:17:21 2010
> @@ -4210,6 +4210,34 @@ svn_wc_delete(const char *path,
> apr_pool_t *pool);
>
> /**
> + * Schedule the node or tree that exists on disk at @a local_abspath for
> + * addition to the working copy, recursively. The added nodes will have
> + * no properties.
> + *
> + * The versioned state of the parent path must be a modifiable directory,
> + * and the versioned state of @a local_abspath must be either nonexistent or
> + * deleted; if deleted, the new node will be a replacement.
> + *
> + * If @a local_abspath does not exist as file, directory or symlink, return
> + * #SVN_ERR_WC_PATH_NOT_FOUND.
> + *
> + * This is equivalent to svn_wc_add4() case 2a.
> + *
> + * ### TODO: Recurse as far as a specified depth?
> + *
> + * ### A better API might allow the caller to walk a tree and add a single
> + * node at a time, specifying each node's properties.
> + */
> +svn_error_t *
> +svn_wc_add_from_disk(svn_wc_context_t *wc_ctx,
> + const char *local_abspath,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + svn_wc_notify_func2_t notify_func,
> + void *notify_baton,
> + apr_pool_t *scratch_pool);
> +
> +/**
> * Put @a local_abspath under version control by registering it as addition
> * or copy in the database containing its parent. The new node is scheduled
> * for addition to the repository below its parent node.
>
> Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=1021795&r1=1021794&r2=1021795&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue Oct 12 15:17:21 2010
> @@ -735,6 +735,28 @@ svn_wc_delete4(svn_wc_context_t *wc_ctx,
> return SVN_NO_ERROR;
> }
>
> +/* Schedule the single node at LOCAL_ABSPATH, of kind KIND, for addition in
> + * its parent directory in the WC. It will have no properties. */
> +static svn_error_t *
> +add_from_disk(svn_wc_context_t *wc_ctx,
> + const char *local_abspath,
> + svn_node_kind_t kind,
> + apr_pool_t *scratch_pool)
> +{
> + svn_wc__db_t *db = wc_ctx->db;
> +
> + if (kind == svn_node_file)
> + {
> + SVN_ERR(svn_wc__db_op_add_file(db, local_abspath, NULL, scratch_pool));
> + }
> + else
> + {
> + SVN_ERR(svn_wc__db_op_add_directory(db, local_abspath, NULL,
> + scratch_pool));
> + }
> + return SVN_NO_ERROR;
> +}
> +
> /* Set *REPOS_ROOT_URL and *REPOS_UUID to the repository of the parent of
> LOCAL_ABSPATH. REPOS_ROOT_URL and/or REPOS_UUID may be NULL if not
> wanted. Check that the parent of LOCAL_ABSPATH is a versioned directory
> @@ -812,20 +834,26 @@ check_can_add_to_parent(svn_wc__db_t *db
> return SVN_NO_ERROR;
> }
>
> -svn_error_t *
> -svn_wc_add4(svn_wc_context_t *wc_ctx,
> - const char *local_abspath,
> - svn_depth_t depth,
> - const char *copyfrom_url,
> - svn_revnum_t copyfrom_rev,
> - svn_cancel_func_t cancel_func,
> - void *cancel_baton,
> - svn_wc_notify_func2_t notify_func,
> - void *notify_baton,
> - apr_pool_t *scratch_pool)
> +/* Check that the on-disk item at LOCAL_ABSPATH can be scheduled for
> + * addition to its WC parent directory.
> + *
> + * Set *KIND_P to the kind of node to be added, *DB_ROW_EXISTS_P to whether
> + * it is already a versioned path, and if so, *IS_WC_ROOT_P to whether it's
> + * a WC root.
> + *
> + * ### The checks here, and the outputs, are geared towards svn_wc_add4().
> + */
> +static svn_error_t *
> +check_can_add_node(svn_node_kind_t *kind_p,
> + svn_boolean_t *db_row_exists_p,
> + svn_boolean_t *is_wc_root_p,
> + svn_wc_context_t *wc_ctx,
> + const char *local_abspath,
> + const char *copyfrom_url,
> + svn_revnum_t copyfrom_rev,
> + apr_pool_t *scratch_pool)
> {
> const char *base_name = svn_dirent_basename(local_abspath, scratch_pool);
> - const char *repos_root_url, *repos_uuid;
> svn_boolean_t is_wc_root;
> svn_node_kind_t kind;
> svn_wc__db_t *db = wc_ctx->db;
> @@ -845,7 +873,7 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
>
> SVN_ERR(svn_path_check_valid(local_abspath, scratch_pool));
>
> - /* Make sure something's there; set KIND. */
> + /* Make sure something's there; set KIND and *KIND_P. */
> SVN_ERR(svn_io_check_path(local_abspath, &kind, scratch_pool));
> if (kind == svn_node_none)
> return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> @@ -857,6 +885,8 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
> _("Unsupported node kind for path '%s'"),
> svn_dirent_local_style(local_abspath,
> scratch_pool));
> + if (kind_p)
> + *kind_p = kind;
>
> /* Determine whether a DB row for this node EXISTS, and whether it
> IS_WC_ROOT. If it exists, check that it is in an acceptable state for
> @@ -912,8 +942,37 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
> scratch_pool));
> }
> } /* err */
> +
> + if (db_row_exists_p)
> + *db_row_exists_p = exists;
> + if (is_wc_root_p)
> + *is_wc_root_p = is_wc_root;
> }
>
> + return SVN_NO_ERROR;
> +}
> +
> +svn_error_t *
> +svn_wc_add4(svn_wc_context_t *wc_ctx,
> + const char *local_abspath,
> + svn_depth_t depth,
> + const char *copyfrom_url,
> + svn_revnum_t copyfrom_rev,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + svn_wc_notify_func2_t notify_func,
> + void *notify_baton,
> + apr_pool_t *scratch_pool)
> +{
> + svn_wc__db_t *db = wc_ctx->db;
> + svn_node_kind_t kind;
> + svn_boolean_t db_row_exists, is_wc_root;
> + const char *repos_root_url, *repos_uuid;
> +
> + SVN_ERR(check_can_add_node(&kind, &db_row_exists, &is_wc_root,
> + wc_ctx, local_abspath, copyfrom_url, copyfrom_rev,
> + scratch_pool));
> +
> /* Get REPOS_ROOT_URL and REPOS_UUID. Check that the
> parent is a versioned directory in an acceptable state. */
> SVN_ERR(check_can_add_to_parent(db, &repos_root_url, &repos_uuid,
> @@ -970,26 +1029,20 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
>
> if (!copyfrom_url) /* Case 2a: It's a simple add */
> {
> - if (kind == svn_node_file)
> - SVN_ERR(svn_wc__db_op_add_file(db, local_abspath, NULL, scratch_pool));
> - else
> + SVN_ERR(add_from_disk(wc_ctx, local_abspath, kind, scratch_pool));
> + if (kind == svn_node_dir && !db_row_exists)
> {
> - SVN_ERR(svn_wc__db_op_add_directory(db, local_abspath, NULL,
> - scratch_pool));
> - if (!exists)
> - {
> - /* If using the legacy 1.6 interface the parent lock may not
> - be recursive and add is expected to lock the new dir.
> + /* If using the legacy 1.6 interface the parent lock may not
> + be recursive and add is expected to lock the new dir.

I maintained this functionality in the "old" function svn_wc_add4 (which

> - ### Perhaps the lock should be created in the same
> - transaction that adds the node? */
> - svn_boolean_t owns_lock;
> - SVN_ERR(svn_wc__db_wclock_owns_lock(&owns_lock, db, local_abspath,
> - FALSE, scratch_pool));
> - if (!owns_lock)
> - SVN_ERR(svn_wc__db_wclock_obtain(db, local_abspath, 0, FALSE,
> - scratch_pool));
> - }
> + ### Perhaps the lock should be created in the same
> + transaction that adds the node? */
> + svn_boolean_t owns_lock;
> + SVN_ERR(svn_wc__db_wclock_owns_lock(&owns_lock, db, local_abspath,
> + FALSE, scratch_pool));
> + if (!owns_lock)
> + SVN_ERR(svn_wc__db_wclock_obtain(db, local_abspath, 0, FALSE,
> + scratch_pool));
> }
> }
> else if (!is_wc_root) /* Case 2b: It's a copy from the repository */
> @@ -1087,6 +1140,37 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
> }
>
> svn_error_t *
> +svn_wc_add_from_disk(svn_wc_context_t *wc_ctx,
> + const char *local_abspath,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + svn_wc_notify_func2_t notify_func,
> + void *notify_baton,
> + apr_pool_t *scratch_pool)
> +{
> + svn_node_kind_t kind;
> +
> + SVN_ERR(check_can_add_node(&kind, NULL, NULL, wc_ctx, local_abspath,
> + NULL, SVN_INVALID_REVNUM, scratch_pool));
> + SVN_ERR(check_can_add_to_parent(wc_ctx->db, NULL, NULL,
> + local_abspath,
> + scratch_pool, scratch_pool));
> + SVN_ERR(add_from_disk(wc_ctx, local_abspath, kind, scratch_pool));
> +
> + /* Report the addition to the caller. */
> + if (notify_func != NULL)
> + {
> + svn_wc_notify_t *notify = svn_wc_create_notify(local_abspath,
> + svn_wc_notify_add,
> + scratch_pool);
> + notify->kind = kind;
> + (*notify_func)(notify_baton, notify, scratch_pool);
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
> +svn_error_t *
> svn_wc__register_file_external(svn_wc_context_t *wc_ctx,
> const char *local_abspath,
> const char *external_url,
>
>
Received on 2010-10-13 13:17:28 CEST

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.