[libcamera-devel] [PATCH] meson: Fix git version parsing

[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 at 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 at wolfvision.net>
> 
> Best regards,
> Michael
> 
> > Looks too obvious. Why didn't I use this in the first place ;-)
> > 
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 
> >>      libcamera_version = project_version
> >>  
> >>      # Append a marker to show we have modified this version string
> >>
> >> base-commit: f852b7fbc4960ea83bab49b75408fb13462db8ba

-- 
Regards,

Laurent Pinchart