[libcamera-devel] meson: Fix git version parsing

18377 diff mbox series

When extracting the build metadata from the git version, we use the
string strip() method to remove the version prefix. This is incorrect,
as the strip() method takes a set of characters to be removed, not a
literal string. Fix it by splitting the git version string on the '+'
character and keeping the suffix.

Fixes: 02518e598e8f ("meson: Rewrite .replace usage")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 meson.build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


base-commit: f852b7fbc4960ea83bab49b75408fb13462db8ba

Quoting Laurent Pinchart (2023-03-09 17:21:15)
> When extracting the build metadata from the git version, we use the
> string strip() method to remove the version prefix. This is incorrect,
> as the strip() method takes a set of characters to be removed, not a
> literal string. Fix it by splitting the git version string on the '+'
> character and keeping the suffix.
> 
> Fixes: 02518e598e8f ("meson: Rewrite .replace usage")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  meson.build | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 0f89b45a0169..189e97736d90 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -41,8 +41,8 @@ if libcamera_version != project_version
>  
>      # Replace the version components reported by git with the release version,
>      # but keep all trailing information supplied by git.
> -    libcamera_git_version = (project_version +
> -                             libcamera_git_version.strip(libcamera_version))
> +    libcamera_git_version = (project_version + '+' +
> +                             libcamera_git_version.split('+')[1])

Looks too obvious. Why didn't I use this in the first place ;-)

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

>      libcamera_version = project_version
>  
>      # Append a marker to show we have modified this version string
> 
> base-commit: f852b7fbc4960ea83bab49b75408fb13462db8ba
> -- 
> Regards,
> 
> Laurent Pinchart
>

Hi Laurant, Kieran,

On 3/9/23 18:37, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2023-03-09 17:21:15)
>> When extracting the build metadata from the git version, we use the
>> string strip() method to remove the version prefix. This is incorrect,
>> as the strip() method takes a set of characters to be removed, not a
>> literal string. Fix it by splitting the git version string on the '+'
>> character and keeping the suffix.
>>
>> Fixes: 02518e598e8f ("meson: Rewrite .replace usage")
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> ---
>>  meson.build | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index 0f89b45a0169..189e97736d90 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -41,8 +41,8 @@ if libcamera_version != project_version
>>  
>>      # Replace the version components reported by git with the release version,
>>      # but keep all trailing information supplied by git.
>> -    libcamera_git_version = (project_version +
>> -                             libcamera_git_version.strip(libcamera_version))
>> +    libcamera_git_version = (project_version + '+' +
>> +                             libcamera_git_version.split('+')[1])

Is it safe to assume that metadata is present in this case, i.e., the
list produced by libcamera_git_version.split('+') contains at least two
entries?

With that addressed:

Reviewed-by: Michael Riesch <michael.riesch@wolfvision.net>

Best regards,
Michael

> 
> Looks too obvious. Why didn't I use this in the first place ;-)
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
>>      libcamera_version = project_version
>>  
>>      # Append a marker to show we have modified this version string
>>
>> base-commit: f852b7fbc4960ea83bab49b75408fb13462db8ba
>> -- 
>> Regards,
>>
>> Laurent Pinchart
>>

Hi Michael,

On Fri, Mar 10, 2023 at 06:00:43AM +0100, Michael Riesch wrote:
> On 3/9/23 18:37, Kieran Bingham wrote:
> > Quoting Laurent Pinchart (2023-03-09 17:21:15)
> >> When extracting the build metadata from the git version, we use the
> >> string strip() method to remove the version prefix. This is incorrect,
> >> as the strip() method takes a set of characters to be removed, not a
> >> literal string. Fix it by splitting the git version string on the '+'
> >> character and keeping the suffix.
> >>
> >> Fixes: 02518e598e8f ("meson: Rewrite .replace usage")
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> ---
> >>  meson.build | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/meson.build b/meson.build
> >> index 0f89b45a0169..189e97736d90 100644
> >> --- a/meson.build
> >> +++ b/meson.build
> >> @@ -41,8 +41,8 @@ if libcamera_version != project_version
> >>  
> >>      # Replace the version components reported by git with the release version,
> >>      # but keep all trailing information supplied by git.
> >> -    libcamera_git_version = (project_version +
> >> -                             libcamera_git_version.strip(libcamera_version))
> >> +    libcamera_git_version = (project_version + '+' +
> >> +                             libcamera_git_version.split('+')[1])
> 
> Is it safe to assume that metadata is present in this case, i.e., the
> list produced by libcamera_git_version.split('+') contains at least two
> entries?

Good point. I don't think it should happen in practice, as this code is
only reached when the meson and git version don't match, and in that
case I wouldn't expect the branch HEAD to point to a tag, so there
should be a '+' sign. Nonetheless, I think we should fix it.

There's also an issue if the string contains multiple '+', which can be
the case when the build metadata contain a timestamp with a timezone.
meson doesn't support arrays slicing (e.g. .split('+')[1:]), and has no
string .length() method that would allow us to use .substring(). I'll
see what I can do.

> With that addressed:
> 
> Reviewed-by: Michael Riesch <michael.riesch@wolfvision.net>
> 
> Best regards,
> Michael
> 
> > Looks too obvious. Why didn't I use this in the first place ;-)
> > 
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> >>      libcamera_version = project_version
> >>  
> >>      # Append a marker to show we have modified this version string
> >>
> >> base-commit: f852b7fbc4960ea83bab49b75408fb13462db8ba