diff mbox

[2/3] manual: cvs: document that a date can be used instead of a tag

Message ID CAHkwnC9rcyr4Y+ydSYiDBY2x=+3QLfGapr75E3+Do+69wbWF=Q@mail.gmail.com
State Not Applicable
Headers show

Commit Message

Fabio Porcedda April 20, 2015, 2:36 a.m. UTC
On Sun, Apr 19, 2015 at 5:45 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 19/04/15 12:08, Fabio Porcedda wrote:
>> On Sun, Apr 19, 2015 at 10:10 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>> Fabio, All,
>>>
>>> On 2015-04-18 18:54 +0200, Fabio Porcedda spake thusly:
>>>> Also instead of using the generic word "timestamp" use the word "tag".
>>>>
>>>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>>>> ---
>>>>  docs/manual/adding-packages-generic.txt | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
>>>> index 8cf6bb6..be754a2 100644
>>>> --- a/docs/manual/adding-packages-generic.txt
>>>> +++ b/docs/manual/adding-packages-generic.txt
>>>> @@ -280,7 +280,7 @@ information is (assuming the package name is +libfoo+) :
>>>>       Only anonymous pserver mode is supported.
>>>>       +LIBFOO_SITE+ 'must' contain the source URL as well as the remote
>>>>       repository directory. The module is the package name.
>>>> -     +LIBFOO_VERSION+ is 'mandatory' and 'must' be a timestamp.
>>>> +     +LIBFOO_VERSION+ is 'mandatory' and 'must' be a tag or a date.
>>>
>>> I'd like we document the format of the date we recognise. Because, IIRC,
>>> cvs accepts 'yesterday' as a date format, and that would be interpreted
>>> as a tag with the current code.
>>>
>>> So maybe, just state something like:
>>>
>>>     ... or a date (YYYYMMDD:hhmmss)
>>
>> What about this:
>> (<YYYY>-<MM>-<DD>[T<HH><MM>[<SS>]] e.g 2015-12-20 or 2015-12-20T1010)
>
>  Can a timezone be added to that? Playing with dates and times without timezone
> is dangerous.

