[libcamera-devel] [PATCH v2 1/1] build: Preserve upstream git versioning using meson dist

[Naushir Patuck]

Hi Laurent,

Thank you for your feedback.

On Thu, 14 Oct 2021 at 01:46, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> Hi Naush,
>
> (CC'ing Kieran)
>
> Thank you for the patch.
>
> On Wed, Oct 13, 2021 at 01:23:12PM +0100, Naushir Patuck wrote:
> > When distributions build and package libcamera libraries, they may not
> > necessarily run the build in the upstream source tree. In these cases,
> the git
> > SHA1 versioning information will be lost.
> >
> > This change addresses that problem by requiring package managers to run
> > 'meson dist' to create a tarball of the source files and build from
> there.
> > On runing 'meson dist', the utils/run-dist.sh script will create a
> version.gen
> > file in the release tarball with the version string generated from the
> existing
> > utils/gen-version.sh script.
> >
> > The utils/gen-version.sh script has been updated to check for the
> presence of
> > this version.gen file and read the version string from it instead of
> creating
> > one.
>
> I came across
> https://sources.debian.org/src/libsmpp34/1.14.0-2/git-version-gen/ which
> seems to store the version in a file named .tarball-version. Should we
> follow the same convention ?
>

Sure, I can rename it to .tarball-version.


>
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  meson.build               |  3 +++
> >  src/libcamera/meson.build | 11 +++++------
> >  utils/gen-version.sh      |  9 +++++++++
> >  utils/run-dist.sh         | 11 +++++++++++
> >  4 files changed, 28 insertions(+), 6 deletions(-)
> >  create mode 100644 utils/run-dist.sh
> >
> > diff --git a/meson.build b/meson.build
> > index a49c484fe64e..85ca0013733e 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -24,6 +24,9 @@ endif
> >
> >  libcamera_version = libcamera_git_version.split('+')[0]
> >
> > +# This script gererates the version.gen file on a 'meson dist' command.
> > +meson.add_dist_script('utils/run-dist.sh')
> > +
> >  # Configure the build environment.
> >  cc = meson.get_compiler('c')
> >  cxx = meson.get_compiler('cpp')
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index 243dd3c180eb..360eaf80ecf1 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -93,12 +93,11 @@ endforeach
> >
> >  libcamera_sources += control_sources
> >
> > -gen_version = meson.source_root() / 'utils' / 'gen-version.sh'
> > -
> > -version_cpp = vcs_tag(command : [gen_version, meson.build_root()],
> > -                      input : 'version.cpp.in',
> > -                      output : 'version.cpp',
> > -                      fallback : meson.project_version())
> > +version_data = configuration_data()
> > +version_data.set('VCS_TAG', libcamera_git_version)
> > +version_cpp = configure_file(input : 'version.cpp.in',
> > +                             output : 'version.cpp',
> > +                             configuration : version_data)
>
> This doesn't seem strictly needed, but it avoids running gen-version.sh
> a second time, which is nice. I however have a nagging feeling that
> there was a reason why we used vcs_tag and not configure_file, but I
> can't recall it. We'll find out if it causes issues :-)
>

There seems to be a subtle difference from my brief messing around with
this.  configure_file() will execute on a meson config/setup command, and
vcs_tag will run on the ninja build command.  I did have a problem with my
change using the latter (I cannot recall exactly right now, but it was
something
to do with the $MESON_SOURCE_ROOT env variable not setup when running
the gen-version script).  I thought this change would be good as you do not
run
the script twice either.


>
> >
> >  libcamera_sources += version_cpp
> >
> > diff --git a/utils/gen-version.sh b/utils/gen-version.sh
> > index b09ad495f86a..09cede84c25e 100755
> > --- a/utils/gen-version.sh
> > +++ b/utils/gen-version.sh
> > @@ -5,6 +5,15 @@
> >
> >  build_dir="$1"
> >
> > +# If version.gen exists, output the version string from the file and
> exit.
> > +# This file is auto-generated on a 'meson dist' command from the
> run-dist.sh
> > +# script.
> > +if [ -f "${MESON_SOURCE_ROOT}"/version.gen ]
> > +then
> > +     cat "${MESON_SOURCE_ROOT}"/version.gen
> > +     exit 0
> > +fi
> > +
>
> Shouldn't we do this only if we're not under git control ? There should
> be no version.gen file in that case of course, but isn't still a better
> default ?
>

In the original github issue, the reporter had a flow where the tarball
would be initialised as a new git repo, and so the sha values extracted
were valid values, but had no relation to the upstream sha values. So for
those cases, we have to check the file unconditionally.


>
> >  # Bail out if the directory isn't under git control
> >  src_dir=$(git rev-parse --git-dir 2>&1) || exit 1
> >  src_dir=$(readlink -f "$src_dir/..")
> > diff --git a/utils/run-dist.sh b/utils/run-dist.sh
> > new file mode 100644
> > index 000000000000..3b6c0adb05ed
> > --- /dev/null
> > +++ b/utils/run-dist.sh
> > @@ -0,0 +1,11 @@
> > +#!/bin/sh
> > +
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +#
> > +# On a meson dist run, generate the version string and store it in a
> file.
> > +# This will later be picked up by the utils/gen-version.sh script and
> used
> > +# instead of re-generating it. This way, if we are not building in the
> upstream
> > +# git source tree, the upstream version information will be preserved.
> > +
> > +cd "$MESON_SOURCE_ROOT" || return
> > +./utils/gen-version.sh > "$MESON_DIST_ROOT"/version.gen
>
> Do we need to handle the case of meson dist being run from a tarball
> instead of git ? It seems like a corner case to me, so probably not very
> useful, but we could possibly support it by copying
> $MESON_SOURCE_ROOT/version.gen to $MESON_DIST_ROOT/ if the source file
> exists. I'm fine either way.
>

Good point.  Although, I cannot see why meson dist should be run from a
tarball and not the upstream git tree :-)

Thanks,
Naush


> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211014/36712c67/attachment-0001.htm>