Message ID | CAHkwnC9rcyr4Y+ydSYiDBY2x=+3QLfGapr75E3+Do+69wbWF=Q@mail.gmail.com |
---|---|
State | Not Applicable |
Headers | show |
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 >
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
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.
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. >
--- 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