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

Thu Oct 14 11:58:49 CEST 2021

Hi Laurent,

On Thu, 14 Oct 2021 at 07:04, Naushir Patuck <naush at raspberrypi.com> wrote:

> 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 :-)
>

So it turns out the above change is not needed after all:

$ meson dist --no-tests
Dist currently only works with Git or Mercurial repos

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