You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Troy Curtis Jr <tr...@gmail.com> on 2008/05/01 03:28:51 UTC
[PATCH 1 of 2] Add peg revision support to svn_opt__arg_canonicalize_*()
This is the first part of the patch necessary to address correctly handling
peg revision specifications in repository root urls.
[[[
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.
(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.
]]]
--
Beware of spyware. If you can, use the Firefox browser. - USA Today
Download now at http://getfirefox.com
Registered Linux User #354814 ( http://counter.li.org/)
Re: [PATCH 1 of 2] Add peg revision support to
svn_opt__arg_canonicalize_*()
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Troy Curtis Jr wrote on Sun, 4 May 2008 at 15:36 -0500:
> On Thu, May 1, 2008 at 8:30 AM, Daniel Shahaf <d....@daniel.shahaf.co.il> wrote:
> > Troy Curtis Jr wrote on Thu, 1 May 2008 at 06:54 -0500:
> > > On Thu, May 1, 2008 at 3:02 AM, Daniel Shahaf <d....@daniel.shahaf.co.il> wrote:
> > > > Troy Curtis Jr wrote on Wed, 30 Apr 2008 at 22:28 -0500:
> > > > > 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 ?
> > > >
> > >
> > > split_arg_at_peg_revision() deals strictly with text, whereas
> > > svn_opt_parse_path() puts the peg revision in to a revision struct.
> >
> > In other words, the former returns the peg as string, and the latter as
> > an svn_opt_revision_t.
> >
> > In this case, it seems to me that you use split_arg_at_peg_revision() as
> > a helper function in svn_opt_parse_path(), instead of embedding the
> > knowledge of peg-rev syntax in both of them.
> >
>
> Really split_arg_at_peg_revision() was created to prevent
> duplication in the svn_opt__arg_canonicalize_*() functions. It seems to me
> that this patch has a specific purpose of teaching
> svn_opt__arg_canonicalize_*() about peg revision syntax. Perhaps a follow on
> patch could also make use of split_arg_at_peg_revision() in
> svn_opt_parse_path(), but I don't think it really 'fits' this patch.
>
Logically, you extract the existing functionality and repack it in
reusable form before you reuse it in a new place, so the "follow on"
should actually come first. :)
It could be one patch, even, if breaking out the helper isn't noisy and
doesn't distract or drown the main part of the patch (peg support).
> > Looking deeper, I see now that svn_opt_args_to_target_array3() returns
> > an array of const char *, and subcommands expect to get an array of
> > targets with peg-revs appended, which -- eventually -- is why you have
> > to fold everything back to strings. It would be nicer to use
> > svn_opt_parse_path and teach subcommands to expect structs, of course,
> > but this can be done later.
>
> Yeah this was discussed when I was originally moving the arg processing into
> libsvn_client. We talked about returning an array of structure with
> several bits
> of useful information. I actually started down the path to figure out how to do
> it, but the expectation of an array of const char * was extremely deeply
> embedded.
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH 1 of 2] Add peg revision support to svn_opt__arg_canonicalize_*()
Posted by Troy Curtis Jr <tr...@gmail.com>.
On Thu, May 1, 2008 at 8:30 AM, Daniel Shahaf <d....@daniel.shahaf.co.il> wrote:
> Please CC the list.
>
> Troy Curtis Jr wrote on Thu, 1 May 2008 at 06:54 -0500:
>
> > On Thu, May 1, 2008 at 3:02 AM, Daniel Shahaf <d....@daniel.shahaf.co.il> wrote:
> > > 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?
> > >
> >
> > Yes in reality it is. The assertion failure was caused by the use of
> > svn_path_join() which requires canonicalized inputs. I submitted them
> > together before because they were "root relative url fixes".
> >
>
> OK, that wasn't clear to me. That means I or someone else can review
> the other patch in parallel to reviewing this one.
>
>
> > >
> > > > [[[
> > > > 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 ?
> > >
> >
> > split_arg_at_peg_revision() deals strictly with text, whereas
> > svn_opt_parse_path() puts the peg revision in to a revision struct.
>
> In other words, the former returns the peg as string, and the latter as
> an svn_opt_revision_t.
>
> In this case, it seems to me that you use split_arg_at_peg_revision() as
> a helper function in svn_opt_parse_path(), instead of embedding the
> knowledge of peg-rev syntax in both of them.
>
Really split_arg_at_peg_revision() was created to prevent
duplication in the svn_opt__arg_canonicalize_*() functions. It seems to me
that this patch has a specific purpose of teaching
svn_opt__arg_canonicalize_*() about peg revision syntax. Perhaps a follow on
patch could also make use of split_arg_at_peg_revision() in
svn_opt_parse_path(), but I don't think it really 'fits' this patch.
>
>
> > Actually I copied a comment explaining this reasoning from the
> > svn_opt_args_to_target_array3() function to the doc string of this
> > function to address just this issue.
> >
> > >
> > > > (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?
> > >
> >
> > The doc string of split_arg_at_peg_revision explains why.
> (the docstring is quoted below)
>
> > I kept it
> > this way because I believed the original reasoning made sense. In my
> > new implementation I would have duplicated the logic doing this in
> > three diffrenent places: back in the old
> > svn_opt_args_to_target_arrray3(), non root relative targets in
> > svn_client_args_to_target_array(), and root relative target in
> > svn_client_args_to_target_array(). This is why I factored it out. Of
> > course this change does not actually fix the ultimate issue, it lays
> > the foundation work for the 2 of 2 patch. This was suggested by
> > Blair.
> >
>
> The part that bothers me is "concat it back". The only callers of
> split_arg_at_peg_revision() are svn_opt__arg_canonicalize_{url,path};
> and you could modify them to return target and target's peg separately.
>
> Looking deeper, I see now that svn_opt_args_to_target_array3() returns
> an array of const char *, and subcommands expect to get an array of
> targets with peg-revs appended, which -- eventually -- is why you have
> to fold everything back to strings. It would be nicer to use
> svn_opt_parse_path and teach subcommands to expect structs, of course,
> but this can be done later.
Yeah this was discussed when I was originally moving the arg processing into
libsvn_client. We talked about returning an array of structure with
several bits
of useful information. I actually started down the path to figure out how to do
it, but the expectation of an array of const char * was extremely deeply
embedded.
>
>
> This is the docstring:
>
> +/* Extract the peg revision, if any, from UTF8_TARGET. Return the
> + * peg revision in *PEG_REVISION and the true target portion
> + * in *TRUE_TARGET.
> + *
> + * This is needed so that UTF8_TARGET can be properly canonicalized,
> + * otherwise the canonicalization does not treat a ".@BASE" as a "."
> + * with a BASE peg revision, and it is not canonicalized to "@BASE".
> + * If any peg revision exists, it is appended to the final
> + * canonicalized path or URL. Do not use svn_opt_parse_path()
> + * because the resulting peg revision is a structure that would have
> + * to be converted back into a string. Converting from a string date
> + * to the apr_time_t field in the svn_opt_revision_value_t and back to
> + * a string would not necessarily preserve the exact bytes of the
> + * input date, so its easier just to keep it in string form.
> + *
> + * All allocations are done in POOL.
> + */
> +static svn_error_t *
> +split_arg_at_peg_revision(const char **true_target,
> + const char **peg_revision,
> + const char *utf8_target,
> + apr_pool_t *pool)
>
>
>
>
> > > > *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?
> > >
> >
> > I don't think so, no where it makes sense. Pretty much it would be
> > replacing apr_pstrcat() with some other function that just did an
> > apr_pstrcat(). I'd still need the check there to see IF I needed to
> > append the revision.
> >
>
> I agree now. Earlier, I saw the svn_path_canonicalize() call in both
> hunks and the identical four lines added, and automatically assumed
> these two functions might have enough in common to justify a shared
> helper. Looking at the actual functions now, that is not the case.
> Sorry for misleading you.
>
> Daniel
>
>
>
> > > > return SVN_NO_ERROR;
> > > > }
> > > >
> > >
> >
> > Troy
> >
> >
>
Troy
--
Beware of spyware. If you can, use the Firefox browser. - USA Today
Download now at http://getfirefox.com
Registered Linux User #354814 ( http://counter.li.org/)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH 1 of 2] Add peg revision support to
svn_opt__arg_canonicalize_*()
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Please CC the list.
Troy Curtis Jr wrote on Thu, 1 May 2008 at 06:54 -0500:
> On Thu, May 1, 2008 at 3:02 AM, Daniel Shahaf <d....@daniel.shahaf.co.il> wrote:
> > 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?
> >
>
> Yes in reality it is. The assertion failure was caused by the use of
> svn_path_join() which requires canonicalized inputs. I submitted them
> together before because they were "root relative url fixes".
>
OK, that wasn't clear to me. That means I or someone else can review
the other patch in parallel to reviewing this one.
> >
> > > [[[
> > > 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 ?
> >
>
> split_arg_at_peg_revision() deals strictly with text, whereas
> svn_opt_parse_path() puts the peg revision in to a revision struct.
In other words, the former returns the peg as string, and the latter as
an svn_opt_revision_t.
In this case, it seems to me that you use split_arg_at_peg_revision() as
a helper function in svn_opt_parse_path(), instead of embedding the
knowledge of peg-rev syntax in both of them.
> Actually I copied a comment explaining this reasoning from the
> svn_opt_args_to_target_array3() function to the doc string of this
> function to address just this issue.
>
> >
> > > (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?
> >
>
> The doc string of split_arg_at_peg_revision explains why.
(the docstring is quoted below)
> I kept it
> this way because I believed the original reasoning made sense. In my
> new implementation I would have duplicated the logic doing this in
> three diffrenent places: back in the old
> svn_opt_args_to_target_arrray3(), non root relative targets in
> svn_client_args_to_target_array(), and root relative target in
> svn_client_args_to_target_array(). This is why I factored it out. Of
> course this change does not actually fix the ultimate issue, it lays
> the foundation work for the 2 of 2 patch. This was suggested by
> Blair.
>
The part that bothers me is "concat it back". The only callers of
split_arg_at_peg_revision() are svn_opt__arg_canonicalize_{url,path};
and you could modify them to return target and target's peg separately.
Looking deeper, I see now that svn_opt_args_to_target_array3() returns
an array of const char *, and subcommands expect to get an array of
targets with peg-revs appended, which -- eventually -- is why you have
to fold everything back to strings. It would be nicer to use
svn_opt_parse_path and teach subcommands to expect structs, of course,
but this can be done later.
This is the docstring:
+/* Extract the peg revision, if any, from UTF8_TARGET. Return the
+ * peg revision in *PEG_REVISION and the true target portion
+ * in *TRUE_TARGET.
+ *
+ * This is needed so that UTF8_TARGET can be properly canonicalized,
+ * otherwise the canonicalization does not treat a ".@BASE" as a "."
+ * with a BASE peg revision, and it is not canonicalized to "@BASE".
+ * If any peg revision exists, it is appended to the final
+ * canonicalized path or URL. Do not use svn_opt_parse_path()
+ * because the resulting peg revision is a structure that would have
+ * to be converted back into a string. Converting from a string date
+ * to the apr_time_t field in the svn_opt_revision_value_t and back to
+ * a string would not necessarily preserve the exact bytes of the
+ * input date, so its easier just to keep it in string form.
+ *
+ * All allocations are done in POOL.
+ */
+static svn_error_t *
+split_arg_at_peg_revision(const char **true_target,
+ const char **peg_revision,
+ const char *utf8_target,
+ apr_pool_t *pool)
> > > *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?
> >
>
> I don't think so, no where it makes sense. Pretty much it would be
> replacing apr_pstrcat() with some other function that just did an
> apr_pstrcat(). I'd still need the check there to see IF I needed to
> append the revision.
>
I agree now. Earlier, I saw the svn_path_canonicalize() call in both
hunks and the identical four lines added, and automatically assumed
these two functions might have enough in common to justify a shared
helper. Looking at the actual functions now, that is not the case.
Sorry for misleading you.
Daniel
> > > return SVN_NO_ERROR;
> > > }
> > >
> >
>
> Troy
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH 1 of 2] Add peg revision support to
svn_opt__arg_canonicalize_*()
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
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@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH 1 of 2] Add peg revision support to
svn_opt__arg_canonicalize_*()
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Troy Curtis Jr wrote on Mon, 12 May 2008 at 22:14 -0500:
> On Sun, May 11, 2008 at 11:19 PM, Karl Fogel <kf...@red-bean.com> wrote:
> > "Troy Curtis Jr" <tr...@gmail.com> writes:
> >>> Filed both patches as issue #3193 with milestone 1.6-consider. Troy, I
> >>> didn't CC or assign to you because I don't know your tigris username.
> >>
> >> Neither do I as I don't have one!
> >
> > Ooooh, can you get one? It's easy, and... Resistance Is Futile, You
> > Will Be Assimilated.
> >
> > -Karl
> >
>
> Nooooo!! Oh ok, I just signed up. FYI it's troycurtisjr.
>
>
Feel free to CC yourself to #3193 now, I assume you're interested in it :)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH 1 of 2] Add peg revision support to svn_opt__arg_canonicalize_*()
Posted by Troy Curtis Jr <tr...@gmail.com>.
On Sun, May 11, 2008 at 11:19 PM, Karl Fogel <kf...@red-bean.com> wrote:
> "Troy Curtis Jr" <tr...@gmail.com> writes:
>>> Filed both patches as issue #3193 with milestone 1.6-consider. Troy, I
>>> didn't CC or assign to you because I don't know your tigris username.
>>
>> Neither do I as I don't have one!
>
> Ooooh, can you get one? It's easy, and... Resistance Is Futile, You
> Will Be Assimilated.
>
> -Karl
>
Nooooo!! Oh ok, I just signed up. FYI it's troycurtisjr.
--
"Beware of spyware. If you can, use the Firefox browser." - USA Today
Download now at http://getfirefox.com
Registered Linux User #354814 ( http://counter.li.org/)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH 1 of 2] Add peg revision support to svn_opt__arg_canonicalize_*()
Posted by Karl Fogel <kf...@red-bean.com>.
"Troy Curtis Jr" <tr...@gmail.com> writes:
>> Filed both patches as issue #3193 with milestone 1.6-consider. Troy, I
>> didn't CC or assign to you because I don't know your tigris username.
>
> Neither do I as I don't have one!
Ooooh, can you get one? It's easy, and... Resistance Is Futile, You
Will Be Assimilated.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH 1 of 2] Add peg revision support to svn_opt__arg_canonicalize_*()
Posted by Troy Curtis Jr <tr...@gmail.com>.
On Sat, May 10, 2008 at 11:08 AM, Daniel Shahaf <d....@daniel.shahaf.co.il> wrote:
> 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.
>>
>
> Filed both patches as issue #3193 with milestone 1.6-consider. Troy, I
> didn't CC or assign to you because I don't know your tigris username.
>
Neither do I as I don't have one!
I'm almost done with a patch which incorporates your suggestion of
putting my split_arg_at_peg_revision() into svn_opt_parse_path().
Troy
> http://subversion.tigris.org/issues/show_bug.cgi?id=3193
>
> Daniel
>
>> [[[
>> 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.
>> (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.
>> ]]]
>>
>>
>
--
"Beware of spyware. If you can, use the Firefox browser." - USA Today
Download now at http://getfirefox.com
Registered Linux User #354814 ( http://counter.li.org/)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH 1 of 2] Add peg revision support to
svn_opt__arg_canonicalize_*()
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
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.
>
Filed both patches as issue #3193 with milestone 1.6-consider. Troy, I
didn't CC or assign to you because I don't know your tigris username.
http://subversion.tigris.org/issues/show_bug.cgi?id=3193
Daniel
> [[[
> 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.
> (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.
> ]]]
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org