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

Re: Fix for issue 3609 - ls command

From: Noorul Islam K M <noorul_at_collab.net>
Date: Wed, 10 Nov 2010 11:53:18 +0530

Julian Foad <julian.foad_at_wandisco.com> writes:

> On Tue, 2010-11-09, Kamesh Jayachandran wrote:
>
>> On 11/08/2010 11:59 AM, Noorul Islam K M wrote:
>> > [[[
>> > Canonicalize paths before passing them to svn_client_list2.
>> >
>> > * subversion/svn/list-cmd.c
>> > (svn_cl__list): Canonicalize url or dirent before passing over to API.
>> >
>> > * subversion/tests/cmdline/basic_tests.py
>> > (ls_url_special_characters, test_list): New test
>> >
>> > Patch by: Noorul Islam K M<noorul{_AT_}collab.net>
>> > ]]]
>> >
>> Thanks Noorul. I tweaked for a cosmetic consistency and committed in
>> r1033045.
>
> Hi Noorul. These fixes are useful, but I can see a pattern emerging:
> probably every time the command-line client calls svn_opt_parse_path(),
> it is going to want the resulting "true path" to be canonicalized,
> wither as a URL or as a WC path.
>
>> Index: subversion/svn/list-cmd.c
>> ===================================================================
>>
>> SVN_ERR(svn_opt_parse_path(&peg_revision, &truepath, target,
>> subpool));
>>
>> + if (svn_path_is_url(truepath))
>> + truepath = svn_uri_canonicalize(truepath, subpool);
>> + else
>> + truepath = svn_dirent_canonicalize(truepath, subpool);
>
> So I think it would be good if you could create a function that
> encapsulates the svn_opt_parse_path() and the new "if(url)..."
> statements. It could be called svn_cl__parse_path(), and it could live
> in subversion/svn/util.c.
>
> Then the svn client code could call it instead of svn_opt_parse_path().
>

I implemented the same and attached is the patch.

Log

[[[
New wrapper function svn_cl__opt_parse_path() for
svn_opt_parse_path(), which canonicalizes dirent/URL.

Make use of new function in "ls" and "cat" commands.

* subversion/svn/cl.h
  (svn_cl__opt_parse_path): New function.

* subversion/svn/util.c
  (svn_cl__opt_parse_path): New function.

* subversion/svn/list-cmd.c
  (svn_cl__list),
* subversion/svn/cat-cmd.c
  (svn_cl__cat): Use wrapper function svn_cl__opt_parse_path() instead
  of svn_opt_parse_path()

Suggested by: Julian Foad <julian.foad{_AT_}wandisco.com>
Patch by: Noorul Islam K M <noorul{_AT_}collab.net>

]]]

Thanks and Regards
Noorul

Index: subversion/svn/cl.h
===================================================================
--- subversion/svn/cl.h (revision 1033355)
+++ subversion/svn/cl.h (working copy)
@@ -810,6 +810,13 @@
                           const apr_array_header_t *targets,
                           apr_pool_t *pool);
 
+/* Like svn_opt_parse_path(), but canonicalizes dirent/URL */
+svn_error_t *
+svn_cl__opt_parse_path(svn_opt_revision_t *rev,
+ const char **truepath,
+ const char *path,
+ apr_pool_t *pool);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
Index: subversion/svn/cat-cmd.c
===================================================================
--- subversion/svn/cat-cmd.c (revision 1033355)
+++ subversion/svn/cat-cmd.c (working copy)
@@ -27,7 +27,6 @@
 
 /*** Includes. ***/
 
-#include "svn_path.h"
 #include "svn_pools.h"
 #include "svn_client.h"
 #include "svn_error.h"
@@ -69,14 +68,9 @@
       SVN_ERR(svn_cl__check_cancel(ctx->cancel_baton));
 
       /* Get peg revisions. */
- SVN_ERR(svn_opt_parse_path(&peg_revision, &truepath, target,
- subpool));
+ SVN_ERR(svn_cl__opt_parse_path(&peg_revision, &truepath, target,
+ subpool));
 
- if (svn_path_is_url(truepath))
- truepath = svn_uri_canonicalize(truepath, subpool);
- else
- truepath = svn_dirent_canonicalize(truepath, subpool);
-
       SVN_ERR(svn_cl__try(svn_client_cat2(out, truepath, &peg_revision,
                                           &(opt_state->start_revision),
                                           ctx, subpool),
Index: subversion/svn/list-cmd.c
===================================================================
--- subversion/svn/list-cmd.c (revision 1033355)
+++ subversion/svn/list-cmd.c (working copy)
@@ -268,14 +268,9 @@
       SVN_ERR(svn_cl__check_cancel(ctx->cancel_baton));
 
       /* Get peg revisions. */
- SVN_ERR(svn_opt_parse_path(&peg_revision, &truepath, target,
- subpool));
+ SVN_ERR(svn_cl__opt_parse_path(&peg_revision, &truepath, target,
+ subpool));
 
- if (svn_path_is_url(truepath))
- truepath = svn_uri_canonicalize(truepath, subpool);
- else
- truepath = svn_dirent_canonicalize(truepath, subpool);
-
       if (opt_state->xml)
         {
           svn_stringbuf_t *sb = svn_stringbuf_create("", pool);
Index: subversion/svn/util.c
===================================================================
--- subversion/svn/util.c (revision 1033355)
+++ subversion/svn/util.c (working copy)
@@ -1324,3 +1324,19 @@
 
   return SVN_NO_ERROR;
 }
+
+svn_error_t *
+svn_cl__opt_parse_path(svn_opt_revision_t *rev,
+ const char **truepath,
+ const char *path /* UTF-8! */,
+ apr_pool_t *pool)
+{
+ SVN_ERR(svn_opt_parse_path(rev, truepath, path, pool));
+
+ if (svn_path_is_url(*truepath))
+ *truepath = svn_uri_canonicalize(*truepath, pool);
+ else
+ *truepath = svn_dirent_canonicalize(*truepath, pool);
+
+ return SVN_NO_ERROR;
+}
Received on 2010-11-10 07:25:51 CET

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.