<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br></div><div>Thank you for your feedback.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 13 Oct 2021 at 11:38, 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 Wed, Oct 13, 2021 at 11:20:53AM +0100, Naushir Patuck wrote:<br>
> On Wed, 13 Oct 2021 at 11:19, Naushir Patuck wrote:<br>
> > When distributions build and package libcamera libraries, they may not<br>
> > necessarily run the build in the upstream source tree. In these cases, the git<br>
> > SHA1 versioning information will be lost.<br>
> ><br>
> > This change addresses that problem by requiring package managers to run<br>
> > 'meson dist' to create a tarball of the source files and build from there.<br>
> > On runing 'meson dist', the utils/run-dist.sh script will create a version.gen<br>
> > file in the release tarball with the version string generated from the existing<br>
> > utils/gen-version.sh script.<br>
> ><br>
> > The build system has been updated to check for the presence of this version.gen<br>
> > file and read the version string from it. If the file is not present, the<br>
> > version string is generated by running utils/gen-version.sh as it currently<br>
> > does.<br>
> ><br>
> > Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> > ---<br>
> > meson.build | 30 ++++++++++++++++++++----------<br>
> > src/libcamera/meson.build | 11 +++++------<br>
> > utils/run-dist.sh | 9 +++++++++<br>
> > 3 files changed, 34 insertions(+), 16 deletions(-)<br>
> > create mode 100644 utils/run-dist.sh<br>
> ><br>
> > diff --git a/meson.build b/meson.build<br>
> > index a49c484fe64e..f27bfd479a5c 100644<br>
> > --- a/meson.build<br>
> > +++ b/meson.build<br>
> > @@ -10,20 +10,30 @@ project('libcamera', 'c', 'cpp',<br>
> > ],<br>
> > license : 'LGPL 2.1+')<br>
> ><br>
> > -# Generate version information. The libcamera_git_version variable contains the<br>
> > -# full version with git patch count and SHA1 (e.g. 1.2.3+211-c94a24f4), while<br>
> > -# the libcamera_version variable contains the major.minor.patch (e.g. 1.2.3)<br>
> > -# only. If the source tree isn't under git control, or if it matches the last<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>
> > -if libcamera_git_version == ''<br>
> > - libcamera_git_version = meson.project_version()<br>
> > +fs_mod = import('fs')<br>
> > +if not fs_mod.is_file('version.gen')<br>
> > + # Generate version information. The libcamera_git_version variable contains the<br>
> > + # the full version with git patch count and SHA1 (e.g. 1.2.3+211-c94a24f4), while<br>
> > + # the libcamera_version variable contains the major.minor.patch (e.g. 1.2.3)<br>
> > + # only. If the source tree isn't under git control, or if it matches the last<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>
> > + if libcamera_git_version == ''<br>
> > + libcamera_git_version = meson.project_version()<br>
> > + endif<br>
> > +else<br>
> > + # Read the version information from the file generated by the utils/run-dist.sh<br>
> > + # script on a 'meson dist' command.<br>
> > + libcamera_git_version = run_command(['cat', 'version.gen']).stdout().strip()<br>
> > endif<br>
> <br>
> I've added an if clause here to check for the presence of the version file, but I<br>
> could equally leave this as-is and check for the file in gen-version.sh. I don't have<br>
> a strong preference either way.<br>
<br>
I think I'd prefer handling this in gen-version.sh, as it will<br>
centralize all the version handling code in a single place.<br></blockquote><div><br></div><div>Ok, 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>
> > libcamera_version = libcamera_git_version.split('+')[0]<br>
> ><br>
> > +# This script gererates the version.gen file on a 'meson dist' command.<br>
> > +meson.add_dist_script('utils/run-dist.sh')<br>
> > +<br>
> > # Configure the build environment.<br>
> > cc = meson.get_compiler('c')<br>
> > cxx = meson.get_compiler('cpp')<br>
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build<br>
> > index 243dd3c180eb..360eaf80ecf1 100644<br>
> > --- a/src/libcamera/meson.build<br>
> > +++ b/src/libcamera/meson.build<br>
> > @@ -93,12 +93,11 @@ endforeach<br>
> ><br>
> > libcamera_sources += control_sources<br>
> ><br>
> > -gen_version = meson.source_root() / 'utils' / 'gen-version.sh'<br>
> > -<br>
> > -version_cpp = vcs_tag(command : [gen_version, meson.build_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>
> > +version_data = configuration_data()<br>
> > +version_data.set('VCS_TAG', libcamera_git_version)<br>
> > +version_cpp = configure_file(input : '<a href="http://version.cpp.in" rel="noreferrer" target="_blank">version.cpp.in</a>',<br>
> > + output : 'version.cpp',<br>
> > + configuration : version_data)<br>
> ><br>
> > libcamera_sources += version_cpp<br>
> ><br>
> > diff --git a/utils/run-dist.sh b/utils/run-dist.sh<br>
> > new file mode 100644<br>
> > index 000000000000..1b2a6348c6bc<br>
> > --- /dev/null<br>
> > +++ b/utils/run-dist.sh<br>
> > @@ -0,0 +1,9 @@<br>
> > +#!/bin/sh<br>
> > +<br>
> > +# SPDX-License-Identifier: GPL-2.0-or-later<br>
> > +#<br>
> > +# On a meson dist run, generate the version string and store it in a file.<br>
> > +# This will late be picked up by the build system and used in place of running<br>
<br>
s/late/later/<br>
<br>
> > +# the utils/gen-version.sh script again since we may not be running in the<br>
> > +# upstream git source tree, and so version information would be lost.<br>
<br>
A blank line here would be nice.<br>
<br>
> > +"$MESON_DIST_ROOT"/utils/gen-version.sh > "$MESON_DIST_ROOT"/version.gen<br>
<br>
Do we need to cd to MESON_SOURCE_ROOT first (or set GIT_DIR), or is<br>
there a guarantee that this script will always run from a subdirectory<br>
of MESON_SOURCE_ROOT ?<br></blockquote><div><br></div><div>From what I can tell, meson dist does not seem to allow the user to specify the dist output directory,</div><div>it creates a 'meson-dist' direcotry under MESON_BUILD_ROOT. I had assumed the latter will</div><div>will always be a subdirectly of MESON_SOURCE_ROOT, but perhaps that will not always be the</div><div>case, so I could cd MESON_SOURCE_ROOT to be sure!</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>
To end with some nitpicking, I wonder if there's a standard name for<br>
files containing a version number (I initially thought about .version<br>
but have no evidence that it would be better than version.gen).<br></blockquote><div><br></div><div>I don't know of any specific standard filename for this. I chose the .gen extension to loosely signify</div><div>that this was an auto generated file. I can change to .version if you prefer?</div><div><br></div><div>Regards,</div><div>Naush</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>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>