You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Paul Burba <pa...@softlanding.com> on 2006/08/08 15:57:55 UTC
[PATCH] Moving/copying added files and dirs
Paul Burba <pa...@softlanding.com> wrote on 07/27/2006 08:37:31 AM:
> Ivan,
>
> Sorry, I wasn't able to look at this until yesterday afternoon. I'm
> afraid it's not as simple as defeating sanity check 2 (i.e. in
> copy_file_administratively(): /* Sanity check 2: You cannot make a copy
of
> something that's not in the repository unless it's a copy of an
> uncommitted copy. */). Here are some of the problems I see:
>
> 1) An added file has no text base, so when copy_file_administratively()
> tries to copy the pristine text base it fails.
>
> 2) If we are going to support copying an added file, it only makes sense
> (?) to support the following as well:
>
> a) moving an added file
> b) copying an added directory
> c) moving an added directory
>
> This isn't so much a problem, as an added complication.
>
> 3) If we move an added file how do we get rid of the add entry in the
> source directory? It's not like moving a versioned item, we don't want
> any evidence left behind in entries that something once existed there
and
> was scheduled for addition...do we? I suppose the equivalent of svn rm
> --force will work...
>
> Anyhow, I'm looking into this right now, hope to have some ideas
shortly.
...and here here it is. This patch supports copying and moving of added
paths within a WC or from a WC to the REPOS.
Re issue 1 above, after looking at this I think the solution shouldn't
reside in libsvn_wc, but rather in libsvn_client. Trying to teach
svn_wc_copy2() to copy paths that are not versioned doesn't make a lot of
sense. It seems all we really need to do is copy the added files without
the administrative directories and then add them. The first task sounds
an awful lot like an export, and with some borrowed/modified code from
copy_versioned_files() in export.c that's what this patch does. Then a
call to svn_client_add3() adds the copied files. Does this approach sound
reasonable?
Also, there is one obvious oddity regarding my solution, that is the
notification of 'A'dded 'D'eleted paths. Normally svn copy reports 'A'dds
first then 'D'eletes, but this patch reports in the opposite order when
moving/copying an added path (i.e. the Deletes are reported first). It's
certainly possible to make tweak wc_to_wc_copy() to make the ordering the
same, but it would require a bit more cumbersome logic and require
svn_wc_adm_access_t *adm_access/src_access to be opened and closed twice,
and I wasn't sure this is worth it(?).
Let me know your thoughts,
Paul B.
[[[
Support copy and move of paths scheduled for addition.
Follow-up to r20811.
Suggested by: zhakov
* subversion/libsvn_client/copy.c
(svn_pools.h): Include.
(copy_added_path): Helper function for wc_to_wc_copy() which copies
paths scheduled for addition to a new WC location.
(wc_to_wc_copy): Add new argument indicating if src_path is not under
version control but is scheduled for addition. Use copy_added_path()
rather than svn_wc_copy2() when copying added paths, then add these
with svn_client_add3().
(setup_copy): Determine if we are copying a path that is scheduled
for addition by looking at it's revision and pass that info to
wc_to_wc_copy().
* subversion/tests/cmdline/copy_tests.py
(copy_move_added_paths, copy_added_paths_to_URL): New tests.
(test_list): Run new tests.
]]]
Re: [PATCH] Moving/copying added files and dirs
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 8/8/06, Paul Burba <pa...@softlanding.com> wrote:
> ...and here here it is. This patch supports copying and moving of added
> paths within a WC or from a WC to the REPOS.
>
> Re issue 1 above, after looking at this I think the solution shouldn't
> reside in libsvn_wc, but rather in libsvn_client. Trying to teach
> svn_wc_copy2() to copy paths that are not versioned doesn't make a lot of
> sense. It seems all we really need to do is copy the added files without
> the administrative directories and then add them. The first task sounds
> an awful lot like an export, and with some borrowed/modified code from
> copy_versioned_files() in export.c that's what this patch does. Then a
> call to svn_client_add3() adds the copied files. Does this approach sound
> reasonable?
Works for me.
> Also, there is one obvious oddity regarding my solution, that is the
> notification of 'A'dded 'D'eleted paths. Normally svn copy reports 'A'dds
> first then 'D'eletes, but this patch reports in the opposite order when
> moving/copying an added path (i.e. the Deletes are reported first). It's
> certainly possible to make tweak wc_to_wc_copy() to make the ordering the
> same, but it would require a bit more cumbersome logic and require
> svn_wc_adm_access_t *adm_access/src_access to be opened and closed twice,
> and I wasn't sure this is worth it(?).
>
> Let me know your thoughts,
The patch seems reasonable, but I'm getting some test failures,
specifically the new tests are failing because the order of the lines
in the output isn't matching up. Perhaps we're relying on the order
of a hash someplace?
Anyway, other than that the change itself looks great.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Moving/copying added files and dirs
Posted by Paul Burba <pa...@softlanding.com>.
"Ivan Zhakov" <ch...@gmail.com> wrote on 08/21/2006 12:42:37 PM:
> On 8/21/06, Paul Burba <pa...@softlanding.com> wrote:
> > Ok, I think this is about ready if you think these latest changes look
ok.
> Yes, I think the same way. I'm +1 for commit.
Thanks Ivan, I appreciate all the code review help.
Comitted r21152.
Paul B.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Moving/copying added files and dirs
Posted by Ivan Zhakov <ch...@gmail.com>.
On 8/21/06, Paul Burba <pa...@softlanding.com> wrote:
> "Ivan Zhakov" <ch...@gmail.com> wrote on 08/19/2006 03:07:15 AM:
>
> > There are some small glitches, see my comments inside.
> >
> > Index: subversion/libsvn_wc/copy.c
> > ===================================================================
> > --- subversion/libsvn_wc/copy.c (revision 21115)
> > +++ subversion/libsvn_wc/copy.c (working copy)
> > @@ -39,6 +39,242 @@
> >
> > /*** Code. ***/
> >
> > +/* Helper function for svn_wc_copy2() which handles WC->WC copying of
> > + files which are scheduled for addition or unversioned.
> > +
> > + Copy SRC_PATH to DST_PATH.
> > +
> > + DST_PATH is a non-existant path, but it's parent must exist
> > + and be in the same WC as SRC_PATH.
> > +
> > + If SRC_IS_ADDED is true then SRC_PATH is scheduled for addition and
> > + DST_PATH 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_PATH is simply copied.
> > +
> > + DST_PARENT_ACCESS is a 0 depth locked access for the
> > + versioned parent dir of DST_PATH.
> > +
> > + Use POOL for all necessary allocations.
> > +*/
> > +static svn_error_t *
> > +copy_added_file_administratively(const char *src_path,
> > + const char *dst_path,
> > + svn_boolean_t src_is_added,
> > + svn_wc_adm_access_t
> *dst_parent_access,
> > + svn_cancel_func_t cancel_func,
> > + void *cancel_baton,
> > + svn_wc_notify_func2_t notify_func,
> > + void *notify_baton,
> > + apr_pool_t *pool)
> > +{
> > + char *src_parent = svn_path_dirname(src_path, pool);
> >
> > Btw, what do you think about split dst_path parameter to dst_parent
> > and dst_basename in parameters in
> > copy_added_file_administratively/copy_added_dir_administratively?
> > Because you are joining it when calling function and further splitting
> > it function body. It also will be more coordinated with
> > copy_file_administratively/copy_dir_administratively.
>
> I changed dst_path to dst_basename as you suggested to avoid the redundant
> join/split. But adding dst_parent seems unecessary, since
> dst_parent_access already has the path dst_parent readily
> available..
Ok, I've missed this.
> Unless you meant that svn_wc_adm_access_t *dst_parent_access
> should instead be named dst_parent(?). If so I really prefer the name
> with the "_access" suffix as it makes the code easier to read when
> variables representing path names are easily distinguished from
> svn_wc_adm_access_t pointers.
I'm prefer name without "_access" suffix as it present in other
functions, but I keep this to you.
> I also added the argument svn_wc_adm_access_t *src_access to
> copy_added_dir_administratively() since svn_wc_copy2() already has a -1
> unlocked access and there is no reason for me to open/close another
> access, we can simply retrieve the access out of src_access.
>
> Comments for both helper functions updated.
>
[snip
> Ok, I think this is about ready if you think these latest changes look ok.
Yes, I think the same way. I'm +1 for commit.
>
> Paul B.
>
> [[[
> Support copy and move 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.
> ]]]
>
>
>
>
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Moving/copying added files and dirs
Posted by Paul Burba <pa...@softlanding.com>.
"Ivan Zhakov" <ch...@gmail.com> wrote on 08/19/2006 03:07:15 AM:
> There are some small glitches, see my comments inside.
>
> Index: subversion/libsvn_wc/copy.c
> ===================================================================
> --- subversion/libsvn_wc/copy.c (revision 21115)
> +++ subversion/libsvn_wc/copy.c (working copy)
> @@ -39,6 +39,242 @@
>
> /*** Code. ***/
>
> +/* Helper function for svn_wc_copy2() which handles WC->WC copying of
> + files which are scheduled for addition or unversioned.
> +
> + Copy SRC_PATH to DST_PATH.
> +
> + DST_PATH is a non-existant path, but it's parent must exist
> + and be in the same WC as SRC_PATH.
> +
> + If SRC_IS_ADDED is true then SRC_PATH is scheduled for addition and
> + DST_PATH 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_PATH is simply copied.
> +
> + DST_PARENT_ACCESS is a 0 depth locked access for the
> + versioned parent dir of DST_PATH.
> +
> + Use POOL for all necessary allocations.
> +*/
> +static svn_error_t *
> +copy_added_file_administratively(const char *src_path,
> + const char *dst_path,
> + svn_boolean_t src_is_added,
> + svn_wc_adm_access_t
*dst_parent_access,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + svn_wc_notify_func2_t notify_func,
> + void *notify_baton,
> + apr_pool_t *pool)
> +{
> + char *src_parent = svn_path_dirname(src_path, pool);
>
> Btw, what do you think about split dst_path parameter to dst_parent
> and dst_basename in parameters in
> copy_added_file_administratively/copy_added_dir_administratively?
> Because you are joining it when calling function and further splitting
> it function body. It also will be more coordinated with
> copy_file_administratively/copy_dir_administratively.
I changed dst_path to dst_basename as you suggested to avoid the redundant
join/split. But adding dst_parent seems unecessary, since
dst_parent_access already has the path dst_parent readily
available...Unless you meant that svn_wc_adm_access_t *dst_parent_access
should instead be named dst_parent(?). If so I really prefer the name
with the "_access" suffix as it makes the code easier to read when
variables representing path names are easily distinguished from
svn_wc_adm_access_t pointers. Or possibly I'm misundertanding you
completely :-(
I also added the argument svn_wc_adm_access_t *src_access to
copy_added_dir_administratively() since svn_wc_copy2() already has a -1
unlocked access and there is no reason for me to open/close another
access, we can simply retrieve the access out of src_access.
Comments for both helper functions updated.
> + /* Copy this file and possibly put it under version control. */
> + SVN_ERR(svn_io_copy_file(src_path, dst_path, TRUE, pool));
> +
> Trailing spaces.
Removed.
> + 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.
> +
> + Copy SRC_PATH to DST_PATH.
> +
> + DST_PATH is a non-existant path, but it's parent must exist
> + and be in the same WC as SRC_PATH.
> +
> + If SRC_IS_ADDED is true then SRC_PATH is scheduled for addition and
> + DST_PATH 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_PATH is simply
copied.
> +
> + DST_PARENT_ACCESS is a 0 depth locked access for the
> + versioned parent dir of DST_PATH.
> +
> + Use POOL for all necessary allocations.
> +*/
> +static svn_error_t *
> +copy_added_dir_administratively(const char *src_path,
> + const char *dst_path,
> + svn_boolean_t src_is_added,
> + svn_wc_adm_access_t *dst_parent_access,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + svn_wc_notify_func2_t notify_func,
> + void *notify_baton,
> + apr_pool_t *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_path_dirname(dst_path,
pool),
> + svn_path_basename(dst_path,
pool),
> + TRUE, cancel_func,
cancel_baton,
> + pool));
> + }
> + else
> + {
> + char *src_parent, *dst_parent;
> + const svn_wc_entry_t *entry;
> + svn_wc_adm_access_t *src_access;
> + 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);
> +
> + /* Get a svn_wc_adm_access_t for src_path. */
> + if (strcmp(src_parent, dst_parent) == 0)
> + {
> + src_access = dst_parent_access;
> + }
> + else
> + {
> + SVN_ERR(svn_wc_adm_open3(&src_access, NULL, src_parent,
FALSE, 0,
> + cancel_func, cancel_baton,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));
> +
> Trailing spaces. (I hate this stuff too :)
Removed.
> + SVN_ERR(svn_wc_adm_open3(&src_child_dir_access, NULL, src_path,
FALSE,
> + 0, cancel_func, cancel_baton, 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 access for the newly added dir, we'll need it if we
> + recurse or call copy_added_file_administratively(). */
> + SVN_ERR(svn_wc_adm_retrieve(&dst_child_dir_access,
dst_parent_access,
> + dst_path, pool));
> Formatting glitch.
Fixed.
> +
> + /* 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))
> + {
> + 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));
> + src_is_added = entry ? TRUE : FALSE;
> I think this should be deleted, because it's never used further.
Removed.
Ok, I think this is about ready if you think these latest changes look ok.
Paul B.
[[[
Support copy and move 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.
]]]
Re: [PATCH] Moving/copying added files and dirs
Posted by Ivan Zhakov <ch...@gmail.com>.
On 8/18/06, Paul Burba <pa...@softlanding.com> wrote:
> "Ivan Zhakov" <ch...@gmail.com> wrote on 08/17/2006 05:40:01 PM:
>
> > On 8/18/06, Paul Burba <pa...@softlanding.com> wrote:
> > > Take a look if you have some time, thanks,
>
> > Good work, Paul. Your patch looks good, but I have some thoughts:
> >
> > 1. Did you try split copy_added_path_administratively to
> > copy_added_file_administratively/copy_added_dir_administratively()?
>
> No...
>
> > Because copy_added_path_administratively() have separated
> > implementation for each case. I think it will be reasonable to split
> > it.
>
> ...but it's easy enough to do so.
>
> > 2. Might good idea to add src_is_versioned parameter to
> > copy_added_path_administratively (or
> > copy_added_file_administratively/copy_added_dir_administratively),
> > because caller always know this and there is no reason to duplicate
> > stat calls.
>
> In addition to splitting the function I added the new parameter as you
> suggested. I renamed it from src_is_versioned to src_is_added, since we
> are really dealing with src_paths that are either scheduled for addition
> or unversioned. My original name implies that scheduled for addition ==
> versioned.
Ok.
>
> > 3. There is duplication of code at:
> > + else if (this_entry.filetype != APR_UNKFILE)
> > + {
> > + /* Copy the file and add it to version control if the
> > + source of the copy is versioned. */
> > + SVN_ERR(svn_io_copy_file(src_fullpath, dst_fullpath,
> TRUE,
> > + subpool));
> > This code is similiar to what is done at top of
> > copy_added_path_administratively. I think if you implement (1) and (2)
> > you can just call copy_added_file_administratively here.
>
> In splitting the function I eliminated the code at the top, but in the new
> function copy_added_dir_administratively() we still need to check the type
> and versioned/add status of each *child* of src_path before recursing or
> calling copy_added_file_administratively().
Ok.
[snip]
>
> > 5.
> > + 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));
> > + }
> > Btw, why we have not svn_io_dir_close() ??
>
> I'm not sure why we don't have it, probably just due to a lack of need
> outside of io.c. The only call to svn_io_dir_open() outside of io.c is in
> add.c. I can fix that once this patch is in.
>
Ok, I think better to factor out this code in separated patch.
> Thanks, let me know if you see any other problems.
There are some small glitches, see my comments inside.
Index: subversion/libsvn_wc/copy.c
===================================================================
--- subversion/libsvn_wc/copy.c (revision 21115)
+++ subversion/libsvn_wc/copy.c (working copy)
@@ -39,6 +39,242 @@
/*** Code. ***/
+/* Helper function for svn_wc_copy2() which handles WC->WC copying of
+ files which are scheduled for addition or unversioned.
+
+ Copy SRC_PATH to DST_PATH.
+
+ DST_PATH is a non-existant path, but it's parent must exist
+ and be in the same WC as SRC_PATH.
+
+ If SRC_IS_ADDED is true then SRC_PATH is scheduled for addition and
+ DST_PATH 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_PATH is simply copied.
+
+ DST_PARENT_ACCESS is a 0 depth locked access for the
+ versioned parent dir of DST_PATH.
+
+ Use POOL for all necessary allocations.
+*/
+static svn_error_t *
+copy_added_file_administratively(const char *src_path,
+ const char *dst_path,
+ svn_boolean_t src_is_added,
+ svn_wc_adm_access_t *dst_parent_access,
+ svn_cancel_func_t cancel_func,
+ void *cancel_baton,
+ svn_wc_notify_func2_t notify_func,
+ void *notify_baton,
+ apr_pool_t *pool)
+{
+ char *src_parent = svn_path_dirname(src_path, pool);
Btw, what do you think about split dst_path parameter to dst_parent
and dst_basename in parameters in
copy_added_file_administratively/copy_added_dir_administratively?
Because you are joining it when calling function and further splitting
it function body. It also will be more coordinated with
copy_file_administratively/copy_dir_administratively.
+
+ /* Copy this file and possibly put it under version control. */
+ SVN_ERR(svn_io_copy_file(src_path, dst_path, TRUE, pool));
+
Trailing spaces.
+ 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.
+
+ Copy SRC_PATH to DST_PATH.
+
+ DST_PATH is a non-existant path, but it's parent must exist
+ and be in the same WC as SRC_PATH.
+
+ If SRC_IS_ADDED is true then SRC_PATH is scheduled for addition and
+ DST_PATH 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_PATH is simply copied.
+
+ DST_PARENT_ACCESS is a 0 depth locked access for the
+ versioned parent dir of DST_PATH.
+
+ Use POOL for all necessary allocations.
+*/
+static svn_error_t *
+copy_added_dir_administratively(const char *src_path,
+ const char *dst_path,
+ svn_boolean_t src_is_added,
+ svn_wc_adm_access_t *dst_parent_access,
+ svn_cancel_func_t cancel_func,
+ void *cancel_baton,
+ svn_wc_notify_func2_t notify_func,
+ void *notify_baton,
+ apr_pool_t *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_path_dirname(dst_path, pool),
+ svn_path_basename(dst_path, pool),
+ TRUE, cancel_func, cancel_baton,
+ pool));
+ }
+ else
+ {
+ char *src_parent, *dst_parent;
+ const svn_wc_entry_t *entry;
+ svn_wc_adm_access_t *src_access;
+ 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);
+
+ /* Get a svn_wc_adm_access_t for src_path. */
+ if (strcmp(src_parent, dst_parent) == 0)
+ {
+ src_access = dst_parent_access;
+ }
+ else
+ {
+ SVN_ERR(svn_wc_adm_open3(&src_access, NULL, src_parent, FALSE, 0,
+ cancel_func, cancel_baton,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));
+
Trailing spaces. (I hate this stuff too :)
+ SVN_ERR(svn_wc_adm_open3(&src_child_dir_access, NULL, src_path, FALSE,
+ 0, cancel_func, cancel_baton, 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 access for the newly added dir, we'll need it if we
+ recurse or call copy_added_file_administratively(). */
+ SVN_ERR(svn_wc_adm_retrieve(&dst_child_dir_access, dst_parent_access,
+ dst_path, pool));
Formatting glitch.
+
+ /* 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))
+ {
+ 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));
+ src_is_added = entry ? TRUE : FALSE;
I think this should be deleted, because it's never used further.
+
+ /* Recurse on directories; add files; ignore the rest. */
+ if (this_entry.filetype == APR_DIR)
+ {
+ SVN_ERR(copy_added_dir_administratively(src_fullpath,
+ dst_fullpath,
+ entry ? TRUE : FALSE,
+ dst_child_dir_access,
+ cancel_func,
+ cancel_baton,
+ notify_func,
+ notify_baton,
+ subpool));
+ }
+ else if (this_entry.filetype != APR_UNKFILE)
+ {
+ SVN_ERR(copy_added_file_administratively(src_fullpath,
+ dst_fullpath,
+ entry ? TRUE : FALSE,
+ dst_child_dir_access,
+ cancel_func,
+ cancel_baton,
+ notify_func,
+ notify_baton,
+ subpool));
+ }
+
+ /* Clean out the per-iteration pool. */
+ svn_pool_clear(subpool);
+
+ } /* End for loop */
+
+ /* 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));
+ }
+
+ SVN_ERR(svn_wc_adm_close(src_child_dir_access));
+
+ if (src_access != dst_parent_access)
+ {
+ SVN_ERR(svn_wc_adm_close(src_access));
+ }
+
+ } /* End else src_is_added. */
+
+ return SVN_NO_ERROR;
+}
+
+
/* Helper function for copy_file_administratively() and
copy_dir_administratively(). Determines the COPYFROM_URL and
COPYFROM_REV of a file or directory SRC_PATH which is the descendant
@@ -588,15 +824,48 @@
SVN_ERR(svn_io_check_path(src_path, &src_kind, pool));
if (src_kind == svn_node_file)
- SVN_ERR(copy_file_administratively(src_path, adm_access,
- dst_parent, dst_basename,
- notify_func, notify_baton, pool));
-
+ {
+ if (src_entry->schedule == svn_wc_schedule_add
+ && (! src_entry->copied))
+ {
+ SVN_ERR(copy_added_file_administratively(src_path,
+ svn_path_join(dst_path,
+ dst_basename,
+ pool),
+ TRUE, dst_parent,
+ cancel_func, cancel_baton,
+ notify_func, notify_baton,
+ pool));
+ }
+ else
+ {
+ SVN_ERR(copy_file_administratively(src_path, adm_access,
+ dst_parent, dst_basename,
+ notify_func, notify_baton, pool));
+ }
+ }
else if (src_kind == svn_node_dir)
- SVN_ERR(copy_dir_administratively(src_path, adm_access,
- dst_parent, dst_basename,
- cancel_func, cancel_baton,
- notify_func, notify_baton, pool));
+ {
+ if (src_entry->schedule == svn_wc_schedule_add
+ && (! src_entry->copied))
+ {
+ SVN_ERR(copy_added_dir_administratively(src_path,
+ svn_path_join(dst_path,
+ dst_basename,
+ pool),
+ TRUE, dst_parent,
+ cancel_func, cancel_baton,
+ notify_func, notify_baton,
+ pool));
+ }
+ else
+ {
+ SVN_ERR(copy_dir_administratively(src_path, adm_access,
+ dst_parent, dst_basename,
+ cancel_func, cancel_baton,
+ notify_func, notify_baton, pool));
+ }
+ }
SVN_ERR(svn_wc_adm_close(adm_access));
Index: subversion/tests/cmdline/copy_tests.py
===================================================================
--- subversion/tests/cmdline/copy_tests.py (revision 21115)
+++ subversion/tests/cmdline/copy_tests.py (working copy)
[..]
>
> Paul B.
>
> [[[
> Support copy and move 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.
> ]]]
>
>
>
--
Ivan Zhakov
Re: [PATCH] Moving/copying added files and dirs
Posted by Paul Burba <pa...@softlanding.com>.
"Ivan Zhakov" <ch...@gmail.com> wrote on 08/17/2006 05:40:01 PM:
> On 8/18/06, Paul Burba <pa...@softlanding.com> wrote:
> > Take a look if you have some time, thanks,
> Good work, Paul. Your patch looks good, but I have some thoughts:
>
> 1. Did you try split copy_added_path_administratively to
> copy_added_file_administratively/copy_added_dir_administratively()?
No...
> Because copy_added_path_administratively() have separated
> implementation for each case. I think it will be reasonable to split
> it.
...but it's easy enough to do so.
> 2. Might good idea to add src_is_versioned parameter to
> copy_added_path_administratively (or
> copy_added_file_administratively/copy_added_dir_administratively),
> because caller always know this and there is no reason to duplicate
> stat calls.
In addition to splitting the function I added the new parameter as you
suggested. I renamed it from src_is_versioned to src_is_added, since we
are really dealing with src_paths that are either scheduled for addition
or unversioned. My original name implies that scheduled for addition ==
versioned.
> 3. There is duplication of code at:
> + else if (this_entry.filetype != APR_UNKFILE)
> + {
> + /* Copy the file and add it to version control if the
> + source of the copy is versioned. */
> + SVN_ERR(svn_io_copy_file(src_fullpath, dst_fullpath,
TRUE,
> + subpool));
> This code is similiar to what is done at top of
> copy_added_path_administratively. I think if you implement (1) and (2)
> you can just call copy_added_file_administratively here.
In splitting the function I eliminated the code at the top, but in the new
function copy_added_dir_administratively() we still need to check the type
and versioned/add status of each *child* of src_path before recursing or
calling copy_added_file_administratively().
> 4. Also at the same place:
> + else if (this_entry.filetype != APR_UNKFILE)
> + {
> + /* Copy the file and add it to version control if the
> + source of the copy is versioned. */
> + SVN_ERR(svn_io_copy_file(src_fullpath, dst_fullpath,
TRUE,
> + subpool));
> + SVN_ERR(svn_wc_entry(&entry,
> + svn_path_join(src_path,
> + this_entry.name,
> + pool),
> + src_child_dir_access, TRUE,
pool));
> I didn't see reasons to construct full source path again, because you
> have it in variable src_fullpath.
A mistake on my part, it's not necessary. Fixed.
> 5.
> + 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));
> + }
> Btw, why we have not svn_io_dir_close() ??
I'm not sure why we don't have it, probably just due to a lack of need
outside of io.c. The only call to svn_io_dir_open() outside of io.c is in
add.c. I can fix that once this patch is in.
Thanks, let me know if you see any other problems.
Paul B.
[[[
Support copy and move 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.
]]]
Re: [PATCH] Moving/copying added files and dirs
Posted by Ivan Zhakov <ch...@gmail.com>.
On 8/18/06, Paul Burba <pa...@softlanding.com> wrote:
> "Ivan Zhakov" <ch...@gmail.com> wrote on 08/14/2006 05:24:33 AM:
>
> > On 8/10/06, Paul Burba <pa...@softlanding.com> wrote:
> > > Paul Burba <pa...@softlanding.com> wrote on 08/09/2006 02:22:01 PM:
> > >
> > > Ivan,
> > >
> > > I took a second look at doing this in libsvn_wc rather than
> libsvn_client
> > > as you suggested. It doesn't take much to copy added files, but I'm
> > > getting nowhere fast when copying added directories. Is having
> similar
> > > svn add testing logic in libsvn_wc and libsvn_client really that
> > > "dangerous" as you say, why exactly?
> > >
> > I mean that it dangerous because introduce undocumented behavior
> > dependency between two layers (libsvn_client and libsvn_wc), which is
> > not right thing.
> > Just imagine: we have rewritten libsvn_wc using central database and
> > there is no problems to copy added files in this situation, but it
> > will require to change libsvn_client. This is violation of
> > incapsulation between layers.
>
> Hi Ivan,
>
> I understand not making code in libsvn_client dependent on implementation
> details in libsvn_wc, but I don't see that my original patch was doing
> that, it only made calls to libsvn_wc's public API. Short of libsvn_wc
> API changes, how was my patch dependent on libsvn_wc?
>
> Regardless, you might want to wait before answering that, it's a moot
> point! In my attempts to ensure unversioned paths also got copied, I
> encountered other problems which drove me once more to providing a
> solution entirely within libsvn_wc. It wasn't as difficult as I first
> thought...but what is once you've done it :-)
Ok, let's drop this moot topic and save our time :)
>
> This new patch provides a helper function for svn_wc_copy2() to go along
> with copy_file_administratively() and copy_file_administratively(). The
> new function, copy_added_path_administratively(), recursively walks the
> source working copy tree from the top, copying files and
> (non-administrative) dirs one at a time* and adding them via svn_wc_add2()
> if they are versioned in the source of the copy. As versioned dirs are
> copied, the destination path (locked) svn_wc_adm_access_t is expanded down
> one directory at a time.
>
> * Except when an unversioned dir is encountered, then it just recursively
> copies the whole thing.
>
> "Ivan Zhakov" <ch...@gmail.com> wrote on 08/08/2006 02:59:16 PM:
> > Now unversioned files in versioned commited directory copied when you
> > copy directory. I mean:
> > $ mkdir dir
> > $ svn add dir
> > $ svn ci -m ""
> > $ touch dir\foo
> > $ svn st dir
> > ? dir\foo
> >
> > $ svn cp dir dir2
> > $ svn st dir2
> > ? dir2\foo
> >
> > But your patch seems copy only versioned files, i.e.
> > $ mkdir dir
> > $ svn add dir
> > $ svn ci -m ""
> > $ touch dir\foo
> > $ svn st dir
> > A dir
> > ? dir\foo
> >
> > $ svn cp dir dir2
> > $ svn st dir2
> > A dir2
> >
> > So there is inconsistency. I consider it's bad.
>
> As mentioned above, the new patch copies/moves all unversioned files/dirs.
> The copy_move_added_paths test now checks that this is done correctly.
>
> Take a look if you have some time, thanks,
Good work, Paul. Your patch looks good, but I have some thoughts:
1. Did you try split copy_added_path_administratively to
copy_added_file_administratively/copy_added_dir_administratively()?
Because copy_added_path_administratively() have separated
implementation for each case. I think it will be reasonable to split
it.
2. Might good idea to add src_is_versioned parameter to
copy_added_path_administratively (or
copy_added_file_administratively/copy_added_dir_administratively),
because caller always know this and there is no reason to duplicate
stat calls.
3. There is duplication of code at:
+ else if (this_entry.filetype != APR_UNKFILE)
+ {
+ /* Copy the file and add it to version control if the
+ source of the copy is versioned. */
+ SVN_ERR(svn_io_copy_file(src_fullpath, dst_fullpath, TRUE,
+ subpool));
This code is similiar to what is done at top of
copy_added_path_administratively. I think if you implement (1) and (2)
you can just call copy_added_file_administratively here.
4. Also at the same place:
+ else if (this_entry.filetype != APR_UNKFILE)
+ {
+ /* Copy the file and add it to version control if the
+ source of the copy is versioned. */
+ SVN_ERR(svn_io_copy_file(src_fullpath, dst_fullpath, TRUE,
+ subpool));
+ SVN_ERR(svn_wc_entry(&entry,
+ svn_path_join(src_path,
+ this_entry.name,
+ pool),
+ src_child_dir_access, TRUE, pool));
I didn't see reasons to construct full source path again, because you
have it in variable src_fullpath.
5.
+ 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));
+ }
Btw, why we have not svn_io_dir_close() ??
>
> Paul B.
>
> [[[
> Support copy and move of paths scheduled for addition.
>
> Follow-up to r20811.
>
> Suggested by: zhakov
>
> * subversion/libsvn_wc/copy.c
> (copy_added_path_administratively): New recursive helper function which
> copies added paths, including unversioned paths within added
> directories.
> (svn_wc_copy2): Use new helper function 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.
> ]]]
>
>
>
>
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Moving/copying added files and dirs
Posted by Paul Burba <pa...@softlanding.com>.
"Ivan Zhakov" <ch...@gmail.com> wrote on 08/14/2006 05:24:33 AM:
> On 8/10/06, Paul Burba <pa...@softlanding.com> wrote:
> > Paul Burba <pa...@softlanding.com> wrote on 08/09/2006 02:22:01 PM:
> >
> > Ivan,
> >
> > I took a second look at doing this in libsvn_wc rather than
libsvn_client
> > as you suggested. It doesn't take much to copy added files, but I'm
> > getting nowhere fast when copying added directories. Is having
similar
> > svn add testing logic in libsvn_wc and libsvn_client really that
> > "dangerous" as you say, why exactly?
> >
> I mean that it dangerous because introduce undocumented behavior
> dependency between two layers (libsvn_client and libsvn_wc), which is
> not right thing.
> Just imagine: we have rewritten libsvn_wc using central database and
> there is no problems to copy added files in this situation, but it
> will require to change libsvn_client. This is violation of
> incapsulation between layers.
Hi Ivan,
I understand not making code in libsvn_client dependent on implementation
details in libsvn_wc, but I don't see that my original patch was doing
that, it only made calls to libsvn_wc's public API. Short of libsvn_wc
API changes, how was my patch dependent on libsvn_wc?
Regardless, you might want to wait before answering that, it's a moot
point! In my attempts to ensure unversioned paths also got copied, I
encountered other problems which drove me once more to providing a
solution entirely within libsvn_wc. It wasn't as difficult as I first
thought...but what is once you've done it :-)
This new patch provides a helper function for svn_wc_copy2() to go along
with copy_file_administratively() and copy_file_administratively(). The
new function, copy_added_path_administratively(), recursively walks the
source working copy tree from the top, copying files and
(non-administrative) dirs one at a time* and adding them via svn_wc_add2()
if they are versioned in the source of the copy. As versioned dirs are
copied, the destination path (locked) svn_wc_adm_access_t is expanded down
one directory at a time.
* Except when an unversioned dir is encountered, then it just recursively
copies the whole thing.
"Ivan Zhakov" <ch...@gmail.com> wrote on 08/08/2006 02:59:16 PM:
> Now unversioned files in versioned commited directory copied when you
> copy directory. I mean:
> $ mkdir dir
> $ svn add dir
> $ svn ci -m ""
> $ touch dir\foo
> $ svn st dir
> ? dir\foo
>
> $ svn cp dir dir2
> $ svn st dir2
> ? dir2\foo
>
> But your patch seems copy only versioned files, i.e.
> $ mkdir dir
> $ svn add dir
> $ svn ci -m ""
> $ touch dir\foo
> $ svn st dir
> A dir
> ? dir\foo
>
> $ svn cp dir dir2
> $ svn st dir2
> A dir2
>
> So there is inconsistency. I consider it's bad.
As mentioned above, the new patch copies/moves all unversioned files/dirs.
The copy_move_added_paths test now checks that this is done correctly.
Take a look if you have some time, thanks,
Paul B.
[[[
Support copy and move of paths scheduled for addition.
Follow-up to r20811.
Suggested by: zhakov
* subversion/libsvn_wc/copy.c
(copy_added_path_administratively): New recursive helper function which
copies added paths, including unversioned paths within added
directories.
(svn_wc_copy2): Use new helper function 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.
]]]
Re: [PATCH] Moving/copying added files and dirs
Posted by Ivan Zhakov <ch...@gmail.com>.
On 8/10/06, Paul Burba <pa...@softlanding.com> wrote:
> Paul Burba <pa...@softlanding.com> wrote on 08/09/2006 02:22:01 PM:
>
> > "Ivan Zhakov" <ch...@gmail.com> wrote on 08/09/2006 11:20:45 AM:
> >
> > > On 8/9/06, Paul Burba <pa...@softlanding.com> wrote:
> > > >
> > > > Hi All,
> > > >
> > > > Just to be clear on my goal in this chunk of code: it's to determine
>
> > if
> > > > the src_path arg to setup_copy is scheduled for addition as a result
>
> > of
> > > > svn add, as opposed to a WC->WC copy/move. We need to know this
> since
> > > > wc_to_wc_copy() handles these two cases differently. My comment is
> > > > unclear on this and I will improve it with something like this:
> > > >
> > > [..]
> > > > So we can't just look at the schedule value, it's add in both cases.
>
> > The
> > > > explanation of the revision field in
> > > > http://svn.collab.net/repos/svn/trunk/subversion/libsvn_wc/README
> is:
> > > >
> > > > revision:
> > > > The revision that the pristine text and properties of this entry
> > > > represent. Defaults to the revision of the this_dir entry, for
> > > > which it is required. Set to 0 for entries not yet in the
> > > > repository.
> > > >
> > > > Between this and the observed behavior above led me to think that
> > looking
> > > > at the rev is the way to differentiate between the two scenarios.
> The
> > > > docstring for svn_wc_add2() makes no promises regarding the revision
>
> > of
> > > > added paths, though it does explicitly set it to 0 on line 1184:
> > > >
> > > > tmp_entry.revision = 0;
> > > >
> > > > Maybe I'm making a dangerous assumption?
> > > >
> > > > Perhaps it's better to look at the schedule *and* copy fields to
> make
> > the
> > > > determination?
> > > >
> > > > if (!src_is_url)
> > > > {
> > > > /* Are we copying/moving a path that is scheduled for
> addition
> > > > as the result of svn add or svn mv/cp? */
> > > > SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, src_path,
> > FALSE,
> > > > 0,
> > > > ctx->cancel_func,
> > ctx->cancel_baton,
> > > > pool));
> > > > SVN_ERR(svn_wc_entry(&entry, src_path, adm_access, FALSE,
> > pool));
> > > > SVN_ERR(svn_wc_adm_close(adm_access));
> > > > /* If scheduled for addition but not copied, src_path is
> > > > result of svn add not svn cp/mv. */
> > > > src_is_add = (entry
> > > > && entry->schedule == svn_wc_schedule_add
> > > > && !(entry->copied))
> > > > ? TRUE : FALSE;
> > > > }
> > > >
> > > > Thoughts?
> > > >
> > > I consider that anyway condition should be the same that in
> > > copy_file_administratively() and copy_dir_administratively()
> >
> > I've taken a longer look at how entries are created and I'm fairly
> > confident that checking for revision == 0 in entries is a reliable way
> to
> > test for "svn added-ness". But using the entry's schedule and copied
> > fields as you suggest seems equally valid, so I'll defer to you on this
> > and use the latter.
> >
> > > But have
> > > same condition in several place is little bit dangerous,
> > > so might we should reconsider and move your code from libsvn_client to
> > > libsvn_wc?
> >
> > As I mentioned perviously in this thread, an added file has no text
> base,
> > so when copy_file_administratively() tries to copy the pristine text
> base
> > it fails. It's easy enough to work around that I guess, just use the
> > added file itself instead of the text-base. I recall some other
> problems,
> > but let me take a look again.
>
> Ivan,
>
> I took a second look at doing this in libsvn_wc rather than libsvn_client
> as you suggested. It doesn't take much to copy added files, but I'm
> getting nowhere fast when copying added directories. Is having similar
> svn add testing logic in libsvn_wc and libsvn_client really that
> "dangerous" as you say, why exactly?
>
I mean that it dangerous because introduce undocumented behavior
dependency between two layers (libsvn_client and libsvn_wc), which is
not right thing.
Just imagine: we have rewritten libsvn_wc using central database and
there is no problems to copy added files in this situation, but it
will require to change libsvn_client. This is violation of
incapsulation between layers.
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Moving/copying added files and dirs
Posted by Paul Burba <pa...@softlanding.com>.
Paul Burba <pa...@softlanding.com> wrote on 08/09/2006 02:22:01 PM:
> "Ivan Zhakov" <ch...@gmail.com> wrote on 08/09/2006 11:20:45 AM:
>
> > On 8/9/06, Paul Burba <pa...@softlanding.com> wrote:
> > >
> > > Hi All,
> > >
> > > Just to be clear on my goal in this chunk of code: it's to determine
> if
> > > the src_path arg to setup_copy is scheduled for addition as a result
> of
> > > svn add, as opposed to a WC->WC copy/move. We need to know this
since
> > > wc_to_wc_copy() handles these two cases differently. My comment is
> > > unclear on this and I will improve it with something like this:
> > >
> > [..]
> > > So we can't just look at the schedule value, it's add in both cases.
> The
> > > explanation of the revision field in
> > > http://svn.collab.net/repos/svn/trunk/subversion/libsvn_wc/README
is:
> > >
> > > revision:
> > > The revision that the pristine text and properties of this entry
> > > represent. Defaults to the revision of the this_dir entry, for
> > > which it is required. Set to 0 for entries not yet in the
> > > repository.
> > >
> > > Between this and the observed behavior above led me to think that
> looking
> > > at the rev is the way to differentiate between the two scenarios.
The
> > > docstring for svn_wc_add2() makes no promises regarding the revision
> of
> > > added paths, though it does explicitly set it to 0 on line 1184:
> > >
> > > tmp_entry.revision = 0;
> > >
> > > Maybe I'm making a dangerous assumption?
> > >
> > > Perhaps it's better to look at the schedule *and* copy fields to
make
> the
> > > determination?
> > >
> > > if (!src_is_url)
> > > {
> > > /* Are we copying/moving a path that is scheduled for
addition
> > > as the result of svn add or svn mv/cp? */
> > > SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, src_path,
> FALSE,
> > > 0,
> > > ctx->cancel_func,
> ctx->cancel_baton,
> > > pool));
> > > SVN_ERR(svn_wc_entry(&entry, src_path, adm_access, FALSE,
> pool));
> > > SVN_ERR(svn_wc_adm_close(adm_access));
> > > /* If scheduled for addition but not copied, src_path is
> > > result of svn add not svn cp/mv. */
> > > src_is_add = (entry
> > > && entry->schedule == svn_wc_schedule_add
> > > && !(entry->copied))
> > > ? TRUE : FALSE;
> > > }
> > >
> > > Thoughts?
> > >
> > I consider that anyway condition should be the same that in
> > copy_file_administratively() and copy_dir_administratively()
>
> I've taken a longer look at how entries are created and I'm fairly
> confident that checking for revision == 0 in entries is a reliable way
to
> test for "svn added-ness". But using the entry's schedule and copied
> fields as you suggest seems equally valid, so I'll defer to you on this
> and use the latter.
>
> > But have
> > same condition in several place is little bit dangerous,
> > so might we should reconsider and move your code from libsvn_client to
> > libsvn_wc?
>
> As I mentioned perviously in this thread, an added file has no text
base,
> so when copy_file_administratively() tries to copy the pristine text
base
> it fails. It's easy enough to work around that I guess, just use the
> added file itself instead of the text-base. I recall some other
problems,
> but let me take a look again.
Ivan,
I took a second look at doing this in libsvn_wc rather than libsvn_client
as you suggested. It doesn't take much to copy added files, but I'm
getting nowhere fast when copying added directories. Is having similar
svn add testing logic in libsvn_wc and libsvn_client really that
"dangerous" as you say, why exactly?
Paul B.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Moving/copying added files and dirs
Posted by Paul Burba <pa...@softlanding.com>.
"Ivan Zhakov" <ch...@gmail.com> wrote on 08/09/2006 11:20:45 AM:
> On 8/9/06, Paul Burba <pa...@softlanding.com> wrote:
> >
> > Hi All,
> >
> > Just to be clear on my goal in this chunk of code: it's to determine
if
> > the src_path arg to setup_copy is scheduled for addition as a result
of
> > svn add, as opposed to a WC->WC copy/move. We need to know this since
> > wc_to_wc_copy() handles these two cases differently. My comment is
> > unclear on this and I will improve it with something like this:
> >
> [..]
> > So we can't just look at the schedule value, it's add in both cases.
The
> > explanation of the revision field in
> > http://svn.collab.net/repos/svn/trunk/subversion/libsvn_wc/README is:
> >
> > revision:
> > The revision that the pristine text and properties of this entry
> > represent. Defaults to the revision of the this_dir entry, for
> > which it is required. Set to 0 for entries not yet in the
> > repository.
> >
> > Between this and the observed behavior above led me to think that
looking
> > at the rev is the way to differentiate between the two scenarios. The
> > docstring for svn_wc_add2() makes no promises regarding the revision
of
> > added paths, though it does explicitly set it to 0 on line 1184:
> >
> > tmp_entry.revision = 0;
> >
> > Maybe I'm making a dangerous assumption?
> >
> > Perhaps it's better to look at the schedule *and* copy fields to make
the
> > determination?
> >
> > if (!src_is_url)
> > {
> > /* Are we copying/moving a path that is scheduled for addition
> > as the result of svn add or svn mv/cp? */
> > SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, src_path,
FALSE,
> > 0,
> > ctx->cancel_func,
ctx->cancel_baton,
> > pool));
> > SVN_ERR(svn_wc_entry(&entry, src_path, adm_access, FALSE,
pool));
> > SVN_ERR(svn_wc_adm_close(adm_access));
> > /* If scheduled for addition but not copied, src_path is
> > result of svn add not svn cp/mv. */
> > src_is_add = (entry
> > && entry->schedule == svn_wc_schedule_add
> > && !(entry->copied))
> > ? TRUE : FALSE;
> > }
> >
> > Thoughts?
> >
> I consider that anyway condition should be the same that in
> copy_file_administratively() and copy_dir_administratively()
I've taken a longer look at how entries are created and I'm fairly
confident that checking for revision == 0 in entries is a reliable way to
test for "svn added-ness". But using the entry's schedule and copied
fields as you suggest seems equally valid, so I'll defer to you on this
and use the latter.
> But have
> same condition in several place is little bit dangerous,
> so might we should reconsider and move your code from libsvn_client to
> libsvn_wc?
As I mentioned perviously in this thread, an added file has no text base,
so when copy_file_administratively() tries to copy the pristine text base
it fails. It's easy enough to work around that I guess, just use the
added file itself instead of the text-base. I recall some other problems,
but let me take a look again.
> On 8/9/06, Paul Burba <pa...@softlanding.com> wrote:
> > Notice none of the unversioned children of H get moved, while only
A\D\I
> > gets copied.
> >
> > Also, a related oddity: While svn copy normally copies these first
> > generation unversioned children, svn move requires you to use --force
and
> > then it *deletes* all the unversioned items in the source and does
*not*
> > copy any of them...so much for "this subcommand is equivalent to a
'copy'
> > and 'delete'."
> >
> You are not right, they copied, but doesn't displayed in svn st -v.
> Just check directory D_move\I by hand.
Ouch, you're right, I was clearly not in my right mind when I wrote that
Sorry about that! :-\
Paul B.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Moving/copying added files and dirs
Posted by Ivan Zhakov <ch...@gmail.com>.
On 8/9/06, Paul Burba <pa...@softlanding.com> wrote:
>
> Hi All,
>
> Just to be clear on my goal in this chunk of code: it's to determine if
> the src_path arg to setup_copy is scheduled for addition as a result of
> svn add, as opposed to a WC->WC copy/move. We need to know this since
> wc_to_wc_copy() handles these two cases differently. My comment is
> unclear on this and I will improve it with something like this:
>
[..]
> So we can't just look at the schedule value, it's add in both cases. The
> explanation of the revision field in
> http://svn.collab.net/repos/svn/trunk/subversion/libsvn_wc/README is:
>
> revision:
> The revision that the pristine text and properties of this entry
> represent. Defaults to the revision of the this_dir entry, for
> which it is required. Set to 0 for entries not yet in the
> repository.
>
> Between this and the observed behavior above led me to think that looking
> at the rev is the way to differentiate between the two scenarios. The
> docstring for svn_wc_add2() makes no promises regarding the revision of
> added paths, though it does explicitly set it to 0 on line 1184:
>
> tmp_entry.revision = 0;
>
> Maybe I'm making a dangerous assumption?
>
> Perhaps it's better to look at the schedule *and* copy fields to make the
> determination?
>
> if (!src_is_url)
> {
> /* Are we copying/moving a path that is scheduled for addition
> as the result of svn add or svn mv/cp? */
> SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, src_path, FALSE,
> 0,
> ctx->cancel_func, ctx->cancel_baton,
> pool));
> SVN_ERR(svn_wc_entry(&entry, src_path, adm_access, FALSE, pool));
> SVN_ERR(svn_wc_adm_close(adm_access));
> /* If scheduled for addition but not copied, src_path is
> result of svn add not svn cp/mv. */
> src_is_add = (entry
> && entry->schedule == svn_wc_schedule_add
> && !(entry->copied))
> ? TRUE : FALSE;
> }
>
> Thoughts?
>
I consider that anyway condition should be the same that in
copy_file_administratively() and copy_dir_administratively() But have
same condition in several place is little bit dangerous,
so might we should reconsider and move your code from libsvn_client to
libsvn_wc?
On 8/9/06, Paul Burba <pa...@softlanding.com> wrote:
> Notice none of the unversioned children of H get moved, while only A\D\I
> gets copied.
>
> Also, a related oddity: While svn copy normally copies these first
> generation unversioned children, svn move requires you to use --force and
> then it *deletes* all the unversioned items in the source and does *not*
> copy any of them...so much for "this subcommand is equivalent to a 'copy'
> and 'delete'."
>
You are not right, they copied, but doesn't displayed in svn st -v.
Just check directory D_move\I by hand.
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Moving/copying added files and dirs
Posted by Paul Burba <pa...@softlanding.com>.
rooneg@gmail.com wrote on 08/08/2006 03:03:20 PM:
> On 8/8/06, Ivan Zhakov <ch...@gmail.com> wrote:
>
> > Second problem in setup_copy():
> > if (!src_is_url)
> > {
> > /* Are we copying/moving a path that is scheduled for addition?
*/
> > SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, src_path,
FALSE, 0,
> > ctx->cancel_func,
ctx->cancel_baton,
> > pool));
> > SVN_ERR(svn_wc_entry(&entry, src_path, adm_access, FALSE,
pool));
> > SVN_ERR(svn_wc_adm_close(adm_access));
> > src_is_add = (entry && entry->revision == 0) ? TRUE : FALSE;
> > }
> > Might be I forget something, but AFAIK zero revision number is valid.
> > So you should check against SVN_INVALID_REVNUM or check url field. I
> > don't remember exactly.
>
> Well, 0 is a valid revision, but it'll only ever exist on the root of
> the repos, so it's not something you can actually copy...
Hi All,
Just to be clear on my goal in this chunk of code: it's to determine if
the src_path arg to setup_copy is scheduled for addition as a result of
svn add, as opposed to a WC->WC copy/move. We need to know this since
wc_to_wc_copy() handles these two cases differently. My comment is
unclear on this and I will improve it with something like this:
if (!src_is_url)
{
/* Are we copying/moving a path that is scheduled for addition
as the result of svn add or svn mv/cp? */
SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, src_path, FALSE,
0,
ctx->cancel_func, ctx->cancel_baton,
pool));
Anyway, how do we differentiate between these two scenarios? Comparing
entry fields for each:
ENTRY ADDED WC->WC
FIELD FILE COPIED/MOVED
FILE
----------- ----- ------------
name upsilon alpha_copied
kind file
revision 0
url
repos
schedule add add
text-time
checksum d1fa4a3ced98961674a441930a51f2d3
committed-date
committed-rev
last-author
has-props
has-prop-mods
cachable-props
present-props
conflict-old
conflict-new
conflict-wrk
prop-reject-file
copied copied
comyfrom-url
http://localhost/svn-test-work/repositories/copy_tests-1/A/B/E/alpha
copyfrom-rev 1
deleted
absent
incomplete
uuid
lock-token
lock-owner
lock-comment
lock-creation-date
(A copied/moved directory entry differs only in that it has no checksum)
So we can't just look at the schedule value, it's add in both cases. The
explanation of the revision field in
http://svn.collab.net/repos/svn/trunk/subversion/libsvn_wc/README is:
revision:
The revision that the pristine text and properties of this entry
represent. Defaults to the revision of the this_dir entry, for
which it is required. Set to 0 for entries not yet in the
repository.
Between this and the observed behavior above led me to think that looking
at the rev is the way to differentiate between the two scenarios. The
docstring for svn_wc_add2() makes no promises regarding the revision of
added paths, though it does explicitly set it to 0 on line 1184:
tmp_entry.revision = 0;
Maybe I'm making a dangerous assumption?
Perhaps it's better to look at the schedule *and* copy fields to make the
determination?
if (!src_is_url)
{
/* Are we copying/moving a path that is scheduled for addition
as the result of svn add or svn mv/cp? */
SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, src_path, FALSE,
0,
ctx->cancel_func, ctx->cancel_baton,
pool));
SVN_ERR(svn_wc_entry(&entry, src_path, adm_access, FALSE, pool));
SVN_ERR(svn_wc_adm_close(adm_access));
/* If scheduled for addition but not copied, src_path is
result of svn add not svn cp/mv. */
src_is_add = (entry
&& entry->schedule == svn_wc_schedule_add
&& !(entry->copied))
? TRUE : FALSE;
}
Thoughts?
Paul B.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Moving/copying added files and dirs
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 8/8/06, Ivan Zhakov <ch...@gmail.com> wrote:
> Second problem in setup_copy():
> if (!src_is_url)
> {
> /* Are we copying/moving a path that is scheduled for addition? */
> SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, src_path, FALSE, 0,
> ctx->cancel_func, ctx->cancel_baton,
> pool));
> SVN_ERR(svn_wc_entry(&entry, src_path, adm_access, FALSE, pool));
> SVN_ERR(svn_wc_adm_close(adm_access));
> src_is_add = (entry && entry->revision == 0) ? TRUE : FALSE;
> }
> Might be I forget something, but AFAIK zero revision number is valid.
> So you should check against SVN_INVALID_REVNUM or check url field. I
> don't remember exactly.
Well, 0 is a valid revision, but it'll only ever exist on the root of
the repos, so it's not something you can actually copy...
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Moving/copying added files and dirs
Posted by Paul Burba <pa...@softlanding.com>.
"Ivan Zhakov" <ch...@gmail.com> wrote on 08/08/2006 02:59:16 PM:
> On 8/8/06, Paul Burba <pa...@softlanding.com> wrote:
> > ...and here here it is. This patch supports copying and moving of
added
> > paths within a WC or from a WC to the REPOS.
> Hi, Paul!
> Thank you for continue working on this usefull thing.
>
> >
> > Re issue 1 above, after looking at this I think the solution shouldn't
> > reside in libsvn_wc, but rather in libsvn_client. Trying to teach
> > svn_wc_copy2() to copy paths that are not versioned doesn't make a lot
of
> > sense. It seems all we really need to do is copy the added files
without
> > the administrative directories and then add them. The first task
sounds
> > an awful lot like an export, and with some borrowed/modified code from
> > copy_versioned_files() in export.c that's what this patch does. Then
a
> > call to svn_client_add3() adds the copied files. Does this approach
sound
> > reasonable?
>
> After look to code I agree with you. This approach reasonable. But I
> see two problems with your current patch:
>
> Now unversioned files in versioned commited directory copied when you
> copy directory. I mean:
>
> $ mkdir dir
> $ svn add dir
> $ svn ci -m ""
> $ touch dir\foo
> $ svn st dir
> ? dir\foo
>
> $ svn cp dir dir2
> $ svn st dir2
> ? dir2\foo
>
> But your patch seems copy only versioned files, i.e.
> $ mkdir dir
> $ svn add dir
> $ svn ci -m ""
> $ touch dir\foo
> $ svn st dir
> A dir
> ? dir\foo
>
> $ svn cp dir dir2
> $ svn st dir2
> A dir2
Hi Ivan,
I'm assuming you didn't check in dir in the second example and this is
just a cut and past error(?)
> So there is inconsistency. I consider it's bad.
I agree it's somewhat inconsistent, but realize that "svn copy A A_COPY"
copies *only* the first generation unversioned children of "A", for
example, let's take the old reliable greek tree with an unversioned tree
in it rooted at A\D\I, then copy and move it's parent, currently trunk
behaves like this:
------------------------------------------------------------
svn st -v
1 1 jrandom .
1 1 jrandom A
1 1 jrandom A\B
1 1 jrandom A\B\lambda
1 1 jrandom A\B\E
1 1 jrandom A\B\E\alpha
1 1 jrandom A\B\E\beta
1 1 jrandom A\B\F
1 1 jrandom A\mu
1 1 jrandom A\C
? A\D\I
1 1 jrandom A\D
1 1 jrandom A\D\gamma
1 1 jrandom A\D\G
1 1 jrandom A\D\G\pi
1 1 jrandom A\D\G\rho
1 1 jrandom A\D\G\tau
1 1 jrandom A\D\H
1 1 jrandom A\D\H\chi
1 1 jrandom A\D\H\omega
1 1 jrandom A\D\H\psi
1 1 jrandom iota
dir A\D\i /s
Volume in drive C has no label.
Volume Serial Number is BC4D-A20A
Directory of
C:\SVN\svn.trunk.copymove\src-trunk.collabnet.trunk\Release\subversion\tests\cmdline\svn-test-work\working_copies\copy_tests-1\A\D\I
08/08/2006 04:40 PM <DIR> .
08/08/2006 04:40 PM <DIR> ..
08/08/2006 04:40 PM <DIR> J
08/08/2006 04:40 PM 21 unversioned1
1 File(s) 21 bytes
Directory of
C:\SVN\svn.trunk.copymove\src-trunk.collabnet.trunk\Release\subversion\tests\cmdline\svn-test-work\working_copies\copy_tests-1\A\D\I\J
08/08/2006 04:40 PM <DIR> .
08/08/2006 04:40 PM <DIR> ..
08/08/2006 04:40 PM 21 unversioned2
1 File(s) 21 bytes
Total Files Listed:
2 File(s) 42 bytes
5 Dir(s) 78,639,190,016 bytes free
svn cp A\D D_copy
A D_copy
svn mv A\D D_move --force
A D_move
D A\D\gamma
D A\D\G\pi
D A\D\G\rho
D A\D\G\tau
D A\D\G
D A\D\H\chi
D A\D\H\omega
D A\D\H\psi
D A\D\H
D A\D
svn st -v
1 1 jrandom .
1 1 jrandom A
1 1 jrandom A\B
1 1 jrandom A\B\lambda
1 1 jrandom A\B\E
1 1 jrandom A\B\E\alpha
1 1 jrandom A\B\E\beta
1 1 jrandom A\B\F
1 1 jrandom A\mu
1 1 jrandom A\C
D 1 1 jrandom A\D
D 1 1 jrandom A\D\gamma
D 1 1 jrandom A\D\G
D 1 1 jrandom A\D\G\pi
D 1 1 jrandom A\D\G\rho
D 1 1 jrandom A\D\G\tau
D 1 1 jrandom A\D\H
D 1 1 jrandom A\D\H\chi
D 1 1 jrandom A\D\H\omega
D 1 1 jrandom A\D\H\psi
? D_move\I
A + - 1 jrandom D_move
+ - 1 jrandom D_move\gamma
+ - 1 jrandom D_move\G
+ - 1 jrandom D_move\G\pi
+ - 1 jrandom D_move\G\rho
+ - 1 jrandom D_move\G\tau
+ - 1 jrandom D_move\H
+ - 1 jrandom D_move\H\chi
+ - 1 jrandom D_move\H\omega
+ - 1 jrandom D_move\H\psi
1 1 jrandom iota
? D_copy\I
A + - 1 jrandom D_copy
+ - 1 jrandom D_copy\gamma
+ - 1 jrandom D_copy\G
+ - 1 jrandom D_copy\G\pi
+ - 1 jrandom D_copy\G\rho
+ - 1 jrandom D_copy\G\tau
+ - 1 jrandom D_copy\H
+ - 1 jrandom D_copy\H\chi
+ - 1 jrandom D_copy\H\omega
+ - 1 jrandom D_copy\H\psi
------------------------------------------------------------
Notice none of the unversioned children of H get moved, while only A\D\I
gets copied.
Also, a related oddity: While svn copy normally copies these first
generation unversioned children, svn move requires you to use --force and
then it *deletes* all the unversioned items in the source and does *not*
copy any of them...so much for "this subcommand is equivalent to a 'copy'
and 'delete'."
My thought when making the patch was to make svn copy more like svn move
when operating on added dirs with unversioned children. I'm definitely
not saying this is the right approach, but to tweak my patch to copy only
first generation children of the source doesn't seem like a good idea
either.
Anyone else have some thoughts on this?
Paul B.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Moving/copying added files and dirs
Posted by Ivan Zhakov <ch...@gmail.com>.
On 8/8/06, Paul Burba <pa...@softlanding.com> wrote:
> Paul Burba <pa...@softlanding.com> wrote on 07/27/2006 08:37:31 AM:
>
> > Ivan,
> >
> > Sorry, I wasn't able to look at this until yesterday afternoon. I'm
> > afraid it's not as simple as defeating sanity check 2 (i.e. in
> > copy_file_administratively(): /* Sanity check 2: You cannot make a copy
> of
> > something that's not in the repository unless it's a copy of an
> > uncommitted copy. */). Here are some of the problems I see:
> >
> > 1) An added file has no text base, so when copy_file_administratively()
> > tries to copy the pristine text base it fails.
> >
> > 2) If we are going to support copying an added file, it only makes sense
>
> > (?) to support the following as well:
> >
> > a) moving an added file
> > b) copying an added directory
> > c) moving an added directory
> >
> > This isn't so much a problem, as an added complication.
> >
> > 3) If we move an added file how do we get rid of the add entry in the
> > source directory? It's not like moving a versioned item, we don't want
> > any evidence left behind in entries that something once existed there
> and
> > was scheduled for addition...do we? I suppose the equivalent of svn rm
> > --force will work...
> >
> > Anyhow, I'm looking into this right now, hope to have some ideas
> shortly.
>
> ...and here here it is. This patch supports copying and moving of added
> paths within a WC or from a WC to the REPOS.
Hi, Paul!
Thank you for continue working on this usefull thing.
>
> Re issue 1 above, after looking at this I think the solution shouldn't
> reside in libsvn_wc, but rather in libsvn_client. Trying to teach
> svn_wc_copy2() to copy paths that are not versioned doesn't make a lot of
> sense. It seems all we really need to do is copy the added files without
> the administrative directories and then add them. The first task sounds
> an awful lot like an export, and with some borrowed/modified code from
> copy_versioned_files() in export.c that's what this patch does. Then a
> call to svn_client_add3() adds the copied files. Does this approach sound
> reasonable?
After look to code I agree with you. This approach reasonable. But I
see two problems with your current patch:
Now unversioned files in versioned commited directory copied when you
copy directory. I mean:
$ mkdir dir
$ svn add dir
$ svn ci -m ""
$ touch dir\foo
$ svn st dir
? dir\foo
$ svn cp dir dir2
$ svn st dir2
? dir2\foo
But your patch seems copy only versioned files, i.e.
$ mkdir dir
$ svn add dir
$ svn ci -m ""
$ touch dir\foo
$ svn st dir
A dir
? dir\foo
$ svn cp dir dir2
$ svn st dir2
A dir2
So there is inconsistency. I consider it's bad.
Second problem in setup_copy():
if (!src_is_url)
{
/* Are we copying/moving a path that is scheduled for addition? */
SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, src_path, FALSE, 0,
ctx->cancel_func, ctx->cancel_baton,
pool));
SVN_ERR(svn_wc_entry(&entry, src_path, adm_access, FALSE, pool));
SVN_ERR(svn_wc_adm_close(adm_access));
src_is_add = (entry && entry->revision == 0) ? TRUE : FALSE;
}
Might be I forget something, but AFAIK zero revision number is valid.
So you should check against SVN_INVALID_REVNUM or check url field. I
don't remember exactly.
>
> Also, there is one obvious oddity regarding my solution, that is the
> notification of 'A'dded 'D'eleted paths. Normally svn copy reports 'A'dds
> first then 'D'eletes, but this patch reports in the opposite order when
> moving/copying an added path (i.e. the Deletes are reported first). It's
> certainly possible to make tweak wc_to_wc_copy() to make the ordering the
> same, but it would require a bit more cumbersome logic and require
> svn_wc_adm_access_t *adm_access/src_access to be opened and closed twice,
> and I wasn't sure this is worth it(?).
For me it is not a problem.
>
> [[[
> Support copy and move of paths scheduled for addition.
>
> Follow-up to r20811.
>
> Suggested by: zhakov
>
> * subversion/libsvn_client/copy.c
> (svn_pools.h): Include.
> (copy_added_path): Helper function for wc_to_wc_copy() which copies
> paths scheduled for addition to a new WC location.
> (wc_to_wc_copy): Add new argument indicating if src_path is not under
> version control but is scheduled for addition. Use copy_added_path()
> rather than svn_wc_copy2() when copying added paths, then add these
> with svn_client_add3().
> (setup_copy): Determine if we are copying a path that is scheduled
> for addition by looking at it's revision and pass that info to
> wc_to_wc_copy().
>
> * subversion/tests/cmdline/copy_tests.py
> (copy_move_added_paths, copy_added_paths_to_URL): New tests.
> (test_list): Run new tests.
> ]]]
>
>
>
>
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org