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

1642 diff mbox series

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.

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.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 meson.build          |  3 ++-
 utils/gen-version.sh | 11 +++++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

Hi Paul,

Thank you for the patch.

On Wed, Jul 10, 2019 at 08:49:16PM +0900, 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.
> 
> 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.

It's a bit of a hack though, so I'd make this clear

"As the build runs in a sandbox that traps write accesses instead of
disallowing them through regular permissions, we can't easily determine
if write access to the source tree is permitted or will cause a fault.
To work around this, check if the build directory is not a subdirectory
of the source directory, in which case we don't assume write access and
skip git update-index. This shouldn't generate too many false positives
as the build directory is typically placed in the source directory
during development, and building packages usually doesn't use a dirty
git tree."

> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  meson.build          |  3 ++-
>  utils/gen-version.sh | 11 +++++++++--
>  2 files changed, 11 insertions(+), 3 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/utils/gen-version.sh b/utils/gen-version.sh
> index 708c01d..8700479 100755
> --- a/utils/gen-version.sh
> +++ b/utils/gen-version.sh
> @@ -3,7 +3,10 @@
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  # Generate a version string using git describe
>  
> -if [ -n "$1" ]
> +SRC_DIR="$1"
> +BUILD_DIR="$2"

We tend to use lower-case variables.

> +
> +if [ -n $SRC_DIR ]
>  then
>  	cd "$1" 2>/dev/null || exit 1
>  fi
> @@ -24,7 +27,11 @@ 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 $SRC_DIR \) -a \( -n $BUILD_DIR \) -a
> +     $(echo $BUILD_DIR | grep $SRC_DIR) ]

How about simplifying this by just returning at the top of the script if
either variable isn't set ? The script is only called by meson, so we
control what parameters get passed to it.

> +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

Hi Paul,

Thanks for your work.

On 2019-07-10 20:49:16 +0900, 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.
> 
> 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.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  meson.build          |  3 ++-
>  utils/gen-version.sh | 11 +++++++++--
>  2 files changed, 11 insertions(+), 3 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/utils/gen-version.sh b/utils/gen-version.sh
> index 708c01d..8700479 100755
> --- a/utils/gen-version.sh
> +++ b/utils/gen-version.sh
> @@ -3,7 +3,10 @@
>  # 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 [ -n $SRC_DIR ]
>  then
>  	cd "$1" 2>/dev/null || exit 1
>  fi
> @@ -24,7 +27,11 @@ 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 $SRC_DIR \) -a \( -n $BUILD_DIR \) -a
> +     $(echo $BUILD_DIR | grep $SRC_DIR) ]

When doing this you should either 'set -o pipefail' or examine both 
${PIPESTATUS[0]} and ${PIPESTATUS[1]}. In this case the subtle error 
will only hit if echo fails for some reason tho.

Another option is to look at the string and ignore the return code of 
the two commands,

-n "$(echo $BUILD_DIR | grep $SRC_DIR)"

> +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
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel