| Message ID | 20211014115951.2996808-1-naush@raspberrypi.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Naush, Thank you for the patch. On Thu, Oct 14, 2021 at 12:59:50PM +0100, Naushir Patuck wrote: > The gen-version.sh script expects to be called from a git repo, and sets its > src_root variable accordingly. This may not always be the case if it is built > from a tarball source - full support for which is in a future commit. > > The MESON_SOURCE_ROOT environnement variable does not get set when called from > the meson vcs_tag() function, but does when called from the run_command() > function, so that cannot be used either. > > Instead, explicitly pass the meson source root to the gen-version.sh script. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > meson.build | 3 ++- > src/libcamera/meson.build | 2 +- > utils/gen-version.sh | 4 ++-- > 3 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/meson.build b/meson.build > index a49c484fe64e..556a3f7c42f8 100644 > --- a/meson.build > +++ b/meson.build > @@ -17,7 +17,8 @@ project('libcamera', 'c', 'cpp', > # git version tag, the build metadata (e.g. +211-c94a24f4) is omitted from > # libcamera_git_version. > libcamera_git_version = run_command('utils/gen-version.sh', > - meson.build_root()).stdout().strip() > + meson.build_root(), > + meson.source_root()).stdout().strip() > if libcamera_git_version == '' > libcamera_git_version = meson.project_version() > endif > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index 243dd3c180eb..d8dd8344002c 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -95,7 +95,7 @@ libcamera_sources += control_sources > > gen_version = meson.source_root() / 'utils' / 'gen-version.sh' > While at it, could you please add # Use vcs_tag() and not configure_file() or run_command(), to ensure that the # version gets updated with every ninja build and not just at meson setup time. so we'll remember next time ? > -version_cpp = vcs_tag(command : [gen_version, meson.build_root()], > +version_cpp = vcs_tag(command : [gen_version, meson.build_root(), meson.source_root()], > input : 'version.cpp.in', > output : 'version.cpp', > fallback : meson.project_version()) > diff --git a/utils/gen-version.sh b/utils/gen-version.sh > index b09ad495f86a..da191691a7c5 100755 > --- a/utils/gen-version.sh > +++ b/utils/gen-version.sh > @@ -4,10 +4,10 @@ > # Generate a version string using git describe > > build_dir="$1" > +src_dir="$2" > > # 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/..") > +git rev-parse --git-dir > /dev/null 2>&1 || exit 1 It would be nice if we could make the source directory optional, the same way the build directory is, so that running gen-version.sh from the command line for testing would be easier. How about the following ? # Bail out if the directory isn't under git control git_dir=$(git rev-parse --git-dir 2>&1) || exit 1 # Derive the source directory from the git directory if not specified. if [ -z "$src_dir" ] then src_dir=$(readlink -f "$git_dir/..") fi With this (and assuming it works :-)), Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > # Get a short description from the tree. > version=$(git describe --abbrev=8 --match "v[0-9]*" 2>/dev/null)
Hi Naush, Thank you for the patch. On Thu, Oct 14, 2021 at 12:59:51PM +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 > .tarball-version 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 .tarball-version file and read the version string from it instead of > creating one. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > meson.build | 3 +++ > utils/gen-version.sh | 9 +++++++++ > utils/run-dist.sh | 11 +++++++++++ > 3 files changed, 23 insertions(+) > create mode 100644 utils/run-dist.sh > > diff --git a/meson.build b/meson.build > index 556a3f7c42f8..bd19bd836579 100644 > --- a/meson.build > +++ b/meson.build > @@ -25,6 +25,9 @@ endif > > libcamera_version = libcamera_git_version.split('+')[0] > > +# This script gererates the .tarball-version 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/utils/gen-version.sh b/utils/gen-version.sh > index da191691a7c5..8759e722ffe1 100755 > --- a/utils/gen-version.sh > +++ b/utils/gen-version.sh > @@ -6,6 +6,15 @@ > build_dir="$1" > src_dir="$2" > > +# If .tarball-version 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 "$src_dir"/.tarball-version ] If you agree with the comments on the previous patch, this should likely become if [ -n "$src_dir" ] && [ -f "$src_dir"/.tarball-version ] Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > +then > + cat "$src_dir"/.tarball-version > + exit 0 > +fi > + > # Bail out if the directory isn't under git control > git rev-parse --git-dir > /dev/null 2>&1 || exit 1 > > diff --git a/utils/run-dist.sh b/utils/run-dist.sh > new file mode 100644 > index 000000000000..e89c3733b56c > --- /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"/.tarball-version
Hi Laurent, On Thu, 14 Oct 2021 at 21:37, Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hi Naush, > > Thank you for the patch. > > On Thu, Oct 14, 2021 at 12:59:50PM +0100, Naushir Patuck wrote: > > The gen-version.sh script expects to be called from a git repo, and sets > its > > src_root variable accordingly. This may not always be the case if it is > built > > from a tarball source - full support for which is in a future commit. > > > > The MESON_SOURCE_ROOT environnement variable does not get set when > called from > > the meson vcs_tag() function, but does when called from the run_command() > > function, so that cannot be used either. > > > > Instead, explicitly pass the meson source root to the gen-version.sh > script. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > --- > > meson.build | 3 ++- > > src/libcamera/meson.build | 2 +- > > utils/gen-version.sh | 4 ++-- > > 3 files changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/meson.build b/meson.build > > index a49c484fe64e..556a3f7c42f8 100644 > > --- a/meson.build > > +++ b/meson.build > > @@ -17,7 +17,8 @@ project('libcamera', 'c', 'cpp', > > # git version tag, the build metadata (e.g. +211-c94a24f4) is omitted > from > > # libcamera_git_version. > > libcamera_git_version = run_command('utils/gen-version.sh', > > - meson.build_root()).stdout().strip() > > + meson.build_root(), > > + > meson.source_root()).stdout().strip() > > if libcamera_git_version == '' > > libcamera_git_version = meson.project_version() > > endif > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > index 243dd3c180eb..d8dd8344002c 100644 > > --- a/src/libcamera/meson.build > > +++ b/src/libcamera/meson.build > > @@ -95,7 +95,7 @@ libcamera_sources += control_sources > > > > gen_version = meson.source_root() / 'utils' / 'gen-version.sh' > > > > While at it, could you please add > > # Use vcs_tag() and not configure_file() or run_command(), to ensure that > the > # version gets updated with every ninja build and not just at meson setup > time. > > so we'll remember next time ? > Will do! > > > -version_cpp = vcs_tag(command : [gen_version, meson.build_root()], > > +version_cpp = vcs_tag(command : [gen_version, meson.build_root(), > meson.source_root()], > > input : 'version.cpp.in', > > output : 'version.cpp', > > fallback : meson.project_version()) > > diff --git a/utils/gen-version.sh b/utils/gen-version.sh > > index b09ad495f86a..da191691a7c5 100755 > > --- a/utils/gen-version.sh > > +++ b/utils/gen-version.sh > > @@ -4,10 +4,10 @@ > > # Generate a version string using git describe > > > > build_dir="$1" > > +src_dir="$2" > > > > # 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/..") > > +git rev-parse --git-dir > /dev/null 2>&1 || exit 1 > > It would be nice if we could make the source directory optional, the > same way the build directory is, so that running gen-version.sh from the > command line for testing would be easier. How about the following ? > > # Bail out if the directory isn't under git control > git_dir=$(git rev-parse --git-dir 2>&1) || exit 1 > > # Derive the source directory from the git directory if not specified. > if [ -z "$src_dir" ] > then > src_dir=$(readlink -f "$git_dir/..") > fi > > > With this (and assuming it works :-)), > Kieran had a suggestion in his feedback on a previous version of the patch. $build_dir/source symlinks to our source tree. This is created from the top level meson.build file: # Create a symlink from the build root to the source root. This is used when # running libcamera from the build directory to locate resources in the source # directory (such as IPA configuration files). run_command('ln', '-fsT', meson.source_root(), meson.build_root() / 'source') Assuming this does not disappear in the future, I can use that to reference the source dir in the gen-version script. This will effectively make this patch (1/2) redundant, and I only need to change gen-version.sh appropriately. What do you think? Naush > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > # Get a short description from the tree. > > version=$(git describe --abbrev=8 --match "v[0-9]*" 2>/dev/null) > > -- > Regards, > > Laurent Pinchart >
Hi Naush, On Fri, Oct 15, 2021 at 08:39:58AM +0100, Naushir Patuck wrote: > On Thu, 14 Oct 2021 at 21:37, Laurent Pinchart wrote: > > On Thu, Oct 14, 2021 at 12:59:50PM +0100, Naushir Patuck wrote: > > > The gen-version.sh script expects to be called from a git repo, and sets its > > > src_root variable accordingly. This may not always be the case if it is built > > > from a tarball source - full support for which is in a future commit. > > > > > > The MESON_SOURCE_ROOT environnement variable does not get set when called from > > > the meson vcs_tag() function, but does when called from the run_command() > > > function, so that cannot be used either. > > > > > > Instead, explicitly pass the meson source root to the gen-version.sh script. > > > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > > --- > > > meson.build | 3 ++- > > > src/libcamera/meson.build | 2 +- > > > utils/gen-version.sh | 4 ++-- > > > 3 files changed, 5 insertions(+), 4 deletions(-) > > > > > > diff --git a/meson.build b/meson.build > > > index a49c484fe64e..556a3f7c42f8 100644 > > > --- a/meson.build > > > +++ b/meson.build > > > @@ -17,7 +17,8 @@ project('libcamera', 'c', 'cpp', > > > # git version tag, the build metadata (e.g. +211-c94a24f4) is omitted from > > > # libcamera_git_version. > > > libcamera_git_version = run_command('utils/gen-version.sh', > > > - meson.build_root()).stdout().strip() > > > + meson.build_root(), > > > + meson.source_root()).stdout().strip() > > > if libcamera_git_version == '' > > > libcamera_git_version = meson.project_version() > > > endif > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > > index 243dd3c180eb..d8dd8344002c 100644 > > > --- a/src/libcamera/meson.build > > > +++ b/src/libcamera/meson.build > > > @@ -95,7 +95,7 @@ libcamera_sources += control_sources > > > > > > gen_version = meson.source_root() / 'utils' / 'gen-version.sh' > > > > While at it, could you please add > > > > # Use vcs_tag() and not configure_file() or run_command(), to ensure that the > > # version gets updated with every ninja build and not just at meson setup time. > > > > so we'll remember next time ? > > Will do! > > > > -version_cpp = vcs_tag(command : [gen_version, meson.build_root()], > > > +version_cpp = vcs_tag(command : [gen_version, meson.build_root(), meson.source_root()], > > > input : 'version.cpp.in', > > > output : 'version.cpp', > > > fallback : meson.project_version()) > > > diff --git a/utils/gen-version.sh b/utils/gen-version.sh > > > index b09ad495f86a..da191691a7c5 100755 > > > --- a/utils/gen-version.sh > > > +++ b/utils/gen-version.sh > > > @@ -4,10 +4,10 @@ > > > # Generate a version string using git describe > > > > > > build_dir="$1" > > > +src_dir="$2" > > > > > > # 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/..") > > > +git rev-parse --git-dir > /dev/null 2>&1 || exit 1 > > > > It would be nice if we could make the source directory optional, the > > same way the build directory is, so that running gen-version.sh from the > > command line for testing would be easier. How about the following ? > > > > # Bail out if the directory isn't under git control > > git_dir=$(git rev-parse --git-dir 2>&1) || exit 1 > > > > # Derive the source directory from the git directory if not specified. > > if [ -z "$src_dir" ] > > then > > src_dir=$(readlink -f "$git_dir/..") > > fi > > > > > > With this (and assuming it works :-)), > > Kieran had a suggestion in his feedback on a previous version of the patch. > $build_dir/source symlinks to our source tree. This is created from the top > level meson.build file: > > # Create a symlink from the build root to the source root. This is used when > # running libcamera from the build directory to locate resources in the source > # directory (such as IPA configuration files). > run_command('ln', '-fsT', meson.source_root(), meson.build_root() / 'source') > > Assuming this does not disappear in the future, I can use that to reference the > source dir in the gen-version script. This will effectively make this patch (1/2) > redundant, and I only need to change gen-version.sh appropriately. What > do you think? That would only work when running the script from the build directory. For testing purpose, it's nice to be able to run it manually in the source directory, and get the same output as when run by meson. I'd thus prefer deriving src_dir from git_dir when not specified. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > # Get a short description from the tree. > > > version=$(git describe --abbrev=8 --match "v[0-9]*" 2>/dev/null)
Hi, This is version 4 of this series. In this revision, I have reverted back to using vcs_tag() to generate the version string. My reason for the original change from vcs_tag() to configure_file() was so that I could use the MESON_SOURCE_ROOT env variable in the gen-version.sh. This does not seem to get set when called from vcs_tag(), but does with configure_file()! I have added a commit in (1/2) to this series that passes the source root explicitly as a parameter to gen-version.sh to work around this problem. Thanks, Naush Naushir Patuck (2): utils: gen-version: Pass the meson source root to the gen-version.sh script build: Preserve upstream git versioning using meson dist meson.build | 6 +++++- src/libcamera/meson.build | 2 +- utils/gen-version.sh | 13 +++++++++++-- utils/run-dist.sh | 11 +++++++++++ 4 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 utils/run-dist.sh