<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 14 Oct 2021 at 21:37, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
<br>
Thank you for the patch.<br>
<br>
On Thu, Oct 14, 2021 at 12:59:50PM +0100, Naushir Patuck wrote:<br>
> The gen-version.sh script expects to be called from a git repo, and sets its<br>
> src_root variable accordingly. This may not always be the case if it is built<br>
> from a tarball source - full support for which is in a future commit.<br>
> <br>
> The MESON_SOURCE_ROOT environnement variable does not get set when called from<br>
> the meson vcs_tag() function, but does when called from the run_command()<br>
> function, so that cannot be used either.<br>
> <br>
> Instead, explicitly pass the meson source root to the gen-version.sh script.<br>
> <br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
>  meson.build               | 3 ++-<br>
>  src/libcamera/meson.build | 2 +-<br>
>  utils/gen-version.sh      | 4 ++--<br>
>  3 files changed, 5 insertions(+), 4 deletions(-)<br>
> <br>
> diff --git a/meson.build b/meson.build<br>
> index a49c484fe64e..556a3f7c42f8 100644<br>
> --- a/meson.build<br>
> +++ b/meson.build<br>
> @@ -17,7 +17,8 @@ project('libcamera', 'c', 'cpp',<br>
>  # git version tag, the build metadata (e.g. +211-c94a24f4) is omitted from<br>
>  # libcamera_git_version.<br>
>  libcamera_git_version = run_command('utils/gen-version.sh',<br>
> -                                    meson.build_root()).stdout().strip()<br>
> +                                    meson.build_root(),<br>
> +                                    meson.source_root()).stdout().strip()<br>
>  if libcamera_git_version == ''<br>
>      libcamera_git_version = meson.project_version()<br>
>  endif<br>
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build<br>
> index 243dd3c180eb..d8dd8344002c 100644<br>
> --- a/src/libcamera/meson.build<br>
> +++ b/src/libcamera/meson.build<br>
> @@ -95,7 +95,7 @@ libcamera_sources += control_sources<br>
>  <br>
>  gen_version = meson.source_root() / 'utils' / 'gen-version.sh'<br>
>  <br>
<br>
While at it, could you please add<br>
<br>
# Use vcs_tag() and not configure_file() or run_command(), to ensure that the<br>
# version gets updated with every ninja build and not just at meson setup time.<br>
<br>
so we'll remember next time ?<br></blockquote><div><br>Will do!</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> -version_cpp = vcs_tag(command : [gen_version, meson.build_root()],<br>
> +version_cpp = vcs_tag(command : [gen_version, meson.build_root(), meson.source_root()],<br>
>                        input : '<a href="http://version.cpp.in" rel="noreferrer" target="_blank">version.cpp.in</a>',<br>
>                        output : 'version.cpp',<br>
>                        fallback : meson.project_version())<br>
> diff --git a/utils/gen-version.sh b/utils/gen-version.sh<br>
> index b09ad495f86a..da191691a7c5 100755<br>
> --- a/utils/gen-version.sh<br>
> +++ b/utils/gen-version.sh<br>
> @@ -4,10 +4,10 @@<br>
>  # Generate a version string using git describe<br>
>  <br>
>  build_dir="$1"<br>
> +src_dir="$2"<br>
>  <br>
>  # Bail out if the directory isn't under git control<br>
> -src_dir=$(git rev-parse --git-dir 2>&1) || exit 1<br>
> -src_dir=$(readlink -f "$src_dir/..")<br>
> +git rev-parse --git-dir > /dev/null 2>&1 || exit 1<br>
<br>
It would be nice if we could make the source directory optional, the<br>
same way the build directory is, so that running gen-version.sh from the<br>
command line for testing would be easier. How about the following ?<br>
<br>
# Bail out if the directory isn't under git control<br>
git_dir=$(git rev-parse --git-dir 2>&1) || exit 1<br>
<br>
# Derive the source directory from the git directory if not specified.<br>
if [ -z "$src_dir" ]<br>
then<br>
        src_dir=$(readlink -f "$git_dir/..")<br>
fi<br>
<br>
<br>
With this (and assuming it works :-)),<br></blockquote><div><br></div><div>Kieran had a suggestion in his feedback on a previous version of the patch.</div><div>$build_dir/source symlinks to our source tree.  This is created from the top</div><div>level meson.build file:</div><div><br></div># Create a symlink from the build root to the source root. This is used when<br># running libcamera from the build directory to locate resources in the source<br># directory (such as IPA configuration files).<br>run_command('ln', '-fsT', meson.source_root(), meson.build_root() / 'source')<div><br></div><div>Assuming this does not disappear in the future, I can use that to reference the</div><div>source dir in the gen-version script. This will effectively make this patch (1/2)</div><div>redundant, and I only need to change gen-version.sh appropriately.  What</div><div>do you think?</div><div><br></div><div>Naush</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Reviewed-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
>  # Get a short description from the tree.<br>
>  version=$(git describe --abbrev=8 --match "v[0-9]*" 2>/dev/null)<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>