You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Blair Zajac <bl...@orcaware.com> on 2007/10/16 21:21:06 UTC

svn_opt_parse_path peg revision inconsistency

If you pass a path to svn_opt_parse_path with no trailing @, then the revision 
is svn_opt_revision_unspecified:

   /* Didn't find an @-sign. */
   *truepath = svn_path_canonicalize(path, pool);
   rev->kind = svn_opt_revision_unspecified;

but if you have path with a trailing @ just to protect another @ earlier in the 
path, then it's set to either svn_opt_revision_head or svn_opt_revision_base 
based if the path is an URL or not:

           if (path[i + 1] == '\0')  /* looking at empty peg revision */
             {
               ret = 0;
               start_revision.kind = is_url ? svn_opt_revision_head
                                            : svn_opt_revision_base;
             }

           ...
           ...

           rev->kind = start_revision.kind;
           rev->value = start_revision.value;

Shouldn't the @ protection just use svn_opt_revision_unspecified?

Blair

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_opt_parse_path peg revision inconsistency

Posted by Blair Zajac <bl...@orcaware.com>.
Blair Zajac wrote:
> Karl Fogel wrote:
>> Blair Zajac <bl...@orcaware.com> writes:
>>> Looking through the that allowed one to protect a @ in the path or
>>> URL, do you recall why the code results in a different revision type
>>> for
>>>
>>> foo@ [base or head depending upon path or URL]
>>>
>>> versus
>>>
>>> foo [always unspecified]
>>
>> No, I don't remember any particular reason.  That is, I can understand
>> why, *if* one is going to make foo@ have an explicit revision at all,
>> that it should be BASE for a local path and HEAD for a url.  But I
>> don't understand why an explicit revision is necessary in the first
>> place -- which I gather is the question you're asking :-).
>>
>> Not sure what I was thinking.  If the tests pass with unspecified
>> revision, then +1 on your change.
> 
> The test suite passes completely with the change, so I'll make the commit.

Committed in r27268.

Blair

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_opt_parse_path peg revision inconsistency

Posted by Blair Zajac <bl...@orcaware.com>.
Karl Fogel wrote:
> Blair Zajac <bl...@orcaware.com> writes:
>> Looking through the that allowed one to protect a @ in the path or
>> URL, do you recall why the code results in a different revision type
>> for
>>
>> foo@ [base or head depending upon path or URL]
>>
>> versus
>>
>> foo [always unspecified]
> 
> No, I don't remember any particular reason.  That is, I can understand
> why, *if* one is going to make foo@ have an explicit revision at all,
> that it should be BASE for a local path and HEAD for a url.  But I
> don't understand why an explicit revision is necessary in the first
> place -- which I gather is the question you're asking :-).
> 
> Not sure what I was thinking.  If the tests pass with unspecified
> revision, then +1 on your change.

The test suite passes completely with the change, so I'll make the commit.

Blair

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_opt_parse_path peg revision inconsistency

Posted by Karl Fogel <kf...@red-bean.com>.
Blair Zajac <bl...@orcaware.com> writes:
> Looking through the that allowed one to protect a @ in the path or
> URL, do you recall why the code results in a different revision type
> for
>
> foo@ [base or head depending upon path or URL]
>
> versus
>
> foo [always unspecified]

No, I don't remember any particular reason.  That is, I can understand
why, *if* one is going to make foo@ have an explicit revision at all,
that it should be BASE for a local path and HEAD for a url.  But I
don't understand why an explicit revision is necessary in the first
place -- which I gather is the question you're asking :-).

Not sure what I was thinking.  If the tests pass with unspecified
revision, then +1 on your change.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_opt_parse_path peg revision inconsistency

Posted by Blair Zajac <bl...@orcaware.com>.
A patch making the suggested change and the necessary tweak to 
subversion/tests/libsvn_subr/opt-test.c passed the test suite completely using 
fsfs with ra_local.

Blair

