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

Re: [PATCH] issue #3620: svn add ^/ triggers assertion failure

From: Stefan Sperling <stsp_at_elego.de>
Date: Fri, 23 Apr 2010 23:24:42 +0200

On Fri, Apr 23, 2010 at 12:46:44PM -0400, C. Michael Pilato wrote:
> As I noted, the check needs to happen inside the client API. The question
> becomes "Do we patch svn_client_addX() and svn_client_statusX() and
> svn_client_updateX() and ..." or is there a more general fix to be made
> (like perhaps a wrapper around svn_dirent_get_absolute() that gracefully
> handles the my-caller-passed-a-url case)?

I think the problem is that the CLI client passes non-canonicalized
paths to the client library.

If we make the CLI client canonicalize its arguments using
svn_dirent_canonicalize(), the assertion does not trigger.
This makes sense since the 'svn add' command only accepts local working
copy paths, so it should try to interpret its arguments as such.

With the patch below I get an error such as:

$ svn add ^/
svn: warning: '/path/to/working/copy/file:/tmp/svn-sandbox/repos' not found

(The repository lives in "/tmp/svn-sandbox/repos".)

Do others agree? If so, I'd like to ask Uwe to send similar patches
for other subcommands, making sure they canonicalize their arguments
correctly.

We should also consider changing the way "^/" is interpreted.
Maybe only commands that expect URLs as arguments should expand "^/"
to a URL? That way the above error would become:

$ svn add ^/
svn: warning: '/path/to/working/copy/^' not found

I'm running the diff below through make check now.

Stefan

[[[
* subversion/svn/add-cmd.c
  (svn_cl__add): Canonicalize target paths before passing them
   to the client library.
]]]

Index: subversion/svn/add-cmd.c
===================================================================
--- subversion/svn/add-cmd.c (revision 937185)
+++ subversion/svn/add-cmd.c (working copy)
@@ -30,6 +30,7 @@
 #include <apr_want.h>
 
 #include "svn_client.h"
+#include "svn_dirent_uri.h"
 #include "svn_error.h"
 #include "svn_pools.h"
 #include "cl.h"
@@ -70,11 +71,15 @@ svn_cl__add(apr_getopt_t *os,
   for (i = 0; i < targets->nelts; i++)
     {
       const char *target = APR_ARRAY_IDX(targets, i, const char *);
+ const char *canon_target;
 
       svn_pool_clear(subpool);
+
       SVN_ERR(svn_cl__check_cancel(ctx->cancel_baton));
+
+ canon_target = svn_dirent_canonicalize(target, subpool);
       SVN_ERR(svn_cl__try
- (svn_client_add4(target,
+ (svn_client_add4(canon_target,
                                opt_state->depth,
                                opt_state->force, opt_state->no_ignore,
                                opt_state->parents, ctx, subpool),
Received on 2010-04-23 23:25:21 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.