-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Stefan Sperling wrote:
> On Fri, Oct 23, 2009 at 08:59:04AM -0700, Julian Foad wrote:
>> Author: julianfoad
>> Date: Fri Oct 23 08:59:03 2009
>> New Revision: 40202
>>
>> Log:
>> Use new dirent/URI/path functions to resolve some deprecation warnings.
>> Also fix a bit of indentation.
>>
>> * subversion/libsvn_ra_neon/commit.c
>>   (get_version_url, create_activity, commit_delete_entry, commit_add_dir,
>>    commit_add_file, commit_close_file): Use `svn_path_url_add_component2()'.
>>   (add_child): Use `svn_path_url_add_component2()' and `svn_uri_join()'.
>>   (checkout_resource): Use `svn_relpath_local_style()'.
>>   (get_child_tokens): Use `svn_uri_is_child()'.
> 
>> @@ -325,8 +325,8 @@ static svn_error_t * create_activity(com
>>       the activity, and create the activity.  The URL for our activity
>>       will be ACTIVITY_COLL/UUID */
>>    SVN_ERR(get_activity_collection(cc, &activity_collection, FALSE, pool));
>> -  url = svn_path_url_add_component(activity_collection->data,
>> -                                   uuid_buf, pool);
>> +  url = svn_path_url_add_component2(activity_collection->data,
>> +                                    uuid_buf, pool);
>>    SVN_ERR(svn_ra_neon__simple_request(&code, cc->ras,
>>                                        "MKACTIVITY", url, NULL, NULL,
>>                                        201 /* Created */,
> 
> I had to revert the above hunk in r40224, to fix the following
> assertion failure during commit over ra_neon:

> The problem is that base has a trailing slash, so "/repos/svn/!svn/act/"
> is not considered canonical.
> The old version canonicalised the URL while the new version does not:
> 
>   const char *
>   svn_path_url_add_component(const char *url,
>                              const char *component,
>                              apr_pool_t *pool)
>   {
>     /* URL can have trailing '/' */
>     url = svn_path_canonicalize(url, pool);
>   
>     return svn_path_url_add_component2(url, component, pool);
>   }
> 
>   const char *
>   svn_path_url_add_component2(const char *url,
>                               const char *component,
>                               apr_pool_t *pool)
>   {
>     /* = svn_path_uri_encode() but without always copying */
>     component = uri_escape(component, uri_char_validity, pool);
>   
>     return svn_path_join(url, component, pool);
>   }
> 
> The docstring of the new version documents this difference, so this
> was done on purpose.
> 
> To fix this properly, we need to canonicalise the base somewhere.
> Please also check any other add_component->add_component2 upgrades
> made in r40202 for similar issues.

Attached herewith is the patch which fixes the failing commit owing to
the above case.

[[[
Log:
Make a proper fix to resolve some deprecation warnings using
`svn_path_url_add_component2()' by canonicalizing the base before
passing to the above method; and a minor indentation fix.

[in subversion/libsvn_ra_neon]

* commit.c
 (get_version_url, create_activity, commit_add_dir, commit_add_file
  commit_close_file, add_child): Use `svn_path_url_add_component2()'.
 (commit_delete_entry): Use `svn_path_url_add_component2()' and a minor
  indentation fix in comment.
 (svn_ra_neon__get_commit_editor, checkout_resource): Canonicalize the
  base before passing to `svn_path_url_add_component2()'.

* props.c
 (svn_ra_neon__get_baseline_info): Canonicalize the base before passing
  to `svn_path_url_add_component2()'.

Patch by: Kannan R <kann...@collab.net>
]]]

- --
Thanks & Regards,
Kannan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEVAwUBSwoli3lTqcY7ytmIAQLv5Qf/dWtRI9zjcFdAmaM0FWUFxOn5snETImgZ
QGGo9nx+S/YMK9gbjT0XWSwGTIThciRtz92hdL9wIhSNDlclrPShzA8S03xLG2jl
cMH31aUIvXqcDigVYjx2t6+6Ho8kB0rn2ciuYRypDkGjcK/A9pVWsWy8SgcBJL/h
Jt0f4YfXhSllEcVscd8Z5swl3nvg0bVT6ZDM+D3rKBBofEaO7pemuChwWUMcA/wk
CcNnoRV5+lhaHxmvS4KynBOvToVszLk6VdnjwwuEXH2bdhB15oMdqZRZIrHml3/N
rNMq/fsjnpJH79eZvoRgW95vSffUDb2NEKtvrrTESD1TEJyp/uXFfg==
=doxg
-----END PGP SIGNATURE-----
Index: ../../subversion/libsvn_ra_neon/commit.c
===================================================================
--- ../../subversion/libsvn_ra_neon/commit.c	(revision 882427)
+++ ../../subversion/libsvn_ra_neon/commit.c	(working copy)
@@ -203,9 +203,9 @@
          the version resource URL of RSRC. */
       if (parent && parent->vsn_url && parent->revision == rsrc->revision)
         {
-          rsrc->vsn_url = svn_path_url_add_component(parent->vsn_url,
-                                                     rsrc->name,
-                                                     rsrc->pool);
+          rsrc->vsn_url = svn_path_url_add_component2(parent->vsn_url,
+                                                      rsrc->name,
+                                                      rsrc->pool);
           return SVN_NO_ERROR;
         }
 
@@ -231,7 +231,7 @@
                                              rsrc->revision,
                                              pool));
 
-      url = svn_path_url_add_component(bc_url.data, bc_relative.data, pool);
+      url = svn_path_url_add_component2(bc_url.data, bc_relative.data, pool);
     }
 
   /* Get the DAV:checked-in property, which contains the URL of the
@@ -325,8 +325,8 @@
      the activity, and create the activity.  The URL for our activity
      will be ACTIVITY_COLL/UUID */
   SVN_ERR(get_activity_collection(cc, &activity_collection, FALSE, pool));
-  url = svn_path_url_add_component(activity_collection->data,
-                                   uuid_buf, pool);
+  url = svn_path_url_add_component2(activity_collection->data,
+                                    uuid_buf, pool);
   SVN_ERR(svn_ra_neon__simple_request(&code, cc->ras,
                                       "MKACTIVITY", url, NULL, NULL,
                                       201 /* Created */,
@@ -338,8 +338,8 @@
   if (code == 404)
     {
       SVN_ERR(get_activity_collection(cc, &activity_collection, TRUE, pool));
-      url = svn_path_url_add_component(activity_collection->data,
-                                       uuid_buf, pool);
+      url = svn_path_url_add_component2(activity_collection->data,
+                                        uuid_buf, pool);
       SVN_ERR(svn_ra_neon__simple_request(&code, cc->ras,
                                           "MKACTIVITY", url, NULL, NULL,
                                           201, 0, pool));
@@ -373,7 +373,7 @@
   rsrc->pool = pool;
   rsrc->revision = revision;
   rsrc->name = name;
-  rsrc->url = svn_path_url_add_component(parent->url, name, pool);
+  rsrc->url = svn_path_url_add_component2(parent->url, name, pool);
   rsrc->local_path = svn_path_join(parent->local_path, name, pool);
 
   /* Case 1:  the resource is truly "new".  Either it was added as a
@@ -382,7 +382,7 @@
      URL by the rules of deltaV:  "copy structure is preserved below
      the WR you COPY to."  */
   if (created || (parent->vsn_url == NULL))
-    rsrc->wr_url = svn_path_url_add_component(parent->wr_url, name, pool);
+    rsrc->wr_url = svn_path_url_add_component2(parent->wr_url, name, pool);
 
   /* Case 2: the resource is already under version-control somewhere.
      This means it has a VR URL already, and the WR URL won't exist
@@ -519,6 +519,12 @@
     }
 
   rsrc->wr_url = apr_pstrdup(rsrc->pool, parse.path);
+
+  /* Canonicalize the base here as `svn_path_url_add_component2()'
+     won't handle it */
+  rsrc->vsn_url = svn_uri_canonicalize(rsrc->vsn_url, pool);
+  rsrc->wr_url = svn_uri_canonicalize(rsrc->wr_url, pool);
+
   ne_uri_free(&parse);
 
   return SVN_NO_ERROR;
@@ -714,7 +720,7 @@
   SVN_ERR(checkout_resource(parent->cc, parent->rsrc, TRUE, NULL, pool));
 
   /* create the URL for the child resource */
-  child = svn_path_url_add_component(parent->rsrc->wr_url, name, pool);
+  child = svn_path_url_add_component2(parent->rsrc->wr_url, name, pool);
 
   /* Start out assuming that we're deleting a file;  try to lookup the
      path itself in the token-hash, and if found, attach it to the If:
@@ -729,8 +735,8 @@
           const char *token_header_val;
           const char *token_uri;
 
-          token_uri = svn_path_url_add_component(parent->cc->ras->url->data,
-                                                 path, pool);
+          token_uri = svn_path_url_add_component2(parent->cc->ras->url->data,
+                                                  path, pool);
           token_header_val = apr_psprintf(pool, "<%s> (<%s>)",
                                           token_uri, token);
           extra_headers = apr_hash_make(pool);
@@ -773,7 +779,7 @@
 
          Note that we're not sending the locks in the If: header, for
          the same reason we're not sending in MERGE's headers: httpd has
-       limits on the amount of data it's willing to receive in headers. */
+         limits on the amount of data it's willing to receive in headers. */
 
       apr_hash_t *child_tokens = NULL;
       svn_ra_neon__request_t *request;
@@ -888,9 +894,9 @@
          "source" argument to the COPY request.  The "Destination:"
          header given to COPY is simply the wr_url that is already
          part of the child object. */
-      copy_src = svn_path_url_add_component(bc_url.data,
-                                            bc_relative.data,
-                                            workpool);
+      copy_src = svn_path_url_add_component2(bc_url.data,
+                                             bc_relative.data,
+                                             workpool);
 
       /* Have neon do the COPY. */
       SVN_ERR(svn_ra_neon__copy(parent->cc->ras,
@@ -1088,9 +1094,9 @@
          "source" argument to the COPY request.  The "Destination:"
          header given to COPY is simply the wr_url that is already
          part of the file_baton. */
-      copy_src = svn_path_url_add_component(bc_url.data,
-                                            bc_relative.data,
-                                            workpool);
+      copy_src = svn_path_url_add_component2(bc_url.data,
+                                             bc_relative.data,
+                                             workpool);
 
       /* Have neon do the COPY. */
       SVN_ERR(svn_ra_neon__copy(parent->cc->ras,
@@ -1271,9 +1277,9 @@
         svn_ra_neon__set_header
           (extra_headers, "If",
            apr_psprintf(pool, "<%s> (<%s>)",
-                        svn_path_url_add_component(cc->ras->url->data,
-                                                   file->rsrc->url,
-                                                   request->pool),
+                        svn_path_url_add_component2(cc->ras->url->data,
+                                                    file->rsrc->url,
+                                                    request->pool),
                         file->token));
 
       if (pb->base_checksum)
@@ -1452,6 +1458,10 @@
   /* ### should we perform an OPTIONS to validate the server we're about
      ### to talk to? */
 
+  /* Canonicalize the base here as `svn_path_url_add_component2()'
+     won't handle it */
+  cc->ras->act_coll = svn_uri_canonicalize(cc->ras->act_coll, pool);
+
   /*
   ** Create an Activity. This corresponds directly to an FS transaction.
   ** We will check out all further resources within the context of this
Index: ../../subversion/libsvn_ra_neon/props.c
===================================================================
--- ../../subversion/libsvn_ra_neon/props.c	(revision 882427)
+++ ../../subversion/libsvn_ra_neon/props.c	(working copy)
@@ -991,7 +991,12 @@
 
   /* maybe return bc_url to the caller */
   if (bc_url)
-    *bc_url = *my_bc_url;
+    {
+      /* Canonicalize the base here as `svn_path_url_add_component2()'
+         won't handle it */
+      bc_url->data = svn_uri_canonicalize(my_bc_url->data, pool);
+      bc_url->len = my_bc_url->len;
+    }
 
   if (latest_rev != NULL)
     {

Reply via email to