Blair Zajac wrote:
> Karl,
> 
> Looks like you made the below discussed change to be able to protect peg 
> revisions.
> 
> Looking through the that allowed one to protect a @ in the path or URL, 
> do you recall why the code results in a different revision type for
> 
> foo@ [base or head depending upon path or URL]
> 
> versus
> 
> foo [always unspecified]
> 
> http://subversion.tigris.org/servlets/ReadMsg?list=users&msgNo=32503
> 
> r15289 | kfogel | 2005-07-07 12:20:02 -0700 (Thu, 07 Jul 2005) | 17 lines
> 
> Fix issue #2317: Handle filenames containing '@' compatibly with peg
> revision parsing.
> 
> Patch by: Alexander Thomas <al...@collab.net>
>           me
> 
> * subversion/libsvn_subr/opt.c
>   (svn_opt_parse_path): Treat empty peg revision suffix '@' as
>   '@HEAD' or '@BASE', depending on URL or local path respectively.
> 
> Blair
> 
> C. Michael Pilato wrote:
>> Blair Zajac wrote:
>>> If you pass a path to svn_opt_parse_path with no trailing @, then the
>>> revision is svn_opt_revision_unspecified:
>>>
>>>   /* Didn't find an @-sign. */
>>>   *truepath = svn_path_canonicalize(path, pool);
>>>   rev->kind = svn_opt_revision_unspecified;
>>>
>>> but if you have path with a trailing @ just to protect another @ earlier
>>> in the path, then it's set to either svn_opt_revision_head or
>>> svn_opt_revision_base based if the path is an URL or not:
>>>
>>>           if (path[i + 1] == '\0')  /* looking at empty peg revision */
>>>             {
>>>               ret = 0;
>>>               start_revision.kind = is_url ? svn_opt_revision_head
>>>                                            : svn_opt_revision_base;
>>>             }
>>>
>>>           ...
>>>           ...
>>>
>>>           rev->kind = start_revision.kind;
>>>           rev->value = start_revision.value;
>>>
>>> Shouldn't the @ protection just use svn_opt_revision_unspecified?
>>
>> Seems so to me.  That looks like a real bug.
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_opt_parse_path peg revision inconsistency

Posted by Blair Zajac <bl...@orcaware.com>.
Karl,

Looks like you made the below discussed change to be able to protect peg revisions.

Looking through the that allowed one to protect a @ in the path or URL, do you 
recall why the code results in a different revision type for

foo@ [base or head depending upon path or URL]

versus

foo [always unspecified]

http://subversion.tigris.org/servlets/ReadMsg?list=users&msgNo=32503

r15289 | kfogel | 2005-07-07 12:20:02 -0700 (Thu, 07 Jul 2005) | 17 lines

Fix issue #2317: Handle filenames containing '@' compatibly with peg
revision parsing.

Patch by: Alexander Thomas <al...@collab.net>
           me

* subversion/libsvn_subr/opt.c
   (svn_opt_parse_path): Treat empty peg revision suffix '@' as
   '@HEAD' or '@BASE', depending on URL or local path respectively.

Blair

C. Michael Pilato wrote:
> Blair Zajac wrote:
>> If you pass a path to svn_opt_parse_path with no trailing @, then the
>> revision is svn_opt_revision_unspecified:
>>
>>   /* Didn't find an @-sign. */
>>   *truepath = svn_path_canonicalize(path, pool);
>>   rev->kind = svn_opt_revision_unspecified;
>>
>> but if you have path with a trailing @ just to protect another @ earlier
>> in the path, then it's set to either svn_opt_revision_head or
>> svn_opt_revision_base based if the path is an URL or not:
>>
>>           if (path[i + 1] == '\0')  /* looking at empty peg revision */
>>             {
>>               ret = 0;
>>               start_revision.kind = is_url ? svn_opt_revision_head
>>                                            : svn_opt_revision_base;
>>             }
>>
>>           ...
>>           ...
>>
>>           rev->kind = start_revision.kind;
>>           rev->value = start_revision.value;
>>
>> Shouldn't the @ protection just use svn_opt_revision_unspecified?
> 
> Seems so to me.  That looks like a real bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_opt_parse_path peg revision inconsistency

Posted by "C. Michael Pilato" <cm...@collab.net>.
Blair Zajac wrote:
> If you pass a path to svn_opt_parse_path with no trailing @, then the
> revision is svn_opt_revision_unspecified:
> 
>   /* Didn't find an @-sign. */
>   *truepath = svn_path_canonicalize(path, pool);
>   rev->kind = svn_opt_revision_unspecified;
> 
> but if you have path with a trailing @ just to protect another @ earlier
> in the path, then it's set to either svn_opt_revision_head or
> svn_opt_revision_base based if the path is an URL or not:
> 
>           if (path[i + 1] == '\0')  /* looking at empty peg revision */
>             {
>               ret = 0;
>               start_revision.kind = is_url ? svn_opt_revision_head
>                                            : svn_opt_revision_base;
>             }
> 
>           ...
>           ...
> 
>           rev->kind = start_revision.kind;
>           rev->value = start_revision.value;
> 
> Shouldn't the @ protection just use svn_opt_revision_unspecified?

Seems so to me.  That looks like a real bug.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand