pburba@tigris.org writes:
> Modified:
> trunk/subversion/libsvn_wc/copy.c
> trunk/subversion/tests/cmdline/copy_tests.py
>
> Log:
> Support copying and moving of paths scheduled for addition.
>
> Follow-up to r20811.
>
> Suggested by: zhakov
>
> * subversion/libsvn_wc/copy.c
> (copy_added_file_administratively, copy_added_dir_administratively):
> New recursive helper functions for copying added paths and unversioned
> children within added directories.
> (svn_wc_copy2): Use new helper functions when copying added paths.
>
> * subversion/tests/cmdline/copy_tests.py
> (copy_move_added_paths, copy_added_paths_to_URL,): New tests.
> (test_list): Run new tests.
>
> Modified: trunk/subversion/libsvn_wc/copy.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/copy.c?pathrev=21152&r1=21151&r2=21152
> ==============================================================================
> --- trunk/subversion/libsvn_wc/copy.c (original)
> +++ trunk/subversion/libsvn_wc/copy.c Mon Aug 21 11:45:24 2006
> @@ -39,6 +39,228 @@
>
> /*** Code. ***/
>
> +/* Helper function for svn_wc_copy2() which handles WC->WC copying of
> + files which are scheduled for addition or unversioned.
> +
> + Copy file SRC_PATH to DST_BASENAME in DST_PARENT_ACCESS.
> +
> + DST_PARENT_ACCESS is a 0 depth locked access for a versioned directory
> + in the same WC as SRC_PATH.
> +
> + If SRC_IS_ADDED is true then SRC_PATH is scheduled for addition and
> + DST_BASENAME will also be scheduled for addition.
> +
> + If SRC_IS_ADDED is false then SRC_PATH is the unversioned child
> + file of a versioned or added parent and DST_BASENAME is simply copied.
> +
> + Use POOL for all necessary allocations.
> +*/
> +static svn_error_t *
> +copy_added_file_administratively(const char *src_path,
> + svn_boolean_t src_is_added,
> + svn_wc_adm_access_t *dst_parent_access,
> + const char *dst_basename,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + svn_wc_notify_func2_t notify_func,
> + void *notify_baton,
> + apr_pool_t *pool)
> +{
> + const char *dst_path
> + = svn_path_join(svn_wc_adm_access_path(dst_parent_access),
> + dst_basename, pool);
> +
> + /* Copy this file and possibly put it under version control. */
> + SVN_ERR(svn_io_copy_file(src_path, dst_path, TRUE, pool));
> +
> + if (src_is_added)
> + {
> + SVN_ERR(svn_wc_add2(dst_path, dst_parent_access, NULL,
> + SVN_INVALID_REVNUM, cancel_func,
> + cancel_baton, notify_func,
> + notify_baton, pool));
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
> +
> +/* Helper function for svn_wc_copy2() which handles WC->WC copying of
> + directories which are scheduled for addition or unversioned.
> +
> + Recursively copy directory SRC_PATH and its children, excluding
> + administrative directories, to DST_BASENAME in DST_PARENT_ACCESS.
> +
> + DST_PARENT_ACCESS is a 0 depth locked access for a versioned directory
> + in the same WC as SRC_PATH.
> +
> + SRC_ACCESS is a -1 depth access for SRC_PATH
> +
> + If SRC_IS_ADDED is true then SRC_PATH is scheduled for addition and
> + DST_BASENAME will also be scheduled for addition.
> +
> + If SRC_IS_ADDED is false then SRC_PATH is the unversioned child
> + directory of a versioned or added parent and DST_BASENAME is simply
> + copied.
> +
> + Use POOL for all necessary allocations.
> +*/
> +static svn_error_t *
> +copy_added_dir_administratively(const char *src_path,
> + svn_boolean_t src_is_added,
> + svn_wc_adm_access_t *dst_parent_access,
> + svn_wc_adm_access_t *src_access,
> + const char *dst_basename,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + svn_wc_notify_func2_t notify_func,
> + void *notify_baton,
> + apr_pool_t *pool)
> +{
> + /* The 'dst_path' is simply dst_parent/dst_basename */
> + const char *dst_path
> + = svn_path_join(svn_wc_adm_access_path(dst_parent_access),
> + dst_basename, pool);
> +
> + if (! src_is_added)
> + {
> + /* src_path is the top of an unversioned tree, just copy
> + the whole thing and we are done. */
> + SVN_ERR(svn_io_copy_dir_recursively(src_path,
> + svn_wc_adm_access_path(dst_parent_access),
> + dst_basename,
> + TRUE, cancel_func, cancel_baton,
> + pool));
> + }
> + else
> + {
> + char *src_parent, *dst_parent;
> + const svn_wc_entry_t *entry;
> + svn_wc_adm_access_t *dst_child_dir_access;
> + svn_wc_adm_access_t *src_child_dir_access;
> + apr_dir_t *dir;
> + apr_finfo_t this_entry;
> + svn_error_t *err;
> + apr_pool_t *subpool;
> + apr_int32_t flags = APR_FINFO_TYPE | APR_FINFO_NAME;
> +
> + /* Check cancellation; note that this catches recursive calls too. */
> + if (cancel_func)
> + SVN_ERR(cancel_func(cancel_baton));
> +
> + src_parent = svn_path_dirname(src_path, pool);
> + dst_parent = svn_path_dirname(dst_path, pool);
> +
> + /* "Copy" the dir dst_path and schedule it, and possibly
> + it's children, for addition. */
> + SVN_ERR(svn_io_dir_make(dst_path, APR_OS_DEFAULT, pool));
> +
> + /* Add the directory, adding locking access for dst_path
> + to dst_parent_access at the same time. */
> + SVN_ERR(svn_wc_add2(dst_path, dst_parent_access, NULL,
> + SVN_INVALID_REVNUM, cancel_func, cancel_baton,
> + notify_func, notify_baton, pool));
> +
> + /* Get the accesses for the newly added dir and its source, we'll
> + need both to process any of SRC_PATHS's children below. */
> + SVN_ERR(svn_wc_adm_retrieve(&dst_child_dir_access, dst_parent_access,
> + dst_path, pool));
> + SVN_ERR(svn_wc_adm_retrieve(&src_child_dir_access, src_access,
> + src_path, pool));
> +
> + /* Create a subpool for iterative memory control. */
> + subpool = svn_pool_create(pool);
> +
> + /* Read src_path's entries one by one. */
> + SVN_ERR(svn_io_dir_open(&dir, src_path, pool));
> + for (err = svn_io_dir_read(&this_entry, flags, dir, subpool);
> + err == SVN_NO_ERROR;
> + err = svn_io_dir_read(&this_entry, flags, dir, subpool))
> + {
It might be better to use either this:
while((err = svn_io_read_dir(...)) == SVN_NO_ERROR)
{
}
if (err)
{ ... both cases of error-handling outside the loop ...
}
or this:
while(1)
{
err = (svn_io_dir_read(...));
if (err)
{ ... both cases of error-handling inside the loop ...
}
}
(Probably you were just following the example in libsvn_client/add.c,
which I also think could be improved in the same way :-). )
Whether the error-handling is inside or outside the loop is not
important (to me, anyway). But in the for-vs-while question, I think
'while' is better because it doesn't duplicate the calls and doesn't
make the reader look twice.
> + const char *src_fullpath, *dst_fullpath;
> +
> + /* Skip entries for this dir and its parent. */
> + if (this_entry.name[0] == '.'
> + && (this_entry.name[1] == '\0'
> + || (this_entry.name[1] == '.'
> + && this_entry.name[2] == '\0')))
> + continue;
> +
> + /* Check cancellation so you can cancel during an
> + * add of a directory with lots of files. */
> + if (cancel_func)
> + SVN_ERR(cancel_func(cancel_baton));
> +
> + /* Skip over SVN admin directories. */
> + if (svn_wc_is_adm_dir(this_entry.name, subpool))
> + continue;
> +
> + /* Construct the full path of the entry. */
> + src_fullpath = svn_path_join(src_path, this_entry.name, subpool);
> + dst_fullpath = svn_path_join(dst_path, this_entry.name, subpool);
> +
> + SVN_ERR(svn_wc_entry(&entry, src_fullpath, src_child_dir_access,
> + TRUE, subpool));
> +
> + /* Recurse on directories; add files; ignore the rest. */
> + if (this_entry.filetype == APR_DIR)
> + {
> + SVN_ERR(copy_added_dir_administratively(src_fullpath,
> + entry ? TRUE : FALSE,
> + dst_child_dir_access,
> + src_child_dir_access,
> + this_entry.name,
> + cancel_func,
> + cancel_baton,
> + notify_func,
> + notify_baton,
> + subpool));
> + }
> + else if (this_entry.filetype != APR_UNKFILE)
> + {
> + SVN_ERR(copy_added_file_administratively(src_fullpath,
> + entry ? TRUE : FALSE,
> + dst_child_dir_access,
> + this_entry.name,
> + cancel_func,
> + cancel_baton,
> + notify_func,
> + notify_baton,
> + subpool));
> + }
> +
> + /* Clean out the per-iteration pool. */
> + svn_pool_clear(subpool);
> +
> + } /* End for loop */
Yeah, I guess we can't do the clear-pool-at-top-of-loop thing here
because this_entry is allocated in subpool. Oh well.
> + /* Check that the loop exited cleanly. */
> + if (! (APR_STATUS_IS_ENOENT(err->apr_err)))
> + {
> + return svn_error_createf(err->apr_err, err,
> + _("Error during recursive copy of '%s'"),
> + svn_path_local_style(src_path,
> + subpool));
> + }
> + else /* Yes, it exited cleanly, so close the dir. */
> + {
> + apr_status_t apr_err;
> +
> + svn_error_clear(err);
> + apr_err = apr_dir_close(dir);
> + if (apr_err)
> + return svn_error_wrap_apr(apr_err,
> + _("Can't close directory '%s'"),
> + svn_path_local_style(src_path,
> + subpool));
> + }
Hmm, why still use subpool here? Instead, shouldn't subpool be
destroyed immediately after the loop? (You don't need it for anything
in the err-handling code that follows -- 'dir' is allocated in pool,
not subpool, and err is allocated in its own pool as usual.)
> + } /* End else src_is_added. */
> +
> + return SVN_NO_ERROR;
> +}
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Aug 22 09:09:04 2006