[v2,1/2] dirty version string: Don't use non-posix arguments for date date will output a date like 2024-06-05T17:06:30EDT

20326 diff mbox series

(time zone is different than currently specified)
---
 utils/gen-version.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi Zach,

Thanks for splitting this out. I'll start with this one.

Firstly and most importantly we need a Signed-off-by: tag from you, or
we can't merge the patch. It has to match your 'author' tag.

You can reply to this mail to provide it without needing to resend the
patch. (I can't write that one for you :D)

Next - we would always form the commit messages slightly differently to
fully justify the change. I'll propose the following which could be
added if you're happy:

"""
utils: gen-version: Use posix compliant date

The version string of libcamera is presently appended with a date/time
argument in the case of a dirty tree to show further detail of when it
was built. This string is generated with the iso-8601 parameter to
'date' and produces a date string in the form:
"2024-06-17T22:15:30+01:00"

Strict posix shells or implementations of 'date' which may be optimised
for size such as the one provided by busybox or Alpine Linux may not
support the '--iso-8601' flag.

To support builds on those platforms, use a direct date-format string
instead matching as closely as possible to the iso-8601 definition.

An exact match is not possible with those restrictions so 'date
%Y-%m-%dT%H:%M:%S%Z' is used which produces 2024-06-17T22:15:30BST.

The use of the human readable timezone identifier provides a friendlier
output for the string than a timezone offset at the expense of being
less sortable in the exceptionally low risk and unlikely event of two
custom builds being compared at the same time of different timezones.
"""




Quoting Zach DeCook (2024-06-15 12:12:43)
> (time zone is different than currently specified)
> ---
>  utils/gen-version.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utils/gen-version.sh b/utils/gen-version.sh
> index e1f7ca7b..1b818e9e 100755
> --- a/utils/gen-version.sh
> +++ b/utils/gen-version.sh
> @@ -42,7 +42,7 @@ if [ -z "$build_dir" ] || (echo "$build_dir" | grep -q "$src_dir")
>  then
>         git update-index --refresh > /dev/null 2>&1
>  fi
> -git diff-index --quiet HEAD || version="$version-dirty ($(date --iso-8601=seconds))"
> +git diff-index --quiet HEAD || version="$version-dirty ($(date +%Y-%m-%dT%H:%M:%S%Z))"
>  
>  # If a project version is provided, use it to replace the version number.
>  if [ -n "$project_version" ]
> -- 
> 2.45.2
>

Quoting Kieran Bingham (2024-06-17 22:25:45)
> Hi Zach,
> 
> Thanks for splitting this out. I'll start with this one.
> 
> Firstly and most importantly we need a Signed-off-by: tag from you, or
> we can't merge the patch. It has to match your 'author' tag.
> 
> You can reply to this mail to provide it without needing to resend the
> patch. (I can't write that one for you :D)
> 
> Next - we would always form the commit messages slightly differently to
> fully justify the change. I'll propose the following which could be
> added if you're happy:
> 
> """
> utils: gen-version: Use posix compliant date
> 
> The version string of libcamera is presently appended with a date/time
> argument in the case of a dirty tree to show further detail of when it
> was built. This string is generated with the iso-8601 parameter to
> 'date' and produces a date string in the form:
> "2024-06-17T22:15:30+01:00"
> 
> Strict posix shells or implementations of 'date' which may be optimised
> for size such as the one provided by busybox or Alpine Linux may not
> support the '--iso-8601' flag.
> 
> To support builds on those platforms, use a direct date-format string
> instead matching as closely as possible to the iso-8601 definition.
> 
> An exact match is not possible with those restrictions so 'date
> %Y-%m-%dT%H:%M:%S%Z' is used which produces 2024-06-17T22:15:30BST.
> 
> The use of the human readable timezone identifier provides a friendlier
> output for the string than a timezone offset at the expense of being
> less sortable in the exceptionally low risk and unlikely event of two
> custom builds being compared at the same time of different timezones.
> """
> 

Err, and with that I could add 


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> 
> 
> Quoting Zach DeCook (2024-06-15 12:12:43)
> > (time zone is different than currently specified)
> > ---
> >  utils/gen-version.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/utils/gen-version.sh b/utils/gen-version.sh
> > index e1f7ca7b..1b818e9e 100755
> > --- a/utils/gen-version.sh
> > +++ b/utils/gen-version.sh
> > @@ -42,7 +42,7 @@ if [ -z "$build_dir" ] || (echo "$build_dir" | grep -q "$src_dir")
> >  then
> >         git update-index --refresh > /dev/null 2>&1
> >  fi
> > -git diff-index --quiet HEAD || version="$version-dirty ($(date --iso-8601=seconds))"
> > +git diff-index --quiet HEAD || version="$version-dirty ($(date +%Y-%m-%dT%H:%M:%S%Z))"
> >  
> >  # If a project version is provided, use it to replace the version number.
> >  if [ -n "$project_version" ]
> > -- 
> > 2.45.2
> >

On Mon, Jun 17, 2024 at 10:25:45PM +0100, Kieran Bingham wrote:
> Hi Zach,
> 
> Thanks for splitting this out. I'll start with this one.
> 
> Firstly and most importantly we need a Signed-off-by: tag from you, or
> we can't merge the patch. It has to match your 'author' tag.
> 
> You can reply to this mail to provide it without needing to resend the
> patch. (I can't write that one for you :D)

https://libcamera.org/contributing.html#developer-s-certificate-of-origin
explains what the tag means.

> Next - we would always form the commit messages slightly differently to
> fully justify the change. I'll propose the following which could be
> added if you're happy:
> 
> """
> utils: gen-version: Use posix compliant date
> 
> The version string of libcamera is presently appended with a date/time
> argument in the case of a dirty tree to show further detail of when it
> was built. This string is generated with the iso-8601 parameter to
> 'date' and produces a date string in the form:
> "2024-06-17T22:15:30+01:00"
> 
> Strict posix shells or implementations of 'date' which may be optimised
> for size such as the one provided by busybox or Alpine Linux may not
> support the '--iso-8601' flag.
> 
> To support builds on those platforms, use a direct date-format string

s/direct/direct POSIX-compliant/ maybe

> instead matching as closely as possible to the iso-8601 definition.
> 
> An exact match is not possible with those restrictions so 'date
> %Y-%m-%dT%H:%M:%S%Z' is used which produces 2024-06-17T22:15:30BST.
> 
> The use of the human readable timezone identifier provides a friendlier
> output for the string than a timezone offset at the expense of being
> less sortable in the exceptionally low risk and unlikely event of two
> custom builds being compared at the same time of different timezones.
> """

With that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> Quoting Zach DeCook (2024-06-15 12:12:43)
> > (time zone is different than currently specified)
> > ---
> >  utils/gen-version.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/utils/gen-version.sh b/utils/gen-version.sh
> > index e1f7ca7b..1b818e9e 100755
> > --- a/utils/gen-version.sh
> > +++ b/utils/gen-version.sh
> > @@ -42,7 +42,7 @@ if [ -z "$build_dir" ] || (echo "$build_dir" | grep -q "$src_dir")
> >  then
> >         git update-index --refresh > /dev/null 2>&1
> >  fi
> > -git diff-index --quiet HEAD || version="$version-dirty ($(date --iso-8601=seconds))"
> > +git diff-index --quiet HEAD || version="$version-dirty ($(date +%Y-%m-%dT%H:%M:%S%Z))"
> >  
> >  # If a project version is provided, use it to replace the version number.
> >  if [ -n "$project_version" ]

On Mon Jun 17, 2024 at 5:48 PM EDT, Laurent Pinchart wrote:
> On Mon, Jun 17, 2024 at 10:25:45PM +0100, Kieran Bingham wrote:
> > Hi Zach,
> > 
> > Thanks for splitting this out. I'll start with this one.
> > 
> > Firstly and most importantly we need a Signed-off-by: tag from you, or
> > we can't merge the patch. It has to match your 'author' tag.
> > 
> > You can reply to this mail to provide it without needing to resend the
> > patch. (I can't write that one for you :D)
>
> https://libcamera.org/contributing.html#developer-s-certificate-of-origin
> explains what the tag means.
>
> s/direct/direct POSIX-compliant/ maybe
>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

That looks good, please add my sign off to it, and I'll be sure to include that in the future.

Signed-off-by: Zach DeCook <zachdecook@librem.one>