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

Re: svn commit: r30931 - trunk/subversion/libsvn_client

From: Daniel Shahaf <d.s_at_daniel.shahaf.co.il>
Date: Sat, 3 May 2008 00:19:59 +0300 (IDT)

kameshj_at_tigris.org wrote on Fri, 2 May 2008 at 06:05 -0700:
> Author: kameshj
> Date: Fri May 2 06:05:58 2008
> New Revision: 30931
>

Reviewing this again prior to voting, I noticed something that I didn't
catch during earlier discussions.

> Log:
> Prior to this Fix,
> '/tmp' *not* being a working copy, following command executed from '/tmp'
> and /tmp/abc having some useful contents, would wipe off /tmp/abc.
> $svn mkdir --parents abc
>
> As a part of this fix we delete '/tmp/abc' upon failure only if this command
> has created it.
>
> * subversion/libsvn_client/add.c
> (svn_client__make_local_parents): Delete path upon failure
> only if this invocation has created it.
>
> Modified:
> trunk/subversion/libsvn_client/add.c
>
> Modified: trunk/subversion/libsvn_client/add.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/add.c?pathrev=30931&r1=30930&r2=30931
> ==============================================================================
> --- trunk/subversion/libsvn_client/add.c Fri May 2 05:27:20 2008 (r30930)
> +++ trunk/subversion/libsvn_client/add.c Fri May 2 06:05:58 2008 (r30931)
> @@ -789,7 +789,8 @@ svn_client__make_local_parents(const cha
> apr_pool_t *pool)
> {
> svn_error_t *err;
> -
> + svn_node_kind_t orig_kind;
> + SVN_ERR(svn_io_check_path(path, &orig_kind, pool));

According to svn_io_check_path's docstring (both before and after the
recent change), when "intermediate directories on the way to @a path
don't exist", it returns an error and leaves the value of orig_kind
uninitialized.

In svn_client__make_local_parents we cannot assume that the intermediate
directories exist; therefore, the above call might raise an error.
However, 'mkdir --parents' works in HEAD; an error is not raised.

This leaves me with two questions:

* Should we be calling svn_io_check_path in
  svn_client__make_local_parents ?

* Is svn_io_check_path's docstring still wrong? (when it says errors
  would be returned)

> if (make_parents)
> SVN_ERR(svn_io_make_dir_recursively(path, pool));
> else
> @@ -800,7 +801,7 @@ svn_client__make_local_parents(const cha
>
> /* We just created a new directory, but couldn't add it to
> version control. Don't leave unversioned directories behind. */
> - if (err)
> + if (err && (orig_kind == svn_node_none))
> {
> /* ### If this returns an error, should we link it onto
> err instead, so that the user is warned that we just
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-02 23:20:26 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.