[libcamera-devel] [PATCH v2] libcamera: skip auto version generation when building for Chromium OS

[Kieran Bingham]

Hi Paul,

On 10/07/2019 15:23, Paul Elder wrote:
> Commit b817bcec6b53 ("libcamera: Auto generate version information")
> causes the build to fail in the Chromium OS build environment, because
> git update-index tries to take a lock (ie. write) in the git repo that
> is outside of the build directory.

ouch ...

> The solution is to simply skip git update-index if we are building in
> the Chromium OS build environment, and this decision is made if the
> build directory is not a subdirectory of the source directory.

Thanks for looking into a fix,

Running shellcheck highlights a few improvements on this patch

> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> Changes in v2:
> - add quotes around variable accessess, and the string matcher
> - make the two path arguments to gen-version.sh required
> - actually run gen-version.sh from the needed place in meson
> 
>  meson.build               |  3 ++-
>  src/libcamera/meson.build |  2 +-
>  utils/gen-version.sh      | 14 +++++++++++---
>  3 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 8f3d0ce..99a3a80 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -15,7 +15,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.source_root()).stdout().strip()
> +                                    meson.source_root(),
> +                                    meson.build_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 97ff86e..4c442b9 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -81,7 +81,7 @@ libcamera_sources += control_types_cpp
>  
>  gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')
>  
> -version_cpp = vcs_tag(command : [gen_version, meson.source_root()],
> +version_cpp = vcs_tag(command : [gen_version, meson.source_root(), meson.build_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 708c01d..5005db9 100755
> --- a/utils/gen-version.sh
> +++ b/utils/gen-version.sh
> @@ -3,11 +3,16 @@
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  # Generate a version string using git describe
>  
> -if [ -n "$1" ]
> +src_dir="$1"
> +build_dir="$2"
> +
> +if [ -z "$src_dir" -o -z "$build_dir" ]

Shellcheck reports the following:

In utils/gen-version.sh line 9:
if [ -z "$src_dir" -o -z "$build_dir" ]
                   ^-- SC2166: Prefer [ p ] || [ q ] as [ p -o q ] is
not well defined.

So perhaps this line could be:

 if [ -z "$src_dir" ] || [ -z "$build_dir" ]

>  then
> -	cd "$1" 2>/dev/null || exit 1
> +	exit
>  fi

It's a shame that we can't just run the command from the source tree any
more to get the output, but as that's just really for testing and
development, that's not a real issue.



>  
> +cd "$src_dir" 2>/dev/null || exit 1
> +
>  # Bail out if the directory isn't under git control
>  git rev-parse --git-dir >/dev/null 2>&1 || exit 1
>  
> @@ -24,7 +29,10 @@ fi
>  
>  # Append a '-dirty' suffix if the working tree is dirty. Prevent false
>  # positives due to changed timestamps by running git update-index.
> -git update-index --refresh > /dev/null 2>&1
> +if [ -n "$(echo "$build_dir" | grep "$src_dir")" ]

Shellcheck reports:

In utils/gen-version.sh line 32:
if [ -n "$(echo "$build_dir" | grep "$src_dir")" ]
     ^-- SC2143: Use grep -q instead of comparing output with [ -n .. ].

Perhaps this line could be:

 if (echo "$build_dir" | grep -vq "$src_dir")



With shellcheck running cleanly on utils/gen-version.sh:

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


> +then
> +	git update-index --refresh > /dev/null 2>&1
> +fi
>  git diff-index --quiet HEAD || version="$version-dirty"
>  
>  # Replace first '-' with a '+' to denote build metadata, strip the 'g' in from
> 

-- 
Regards
--
Kieran