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

Re: [PATCH 1 of 2] Add peg revision support to svn_opt__arg_canonicalize_*()

From: Daniel Shahaf <d.s_at_daniel.shahaf.co.il>
Date: Thu, 1 May 2008 11:02:09 +0300 (IDT)

Troy Curtis Jr wrote on Wed, 30 Apr 2008 at 22:28 -0500:
> This is the first part of the patch necessary to address correctly handling
> peg revision specifications in repository root urls.
>

Is it orthogonal to your "Fix assertion failure with root relative url"
patch?

> [[[
> Add peg revision support to svn_opt__arg_canonicalize_url() and
> svn_opt__arg_canonicalize_path(). These functions are meant to process
> user-input and so should handle the peg revision case.
>
> * subversion/libsvn_subr/opt.c
> (split_arg_at_peg_revision): New function.

How is it different from svn_opt_parse_path ?

> (svn_opt__arg_canonicalize_url,
> svn_opt__arg_canonicalize_path): Allow the input argument to contain a peg
> revision specifier, and preserve it in the output utilizing
> split_arg_at_peg_revision().
>
> * subversion/include/private/svn_opt_private.h
> (svn_opt__arg_canonicalize_url,
> svn_opt__arg_canonicalize_path): Update the doc string to reflect the fact
> that these functions can handle a peg revision in the input argument.
> ]]]
>
>

> Index: subversion/libsvn_subr/opt.c
> ===================================================================
> --- subversion/libsvn_subr/opt.c (revision 30798)
> +++ subversion/libsvn_subr/opt.c (working copy)
> @@ -1114,14 +1083,75 @@
> svn_error_t *
> svn_opt__arg_canonicalize_url(const char **url_out, const char *url_in,
> apr_pool_t *pool)
> {
> const char *target;
> + const char *peg_rev;
>
> + SVN_ERR(split_arg_at_peg_revision(&target, &peg_rev, url_in, pool));
> +
> /* Convert to URI. */
> - target = svn_path_uri_from_iri(url_in, pool);
> + target = svn_path_uri_from_iri(target, pool);
> /* Auto-escape some ASCII characters. */
> target = svn_path_uri_autoescape(target, pool);
>
> @@ -1140,6 +1170,11 @@
> /* strip any trailing '/' and collapse other redundant elements */
> target = svn_path_canonicalize(target, pool);
>
> + /* Append the peg revision back to the canonicalized target if
> + there was a peg revision. */
> + if (peg_rev)
> + target = apr_pstrcat(pool, target, peg_rev, NULL);
> +

So we parse the peg rev out, canonicalize, and concat it back (later
to be parsed again). Why did you choose this way?

> *url_out = target;
> return SVN_NO_ERROR;
> }
> @@ -1151,9 +1186,12 @@
> const char *apr_target;
> char *truenamed_target; /* APR-encoded */
> apr_status_t apr_err;
> + const char *peg_rev;
>
> + SVN_ERR(split_arg_at_peg_revision(&apr_target, &peg_rev, path_in, pool));
> +
> /* canonicalize case, and change all separators to '/'. */
> - SVN_ERR(svn_path_cstring_from_utf8(&apr_target, path_in, pool));
> + SVN_ERR(svn_path_cstring_from_utf8(&apr_target, apr_target, pool));
> apr_err = apr_filepath_merge(&truenamed_target, "", apr_target,
> APR_FILEPATH_TRUENAME, pool);
>
> @@ -1174,6 +1212,11 @@
> SVN_ERR(svn_path_cstring_to_utf8(path_out, apr_target, pool));
> *path_out = svn_path_canonicalize(*path_out, pool);
>
> + /* Append the peg revision back to the canonicalized target if
> + there was a peg revision. */
> + if (peg_rev)
> + *path_out = apr_pstrcat(pool, *path_out, peg_rev, NULL);
> +

I've seen this hunk before. Do you need an helper function here?

> return SVN_NO_ERROR;
> }
>

---------------------------------------------------------------------
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-01 10:02:30 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.