Please, could anybody review my patch?
Rui
On Sun, Jul 06, 2008 at 12:43:50AM +0800, Rui, Guo wrote:
> Sorry, I just fogot to attach the patch in my previous post.
>
> Rui
> On Sun, Jul 06, 2008 at 12:39:43AM +0800, Rui, Guo wrote:
> > Hi all,
> > I found some problems in the copy sub-command while trying to make 'svn cp'
> > works when the destination is a excluded item. I found a suspicious code in
> > copy_file_administratively(). I then did some experiments and verified my
> > suspect. The problem is that, the checking of existing destination does not
> > cover the situation of directory. The following commands can trigger the
> > problem:
> >
> > svn co http://server/greek_tree wc
> > cd wc/A/B
> > rm -rf F
> > svn cp lambda F
> >
> > The error output is like this:
> >
> > subversion/libsvn_wc/log.c:1602: (apr_err=155020)
> > svn: In directory '.'
> > subversion/libsvn_wc/log.c:1602: (apr_err=155020)
> > svn: Error processing command 'modify-entry' in '.'
> > subversion/libsvn_wc/log.c:861: (apr_err=155020)
> > svn: Error modifying entry for 'F'
> > subversion/libsvn_wc/entries.c:2475: (apr_err=155013)
> > svn: Entry 'F' is already under version control
> >
> > And even an 'svn cleanup' can not bring the wc back to work, which is quite
> > annoying. After correcting the check, I get this output instead:
> >
> > subversion/libsvn_client/copy.c:536: (apr_err=155007)
> > svn: Path 'F' is not a directory
> >
> > The error prompt is kinds of confusing. The reason is that, the code in
> > libsvn_client will try to copy the source into the target directory if it
> > exists and the check is not perfect. More effort is needed. Please see the
> > patch for detail.
> >
> > [[[
> > Check the existence of target directory on copy.
> >
> > * subversion/libsvn_subr/io.c
> > (svn_io_copy_dir_recursively): Use SVN_ERR_FS_ALREADY_EXISTS instead of
> > SVN_ERR_ENTRY_EXISTS.
> >
> > * subversion/libsvn_wc/copy.c
> > (copy_file_administratively): The same as above. Also check the existence
> > of target directory.
> >
> > * subversion/libsvn_client/copy.c
> > (wc_to_wc_copy, repos_to_wc_copy): The same as the first one.
> > (svn_client_copy4): Only check SVN_ERR_FS_ALREADY_EXISTS.
> >
> > ]]]
> >
> >
> > If we explicitly rm the destination, the result also changes.
> >
> > svn co http://server/greek_tree wc
> > cd wc/A/B
> > svn rm F
> > rm -rf F
> > svn cp lambda F
> >
> > result:
> > subversion/libsvn_subr/io.c:2628: (apr_err=2)
> > svn: Can't open file 'F/.svn/tmp/dir-prop-revert':
> >
> > This time the result is a little better, since the wc is still usable. Maybe
> > somebody can makes it work better?
> >
> > Still, there is one more strange thing. After the directory F is removed both
> > from revision control and the disk, the 'svn revert' unsurprisingly fails to
> > finish its work. An 'svn update' can bring back the F directory. However, the
> > 'delete' schedule flag of directory F in the parent entries is not cleared.
> > Will it be harmful?
> >
> > Rui
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> > For additional commands, e-mail: dev-help_at_subversion.tigris.org
> >
> Index: libsvn_subr/io.c
> ===================================================================
> --- libsvn_subr/io.c (revision 32000)
> +++ libsvn_subr/io.c (working copy)
> @@ -859,7 +859,7 @@
>
> SVN_ERR(svn_io_check_path(dst_path, &kind, subpool));
> if (kind != svn_node_none)
> - return svn_error_createf(SVN_ERR_ENTRY_EXISTS, NULL,
> + return svn_error_createf(SVN_ERR_FS_ALREADY_EXISTS, NULL,
> _("Destination '%s' already exists"),
> svn_path_local_style(dst_path, pool));
>
> Index: libsvn_wc/copy.c
> ===================================================================
> --- libsvn_wc/copy.c (revision 32000)
> +++ libsvn_wc/copy.c (working copy)
> @@ -399,18 +399,17 @@
> /* Sanity check: if dst file exists already, don't allow overwrite. */
> SVN_ERR(svn_io_check_path(dst_path, &dst_kind, pool));
> if (dst_kind != svn_node_none)
> - return svn_error_createf(SVN_ERR_ENTRY_EXISTS, NULL,
> + return svn_error_createf(SVN_ERR_FS_ALREADY_EXISTS, NULL,
> _("'%s' already exists and is in the way"),
> svn_path_local_style(dst_path, pool));
>
> - /* Even if DST_PATH doesn't exist it may still be a versioned file; it
> + /* Even if DST_PATH doesn't exist it may still be a versioned item; it
> may be scheduled for deletion, or the user may simply have removed the
> working copy. Since we are going to write to DST_PATH text-base and
> prop-base we need to detect such cases and abort. */
> SVN_ERR(svn_wc_entry(&dst_entry, dst_path, dst_parent, FALSE, pool));
> - if (dst_entry && dst_entry->kind == svn_node_file)
> + if (dst_entry && dst_entry->schedule != svn_wc_schedule_delete)
> {
> - if (dst_entry->schedule != svn_wc_schedule_delete)
> return svn_error_createf(SVN_ERR_ENTRY_EXISTS, NULL,
> _("There is already a versioned item '%s'"),
> svn_path_local_style(dst_path, pool));
> Index: libsvn_client/copy.c
> ===================================================================
> --- libsvn_client/copy.c (revision 32000)
> +++ libsvn_client/copy.c (working copy)
> @@ -517,7 +517,7 @@
> Else, just error out. */
> SVN_ERR(svn_io_check_path(pair->dst, &dst_kind, iterpool));
> if (dst_kind != svn_node_none)
> - return svn_error_createf(SVN_ERR_ENTRY_EXISTS, NULL,
> + return svn_error_createf(SVN_ERR_FS_ALREADY_EXISTS, NULL,
> _("Path '%s' already exists"),
> svn_path_local_style(pair->dst, pool));
>
> @@ -1600,7 +1600,7 @@
> SVN_ERR(svn_io_check_path(pair->dst, &dst_kind, iterpool));
> if (dst_kind != svn_node_none)
> {
> - return svn_error_createf(SVN_ERR_ENTRY_EXISTS, NULL,
> + return svn_error_createf(SVN_ERR_FS_ALREADY_EXISTS, NULL,
> _("Path '%s' already exists"),
> svn_path_local_style(pair->dst, pool));
> }
> @@ -2002,8 +2002,7 @@
> /* If the destination exists, try to copy the sources as children of the
> destination. */
> if (copy_as_child && err && (sources->nelts == 1)
> - && (err->apr_err == SVN_ERR_ENTRY_EXISTS
> - || err->apr_err == SVN_ERR_FS_ALREADY_EXISTS))
> + && (err->apr_err == SVN_ERR_FS_ALREADY_EXISTS))
> {
> const char *src_path = APR_ARRAY_IDX(sources, 0,
> svn_client_copy_source_t *)->path;
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-08 04:39:34 CEST