This one works: (<YYYY>-<MM>-<DD>[T<HH>:<MM>[:<SS>][-<ZZ>] e.g
2015-12-20 or 2015-12-20T10:10-00)

But to made it works I must replace the ":" with "_":

index bdb26b8..2beb529 100644
BR

Comments

Arnout Vandecappelle April 20, 2015, 9:06 p.m. UTC | #1
On 20/04/15 04:36, Fabio Porcedda wrote:
> On Sun, Apr 19, 2015 at 5:45 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>> On 19/04/15 12:08, Fabio Porcedda wrote:
>>> On Sun, Apr 19, 2015 at 10:10 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>>> Fabio, All,
>>>>
>>>> On 2015-04-18 18:54 +0200, Fabio Porcedda spake thusly:
>>>>> Also instead of using the generic word "timestamp" use the word "tag".
>>>>>
>>>>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>>>>> ---
>>>>>  docs/manual/adding-packages-generic.txt | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
>>>>> index 8cf6bb6..be754a2 100644
>>>>> --- a/docs/manual/adding-packages-generic.txt
>>>>> +++ b/docs/manual/adding-packages-generic.txt
>>>>> @@ -280,7 +280,7 @@ information is (assuming the package name is +libfoo+) :
>>>>>       Only anonymous pserver mode is supported.
>>>>>       +LIBFOO_SITE+ 'must' contain the source URL as well as the remote
>>>>>       repository directory. The module is the package name.
>>>>> -     +LIBFOO_VERSION+ is 'mandatory' and 'must' be a timestamp.
>>>>> +     +LIBFOO_VERSION+ is 'mandatory' and 'must' be a tag or a date.
>>>>
>>>> I'd like we document the format of the date we recognise. Because, IIRC,
>>>> cvs accepts 'yesterday' as a date format, and that would be interpreted
>>>> as a tag with the current code.
>>>>
>>>> So maybe, just state something like:
>>>>
>>>>     ... or a date (YYYYMMDD:hhmmss)
>>>
>>> What about this:
>>> (<YYYY>-<MM>-<DD>[T<HH><MM>[<SS>]] e.g 2015-12-20 or 2015-12-20T1010)
>>
>>  Can a timezone be added to that? Playing with dates and times without timezone
>> is dangerous.
> 
> This one works: (<YYYY>-<MM>-<DD>[T<HH>:<MM>[:<SS>][-<ZZ>] e.g
> 2015-12-20 or 2015-12-20T10:10-00)

 Well, the example should include a timezone and mention that adding a timezone
is advisable.

 Alternatively, we could specifies that times are in UTC and set TZ=UTC in the
cvs helper.

> 
> But to made it works I must replace the ":" with "_":

 Why? The / substitution is only needed because VERSION is used in filenames,
and : is fine in filenames, no?

 Regards,
 Arnout

> 
> index bdb26b8..2beb529 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -325,14 +325,14 @@ $(2)_RAWNAME                      =  $$(patsubst
> host-%,%,$(1))
>  ifndef $(2)_VERSION
>   ifdef $(3)_VERSION
>    $(2)_DL_VERSION := $$(strip $$($(3)_VERSION))
> -  $(2)_VERSION := $$(subst /,_,$$(strip $$($(3)_VERSION)))
> +  $(2)_VERSION := $$(subst :,_,$$(subst /,_,$$(strip $$($(3)_VERSION))))
>   else
>    $(2)_VERSION = undefined
>    $(2)_DL_VERSION = undefined
>   endif
>  else
>    $(2)_DL_VERSION := $$(strip $$($(2)_VERSION))
> -  $(2)_VERSION := $$(strip $$(subst /,_,$$($(2)_VERSION)))
> +  $(2)_VERSION := $$(strip $$(subst :,_,$$(subst /,_,$$($(2)_VERSION))))
>  endif
> 
> BR
>
Fabio Porcedda April 22, 2015, 8:03 a.m. UTC | #2
On Mon, Apr 20, 2015 at 11:06 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 20/04/15 04:36, Fabio Porcedda wrote:
>> On Sun, Apr 19, 2015 at 5:45 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>>> On 19/04/15 12:08, Fabio Porcedda wrote:
>>>> On Sun, Apr 19, 2015 at 10:10 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>>>>> Fabio, All,
>>>>>
>>>>> On 2015-04-18 18:54 +0200, Fabio Porcedda spake thusly:
>>>>>> Also instead of using the generic word "timestamp" use the word "tag".
>>>>>>
>>>>>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>>>>>> ---
>>>>>>  docs/manual/adding-packages-generic.txt | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
>>>>>> index 8cf6bb6..be754a2 100644
>>>>>> --- a/docs/manual/adding-packages-generic.txt
>>>>>> +++ b/docs/manual/adding-packages-generic.txt
>>>>>> @@ -280,7 +280,7 @@ information is (assuming the package name is +libfoo+) :
>>>>>>       Only anonymous pserver mode is supported.
>>>>>>       +LIBFOO_SITE+ 'must' contain the source URL as well as the remote
>>>>>>       repository directory. The module is the package name.
>>>>>> -     +LIBFOO_VERSION+ is 'mandatory' and 'must' be a timestamp.
>>>>>> +     +LIBFOO_VERSION+ is 'mandatory' and 'must' be a tag or a date.
>>>>>
>>>>> I'd like we document the format of the date we recognise. Because, IIRC,
>>>>> cvs accepts 'yesterday' as a date format, and that would be interpreted
>>>>> as a tag with the current code.
>>>>>
>>>>> So maybe, just state something like:
>>>>>
>>>>>     ... or a date (YYYYMMDD:hhmmss)
>>>>
>>>> What about this:
>>>> (<YYYY>-<MM>-<DD>[T<HH><MM>[<SS>]] e.g 2015-12-20 or 2015-12-20T1010)
>>>
>>>  Can a timezone be added to that? Playing with dates and times without timezone
>>> is dangerous.
>>
>> This one works: (<YYYY>-<MM>-<DD>[T<HH>:<MM>[:<SS>][-<ZZ>] e.g
>> 2015-12-20 or 2015-12-20T10:10-00)
>
>  Well, the example should include a timezone and mention that adding a timezone
> is advisable.

Ok.

>  Alternatively, we could specifies that times are in UTC and set TZ=UTC in the
> cvs helper.

Fine for me.
Yann what do you think about it?

>
>>
>> But to made it works I must replace the ":" with "_":
>
>  Why? The / substitution is only needed because VERSION is used in filenames,
> and : is fine in filenames, no?

The colon is fine for filenames, neverthless if i don't replace it, I
get this error:

$ make expect-source
package/expect/expect.mk:19: *** target pattern contains no '%'.  Stop.

>> index bdb26b8..2beb529 100644
>> --- a/package/pkg-generic.mk
>> +++ b/package/pkg-generic.mk
>> @@ -325,14 +325,14 @@ $(2)_RAWNAME                      =  $$(patsubst
>> host-%,%,$(1))
>>  ifndef $(2)_VERSION
>>   ifdef $(3)_VERSION
>>    $(2)_DL_VERSION := $$(strip $$($(3)_VERSION))
>> -  $(2)_VERSION := $$(subst /,_,$$(strip $$($(3)_VERSION)))
>> +  $(2)_VERSION := $$(subst :,_,$$(subst /,_,$$(strip $$($(3)_VERSION))))
>>   else
>>    $(2)_VERSION = undefined
>>    $(2)_DL_VERSION = undefined
>>   endif
>>  else
>>    $(2)_DL_VERSION := $$(strip $$($(2)_VERSION))
>> -  $(2)_VERSION := $$(strip $$(subst /,_,$$($(2)_VERSION)))
>> +  $(2)_VERSION := $$(strip $$(subst :,_,$$(subst /,_,$$($(2)_VERSION))))
>>  endif

BR
Yann E. MORIN April 22, 2015, 4:29 p.m. UTC | #3
Fabio, Gustavo, All,

On 2015-04-22 10:03 +0200, Fabio Porcedda spake thusly:
> On Mon, Apr 20, 2015 at 11:06 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> > On 20/04/15 04:36, Fabio Porcedda wrote:
[--SNIP--]
> >> This one works: (<YYYY>-<MM>-<DD>[T<HH>:<MM>[:<SS>][-<ZZ>] e.g
> >> 2015-12-20 or 2015-12-20T10:10-00)
> >
> >  Well, the example should include a timezone and mention that adding a timezone
> > is advisable.
> Ok.
> 
> >  Alternatively, we could specifies that times are in UTC and set TZ=UTC in the
> > cvs helper.
> 
> Fine for me.
> Yann what do you think about it?

I don't care. But what if a user specifies a timezone?

The tricky part with dealing with date (aka human date and timei) is
exactly that: having to deal with it. It is very complex to parse a date
in a reliable manner.

So we can't really validate a date. I'd rather we allow the full range
of dates supported by cvs, and let it deal with whatever the user
provides, and if that's incorrect, let cvs fail, not us (well, we'd
eventually fail, but not on our will).

> >> But to made it works I must replace the ":" with "_":
> >
> >  Why? The / substitution is only needed because VERSION is used in filenames,
> > and : is fine in filenames, no?
> 
> The colon is fine for filenames, neverthless if i don't replace it, I
> get this error:
> 
> $ make expect-source
> package/expect/expect.mk:19: *** target pattern contains no '%'.  Stop.

Yes, because the $(@D) contains the version string, and it is a make
goal. So, we'd end up wth something like;

    foo-2015-04-22T18:26/.stamp_extracted:
        blabla

Notice that there are two columns, and that is not valid make syntax
(AFAIR).

Which make me think: "2015-04-22 18:26" is an equally valid date, and we
do not really support spaces in paths, so we'd have to also replaces
spaces as well.

Regards,
Yann E. MORIN.
Arnout Vandecappelle April 22, 2015, 8:27 p.m. UTC | #4
On 04/22/15 18:29, Yann E. MORIN wrote:
> Fabio, Gustavo, All,
> 
> On 2015-04-22 10:03 +0200, Fabio Porcedda spake thusly:
>> On Mon, Apr 20, 2015 at 11:06 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>>> On 20/04/15 04:36, Fabio Porcedda wrote:
> [--SNIP--]
>>>> This one works: (<YYYY>-<MM>-<DD>[T<HH>:<MM>[:<SS>][-<ZZ>] e.g
>>>> 2015-12-20 or 2015-12-20T10:10-00)
>>>
>>>  Well, the example should include a timezone and mention that adding a timezone
>>> is advisable.
>> Ok.
>>
>>>  Alternatively, we could specifies that times are in UTC and set TZ=UTC in the
>>> cvs helper.
>>
>> Fine for me.
>> Yann what do you think about it?
> 
> I don't care. But what if a user specifies a timezone?

 Then the user-specified timezone overrides the environment, so the user gets
exactly what he expects. So I think setting TZ=UTC is the best option.

> 
> The tricky part with dealing with date (aka human date and timei) is
> exactly that: having to deal with it. It is very complex to parse a date
> in a reliable manner.
> 
> So we can't really validate a date. I'd rather we allow the full range
> of dates supported by cvs, and let it deal with whatever the user
> provides, and if that's incorrect, let cvs fail, not us (well, we'd
> eventually fail, but not on our will).

 Well, it was never the intention that we would parse it, just that we provide
an example of a valid date.

> 
>>>> But to made it works I must replace the ":" with "_":
>>>
>>>  Why? The / substitution is only needed because VERSION is used in filenames,
>>> and : is fine in filenames, no?
>>
>> The colon is fine for filenames, neverthless if i don't replace it, I
>> get this error:
>>
>> $ make expect-source
>> package/expect/expect.mk:19: *** target pattern contains no '%'.  Stop.
> 
> Yes, because the $(@D) contains the version string, and it is a make
> goal. So, we'd end up wth something like;
> 
>     foo-2015-04-22T18:26/.stamp_extracted:
>         blabla

 Actually it's in this rule:

$(1)-install-target:            $$($(2)_TARGET_INSTALL_TARGET)
which expands to
foo-install-target: foo-2015-04-22T18:26/.stamp_target_installed

> 
> Notice that there are two columns, and that is not valid make syntax
> (AFAIR).
> 
> Which make me think: "2015-04-22 18:26" is an equally valid date, and we
> do not really support spaces in paths, so we'd have to also replaces
> spaces as well.

 Right!


 Regards,
 Arnout

> 
> Regards,
> Yann E. MORIN.
>
diff mbox

Patch

--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -325,14 +325,14 @@  $(2)_RAWNAME                      =  $$(patsubst
host-%,%,$(1))
 ifndef $(2)_VERSION
  ifdef $(3)_VERSION
   $(2)_DL_VERSION := $$(strip $$($(3)_VERSION))
-  $(2)_VERSION := $$(subst /,_,$$(strip $$($(3)_VERSION)))
+  $(2)_VERSION := $$(subst :,_,$$(subst /,_,$$(strip $$($(3)_VERSION))))
  else
   $(2)_VERSION = undefined
   $(2)_DL_VERSION = undefined
  endif
 else
   $(2)_DL_VERSION := $$(strip $$($(2)_VERSION))
-  $(2)_VERSION := $$(strip $$(subst /,_,$$($(2)_VERSION)))
+  $(2)_VERSION := $$(strip $$(subst :,_,$$(subst /,_,$$($(2)_VERSION))))
 endif