-----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) {