You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Hyrum K. Wright" <hy...@mail.utexas.edu> on 2007/01/03 02:48:07 UTC
Peg Revision Syntax
I'm adding support for peg revision syntax to 'svn copy' and I just want
to make sure that I've got things straight with regard to peg revisions
(peg revs), and operational revisions (op revs).
Each source needs both a peg rev, and an op rev, which may be different.
Depending on which combinations of peg rev and op rev happen several
scenarios are possible. By my count, there are four possible cases for
each source:
1) No peg rev, no op rev
Example: 'svn cp src dst'
Peg rev: HEAD
Op rev: HEAD
2) No peg rev, op rev
Example: 'svn cp src dst -r 200'
Peg rev: 200
Op rev: 200
3) peg rev, no op rev
Example: 'svn cp src@300 dst'
Peg rev: 300
Op rev: 300
4) peg rev and op rev
Example: 'svn cp src@300 dst -r 200'
Peg rev: 300
Op rev: 200
Each of these can be easily extended for multiple sources.
Is this correct? Are there other cases which I need to take into account?
-Hyrum
Re: Peg Revision Syntax
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
C. Michael Pilato wrote:
> Hyrum K. Wright wrote:
>> Index: subversion/libsvn_client/revisions.c
>> ===================================================================
>> --- subversion/libsvn_client/revisions.c (revision 22879)
>> +++ subversion/libsvn_client/revisions.c (working copy)
>> @@ -140,3 +140,24 @@
>> else
>> return TRUE;
>> }
>> +
>> +
>> +svn_error_t *
>> +svn_client__resolve_revisions(svn_opt_revision_t *peg_rev,
>> + svn_opt_revision_t *op_rev,
>> + svn_boolean_t is_url)
>> +{
>> + if (peg_rev->kind == svn_opt_revision_unspecified)
>> + {
>> + if (is_url)
>> + peg_rev->kind = svn_opt_revision_head;
>> + else
>> + peg_rev->kind = svn_opt_revision_base;
>> + }
>
> This looks good.
>
>> +
>> + if (op_rev->kind == svn_opt_revision_unspecified)
>> + op_rev->kind = peg_rev->kind;
>
> But not this. If the op_rev->kind isn't specified, we *do* need to take
> the kind from the peg_rev. But depending on peg_rev's kind, might we
> also need to copy its ->value field, too?
>
> What about this?
>
> if (op_rev->kind == svn_opt_revision_unspecified)
> *op_rev = *peg_rev;
Good point, I'll use that instead. Thanks!
-Hyrum
Re: Peg Revision Syntax
Posted by "C. Michael Pilato" <cm...@collab.net>.
Hyrum K. Wright wrote:
> C. Michael Pilato wrote:
>> Hyrum K. Wright wrote:
>>> Ben Collins-Sussman wrote:
>>>> Actually, it's already documented in the svnbook:
>>>>
>>>> http://svnbook.red-bean.com/nightly/en/svn.advanced.pegrevs.html
>>>>
>>>> Notice the sidebar text there: "Remember that even when you don't
>>>> explicitly supply a peg-revision, it's still present. It defaults to
>>>> BASE for working copy items, and to HEAD for URLs."
>>>>
>>>> On 1/3/07, Giovanni Bajo <ra...@develer.com> wrote:
>>>>> Ben Collins-Sussman wrote:
>>>>>
>>>>>> Let me generalize: my understanding is that for *any* command:
>>>>>>
>>>>>> * URLs pegrevs default to HEAD, unless specified by @N
>>>>>> * wc-path pegrevs default to BASE, unless specified by @N
>>>>>> * operational revs default to pegrev, unless specified by -rN
>>>>> This has been discussed in the past:
>>>>> http://svn.haxx.se/dev/archive-2005-11/1303.shtml
>>>>>
>>>>> I would like if it could be eventually set in stone and documented (in
>>>>> the FAQ
>>>>> maybe). I personally value more consistency among different svn commands
>>>>> rather than endless debates on the "best" defaults :)
>>> Thanks for the pointers. Given this information, where should the
>>> defaulting should occur? For 'svn cat' we do it in libsvn_client,
>>> whereas for 'svn log' we do it in the client application itself. It
>>> doesn't appear to be happening in any single location.
>>>
>>> Once we decide on the right layer to default, I'll just move all the
>>> defaulting behavior there, write a helper function and call it good.
>> The libsvn_client APIs should implement the defaulting behavior
>> themselves, subject (of course) to being overridden by callers who
>> provide non-unspecified svn_opt_revision_t's. Why make every client do
>> this work?
>
> That makes sense. Given Ben's generalization above, would something
> like the following work? It just seems so simple, that I'm concerned
> that I'm overlooking something. :)
>
> Index: subversion/libsvn_client/revisions.c
> ===================================================================
> --- subversion/libsvn_client/revisions.c (revision 22879)
> +++ subversion/libsvn_client/revisions.c (working copy)
> @@ -140,3 +140,24 @@
> else
> return TRUE;
> }
> +
> +
> +svn_error_t *
> +svn_client__resolve_revisions(svn_opt_revision_t *peg_rev,
> + svn_opt_revision_t *op_rev,
> + svn_boolean_t is_url)
> +{
> + if (peg_rev->kind == svn_opt_revision_unspecified)
> + {
> + if (is_url)
> + peg_rev->kind = svn_opt_revision_head;
> + else
> + peg_rev->kind = svn_opt_revision_base;
> + }
This looks good.
> +
> + if (op_rev->kind == svn_opt_revision_unspecified)
> + op_rev->kind = peg_rev->kind;
But not this. If the op_rev->kind isn't specified, we *do* need to take
the kind from the peg_rev. But depending on peg_rev's kind, might we
also need to copy its ->value field, too?
What about this?
if (op_rev->kind == svn_opt_revision_unspecified)
*op_rev = *peg_rev;
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Peg Revision Syntax
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
C. Michael Pilato wrote:
> Hyrum K. Wright wrote:
>> Ben Collins-Sussman wrote:
>>> On 1/3/07, Hyrum K. Wright <hy...@mail.utexas.edu> wrote:
>>>
>>>> +svn_error_t *
>>>> +svn_client__resolve_revisions(svn_opt_revision_t *peg_rev,
>>>> + svn_opt_revision_t *op_rev,
>>>> + svn_boolean_t is_url)
>>>> +{
>>>> + if (peg_rev->kind == svn_opt_revision_unspecified)
>>>> + {
>>>> + if (is_url)
>>>> + peg_rev->kind = svn_opt_revision_head;
>>>> + else
>>>> + peg_rev->kind = svn_opt_revision_base;
>> ^
>> Hmmm...on second thought, should this be svn_opt_revision_working, to
>> make sure that we pick up local modifications?
>
> Maybe add a notice_local_mods parameter to help decide between BASE and
> WORKING?
"Let the caller decide." Quite a novel concept! ;)
Thanks for the suggestion, Mike. I'll make the change shortly.
-Hyrum
Re: Peg Revision Syntax
Posted by "C. Michael Pilato" <cm...@collab.net>.
Hyrum K. Wright wrote:
> Ben Collins-Sussman wrote:
>> On 1/3/07, Hyrum K. Wright <hy...@mail.utexas.edu> wrote:
>>
>>> +svn_error_t *
>>> +svn_client__resolve_revisions(svn_opt_revision_t *peg_rev,
>>> + svn_opt_revision_t *op_rev,
>>> + svn_boolean_t is_url)
>>> +{
>>> + if (peg_rev->kind == svn_opt_revision_unspecified)
>>> + {
>>> + if (is_url)
>>> + peg_rev->kind = svn_opt_revision_head;
>>> + else
>>> + peg_rev->kind = svn_opt_revision_base;
> ^
> Hmmm...on second thought, should this be svn_opt_revision_working, to
> make sure that we pick up local modifications?
Maybe add a notice_local_mods parameter to help decide between BASE and
WORKING?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Peg Revision Syntax
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Ben Collins-Sussman wrote:
> On 1/3/07, Hyrum K. Wright <hy...@mail.utexas.edu> wrote:
>
>> +svn_error_t *
>> +svn_client__resolve_revisions(svn_opt_revision_t *peg_rev,
>> + svn_opt_revision_t *op_rev,
>> + svn_boolean_t is_url)
>> +{
>> + if (peg_rev->kind == svn_opt_revision_unspecified)
>> + {
>> + if (is_url)
>> + peg_rev->kind = svn_opt_revision_head;
>> + else
>> + peg_rev->kind = svn_opt_revision_base;
^
Hmmm...on second thought, should this be svn_opt_revision_working, to
make sure that we pick up local modifications?
>> + }
>> +
>> + if (op_rev->kind == svn_opt_revision_unspecified)
>> + op_rev->kind = peg_rev->kind;
>> +
>> + return SVN_NO_ERROR;
>> +}
>> +
Re: Peg Revision Syntax
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Ben Collins-Sussman wrote:
> On 1/3/07, Hyrum K. Wright <hy...@mail.utexas.edu> wrote:
>
>> +svn_error_t *
>> +svn_client__resolve_revisions(svn_opt_revision_t *peg_rev,
>> + svn_opt_revision_t *op_rev,
>> + svn_boolean_t is_url)
>> +{
>> + if (peg_rev->kind == svn_opt_revision_unspecified)
>> + {
>> + if (is_url)
>> + peg_rev->kind = svn_opt_revision_head;
>> + else
>> + peg_rev->kind = svn_opt_revision_base;
>> + }
>> +
>> + if (op_rev->kind == svn_opt_revision_unspecified)
>> + op_rev->kind = peg_rev->kind;
>> +
>> + return SVN_NO_ERROR;
>> +}
>> +
>
>
> It looks right to me! Are you thinking of making all subcommands use
> this function?
Yes, at least all the subcommands which support peg revs.
> We really need a bunch of tests for pegrev/oprevs. :-)
Agreed. Hopefully someone will write some. :)
-Hyrum
Re: Peg Revision Syntax
Posted by Ben Collins-Sussman <su...@red-bean.com>.
On 1/3/07, Hyrum K. Wright <hy...@mail.utexas.edu> wrote:
> +svn_error_t *
> +svn_client__resolve_revisions(svn_opt_revision_t *peg_rev,
> + svn_opt_revision_t *op_rev,
> + svn_boolean_t is_url)
> +{
> + if (peg_rev->kind == svn_opt_revision_unspecified)
> + {
> + if (is_url)
> + peg_rev->kind = svn_opt_revision_head;
> + else
> + peg_rev->kind = svn_opt_revision_base;
> + }
> +
> + if (op_rev->kind == svn_opt_revision_unspecified)
> + op_rev->kind = peg_rev->kind;
> +
> + return SVN_NO_ERROR;
> +}
> +
It looks right to me! Are you thinking of making all subcommands use
this function?
We really need a bunch of tests for pegrev/oprevs. :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Peg Revision Syntax
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
C. Michael Pilato wrote:
> Hyrum K. Wright wrote:
>> Ben Collins-Sussman wrote:
>>> Actually, it's already documented in the svnbook:
>>>
>>> http://svnbook.red-bean.com/nightly/en/svn.advanced.pegrevs.html
>>>
>>> Notice the sidebar text there: "Remember that even when you don't
>>> explicitly supply a peg-revision, it's still present. It defaults to
>>> BASE for working copy items, and to HEAD for URLs."
>>>
>>> On 1/3/07, Giovanni Bajo <ra...@develer.com> wrote:
>>>> Ben Collins-Sussman wrote:
>>>>
>>>>> Let me generalize: my understanding is that for *any* command:
>>>>>
>>>>> * URLs pegrevs default to HEAD, unless specified by @N
>>>>> * wc-path pegrevs default to BASE, unless specified by @N
>>>>> * operational revs default to pegrev, unless specified by -rN
>>>> This has been discussed in the past:
>>>> http://svn.haxx.se/dev/archive-2005-11/1303.shtml
>>>>
>>>> I would like if it could be eventually set in stone and documented (in
>>>> the FAQ
>>>> maybe). I personally value more consistency among different svn commands
>>>> rather than endless debates on the "best" defaults :)
>> Thanks for the pointers. Given this information, where should the
>> defaulting should occur? For 'svn cat' we do it in libsvn_client,
>> whereas for 'svn log' we do it in the client application itself. It
>> doesn't appear to be happening in any single location.
>>
>> Once we decide on the right layer to default, I'll just move all the
>> defaulting behavior there, write a helper function and call it good.
>
> The libsvn_client APIs should implement the defaulting behavior
> themselves, subject (of course) to being overridden by callers who
> provide non-unspecified svn_opt_revision_t's. Why make every client do
> this work?
That makes sense. Given Ben's generalization above, would something
like the following work? It just seems so simple, that I'm concerned
that I'm overlooking something. :)
Index: subversion/libsvn_client/revisions.c
===================================================================
--- subversion/libsvn_client/revisions.c (revision 22879)
+++ subversion/libsvn_client/revisions.c (working copy)
@@ -140,3 +140,24 @@
else
return TRUE;
}
+
+
+svn_error_t *
+svn_client__resolve_revisions(svn_opt_revision_t *peg_rev,
+ svn_opt_revision_t *op_rev,
+ svn_boolean_t is_url)
+{
+ if (peg_rev->kind == svn_opt_revision_unspecified)
+ {
+ if (is_url)
+ peg_rev->kind = svn_opt_revision_head;
+ else
+ peg_rev->kind = svn_opt_revision_base;
+ }
+
+ if (op_rev->kind == svn_opt_revision_unspecified)
+ op_rev->kind = peg_rev->kind;
+
+ return SVN_NO_ERROR;
+}
+
Re: Peg Revision Syntax
Posted by "C. Michael Pilato" <cm...@collab.net>.
Hyrum K. Wright wrote:
> Ben Collins-Sussman wrote:
>> Actually, it's already documented in the svnbook:
>>
>> http://svnbook.red-bean.com/nightly/en/svn.advanced.pegrevs.html
>>
>> Notice the sidebar text there: "Remember that even when you don't
>> explicitly supply a peg-revision, it's still present. It defaults to
>> BASE for working copy items, and to HEAD for URLs."
>>
>> On 1/3/07, Giovanni Bajo <ra...@develer.com> wrote:
>>> Ben Collins-Sussman wrote:
>>>
>>>> Let me generalize: my understanding is that for *any* command:
>>>>
>>>> * URLs pegrevs default to HEAD, unless specified by @N
>>>> * wc-path pegrevs default to BASE, unless specified by @N
>>>> * operational revs default to pegrev, unless specified by -rN
>>> This has been discussed in the past:
>>> http://svn.haxx.se/dev/archive-2005-11/1303.shtml
>>>
>>> I would like if it could be eventually set in stone and documented (in
>>> the FAQ
>>> maybe). I personally value more consistency among different svn commands
>>> rather than endless debates on the "best" defaults :)
>
> Thanks for the pointers. Given this information, where should the
> defaulting should occur? For 'svn cat' we do it in libsvn_client,
> whereas for 'svn log' we do it in the client application itself. It
> doesn't appear to be happening in any single location.
>
> Once we decide on the right layer to default, I'll just move all the
> defaulting behavior there, write a helper function and call it good.
The libsvn_client APIs should implement the defaulting behavior
themselves, subject (of course) to being overridden by callers who
provide non-unspecified svn_opt_revision_t's. Why make every client do
this work?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Peg Revision Syntax
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Ben Collins-Sussman wrote:
> Actually, it's already documented in the svnbook:
>
> http://svnbook.red-bean.com/nightly/en/svn.advanced.pegrevs.html
>
> Notice the sidebar text there: "Remember that even when you don't
> explicitly supply a peg-revision, it's still present. It defaults to
> BASE for working copy items, and to HEAD for URLs."
>
> On 1/3/07, Giovanni Bajo <ra...@develer.com> wrote:
>> Ben Collins-Sussman wrote:
>>
>> > Let me generalize: my understanding is that for *any* command:
>> >
>> > * URLs pegrevs default to HEAD, unless specified by @N
>> > * wc-path pegrevs default to BASE, unless specified by @N
>> > * operational revs default to pegrev, unless specified by -rN
>>
>> This has been discussed in the past:
>> http://svn.haxx.se/dev/archive-2005-11/1303.shtml
>>
>> I would like if it could be eventually set in stone and documented (in
>> the FAQ
>> maybe). I personally value more consistency among different svn commands
>> rather than endless debates on the "best" defaults :)
Thanks for the pointers. Given this information, where should the
defaulting should occur? For 'svn cat' we do it in libsvn_client,
whereas for 'svn log' we do it in the client application itself. It
doesn't appear to be happening in any single location.
Once we decide on the right layer to default, I'll just move all the
defaulting behavior there, write a helper function and call it good.
-Hyrum
Re: Peg Revision Syntax
Posted by Ben Collins-Sussman <su...@red-bean.com>.
Actually, it's already documented in the svnbook:
http://svnbook.red-bean.com/nightly/en/svn.advanced.pegrevs.html
Notice the sidebar text there: "Remember that even when you don't
explicitly supply a peg-revision, it's still present. It defaults to
BASE for working copy items, and to HEAD for URLs."
On 1/3/07, Giovanni Bajo <ra...@develer.com> wrote:
> Ben Collins-Sussman wrote:
>
> > Let me generalize: my understanding is that for *any* command:
> >
> > * URLs pegrevs default to HEAD, unless specified by @N
> > * wc-path pegrevs default to BASE, unless specified by @N
> > * operational revs default to pegrev, unless specified by -rN
>
> This has been discussed in the past:
> http://svn.haxx.se/dev/archive-2005-11/1303.shtml
>
> I would like if it could be eventually set in stone and documented (in the FAQ
> maybe). I personally value more consistency among different svn commands
> rather than endless debates on the "best" defaults :)
> --
> Giovanni Bajo
>
> ---------------------------------------------------------------------
> 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: Peg Revision Syntax
Posted by Giovanni Bajo <ra...@develer.com>.
Ben Collins-Sussman wrote:
> Let me generalize: my understanding is that for *any* command:
>
> * URLs pegrevs default to HEAD, unless specified by @N
> * wc-path pegrevs default to BASE, unless specified by @N
> * operational revs default to pegrev, unless specified by -rN
This has been discussed in the past:
http://svn.haxx.se/dev/archive-2005-11/1303.shtml
I would like if it could be eventually set in stone and documented (in the FAQ
maybe). I personally value more consistency among different svn commands
rather than endless debates on the "best" defaults :)
--
Giovanni Bajo
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Peg Revision Syntax
Posted by Ben Collins-Sussman <su...@red-bean.com>.
On 1/2/07, Hyrum K. Wright <hy...@mail.utexas.edu> wrote:
> I'm adding support for peg revision syntax to 'svn copy' and I just want
> to make sure that I've got things straight with regard to peg revisions
> (peg revs), and operational revisions (op revs).
>
> Each source needs both a peg rev, and an op rev, which may be different.
> Depending on which combinations of peg rev and op rev happen several
> scenarios are possible. By my count, there are four possible cases for
> each source:
>
> 1) No peg rev, no op rev
> Example: 'svn cp src dst'
> Peg rev: HEAD
> Op rev: HEAD
>
> 2) No peg rev, op rev
> Example: 'svn cp src dst -r 200'
> Peg rev: 200
> Op rev: 200
I think this second example is wrong. My understanding is that if the
path doesn't have an explicit pegrev (@), then it defaults to HEAD.
Let me generalize: my understanding is that for *any* command:
* URLs pegrevs default to HEAD, unless specified by @N
* wc-path pegrevs default to BASE, unless specified by @N
* operational revs default to pegrev, unless specified by -rN
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org