[libcamera-devel] [RFC PATCH] libcamera: Disable automatic vcs versioning

[Laurent Pinchart]

On Mon, Aug 22, 2022 at 09:44:20PM +0300, Tomi Valkeinen wrote:
> On 22/08/2022 17:39, Laurent Pinchart via libcamera-devel wrote:
> > On Mon, Aug 22, 2022 at 05:17:05PM +0300, Laurent Pinchart via libcamera-devel wrote:
> >> On Sat, Aug 20, 2022 at 03:29:48PM +0200, Christian Rauch via libcamera-devel wrote:
> >>> Hi Tomi,
> >>>
> >>> With the recent discussions on "versioning", I would actually argue that
> >>> this should be removed entirely, and we should only rely on the semver
> >>> versioning.
> >>
> >> I'd rather not. It has been proven very useful multiple times to have
> >> the exact git commit ID in bug reports. If we were to drop it, it would
> >> increase our support burden. I'm fine omitting the commit ID when the
> >> libcamera version exactly matches a released version, but not otherwise.
> >>
> >>> Am 20.08.22 um 10:07 schrieb Tomi Valkeinen via libcamera-devel:
> >>>> libcamera builds the git hash version into the libcamera library. This
> >>>> makes ninja create the generated version.cpp on every build, and if the
> >>>> git hash has changed, compilation of version.cpp and linking of
> >>>> libcamera library.
> >>>>
> >>>> This patch adds a meson option to disable this behavior and instead use
> >>>> the meson project-version as the version built into the library, which
> >>>> is only generated at meson configuration time.
> >>>>
> >>>> With this change, ninja will nicely say "no work to do" if there's
> >>>> nothing to compile, instead of starting the build and generating
> >>>> version.cpp.
> >>
> >> I'd prefer a way to skip recompilation (and relinking) if the version
> >> hasn't changed since the last build.
> > 
> > Which of course doesn't work for Tomi's use case.
> > 
> > /me needs to wake up
> > 
> > How can we get the best of both worlds ?
> 
> I think there are two different build types: for publishing and for your 
> own use (development usually). With dev builds I'm fine with omitting 
> versioning info, skipping any signing processes, and so forth. I think 
> that can be a meson option. Which one should be the default, that's a 
> more difficult question (maybe publishing).

I'd like to make this difficult to enable by mistake, extending the
definition of "by mistake" to enabling the option on purpose without
understanding it will hurt bug reports. We should at least print a very
big warning in this case, and I wonder if it would be worth it trying to
hide the option (for instance using an environment variable that
gen-version.sh would interpret instead of a meson option). I'll be quite
cross if we start seeing libcamera logs without a proper version number.

> >>>> But, more importantly, it often reduces build time. For example, this
> >>>> takes a particular git range compilation (~20 py bindings patches) from
> >>>> 43 seconds to 10 seconds. Obviously the improvement depends very much on
> >>>> the patches in question. If every commit changes libcamera itself, there
> >>>> is no difference.
> >>>>
> >>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
> >>>> ---
> >>>>   meson_options.txt         |  5 +++++
> >>>>   src/libcamera/meson.build | 29 +++++++++++++++++++++--------
> >>>>   2 files changed, 26 insertions(+), 8 deletions(-)
> >>>>
> >>>> diff --git a/meson_options.txt b/meson_options.txt
> >>>> index 7a9aecfc..6a0bd0be 100644
> >>>> --- a/meson_options.txt
> >>>> +++ b/meson_options.txt
> >>>> @@ -63,3 +63,8 @@ option('pycamera',
> >>>>           type : 'feature',
> >>>>           value : 'disabled',
> >>>>           description : 'Enable libcamera Python bindings (experimental)')
> >>>> +
> >>>> +option('disable-vcs-versioning',
> >>>> +        type : 'boolean',
> >>>> +        value : false,
> >>>> +        description : 'Disable automatic libcamera library versioning with the git hash')
> >>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> >>>> index ce1f0f2f..6fd6f084 100644
> >>>> --- a/src/libcamera/meson.build
> >>>> +++ b/src/libcamera/meson.build
> >>>> @@ -121,14 +121,27 @@ endforeach
> >>>>
> >>>>   libcamera_sources += control_sources
> >>>>
> >>>> -gen_version = meson.project_source_root() / 'utils' / 'gen-version.sh'
> >>>> -
> >>>> -# 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.
> >>>> -version_cpp = vcs_tag(command : [gen_version, meson.project_build_root(), meson.project_source_root()],
> >>>> -                      input : 'version.cpp.in',
> >>>> -                      output : 'version.cpp',
> >>>> -                      fallback : meson.project_version())
> >>>> +if get_option('disable-vcs-versioning')
> >>>> +    cdata = configuration_data()
> >>>> +    cdata.set('VCS_TAG', meson.project_version())
> >>>> +
> >>>> +    # For some reason using 'version.cpp' as output file works fine for generation
> >>>> +    # but causes meson to delete the file before build. Any other file name
> >>>> +    # seems to work.
> >>>> +    version_cpp = configure_file(input : 'version.cpp.in',
> >>>> +                                 output : 'version_static.cpp',
> >>>> +                                 configuration : cdata)
> > 
> > Do I understand correctly that this will cause a build failure if you
> > don't first run meson setup with the disable-vcs-versioning option
> > disabled ?
> 
> Isn't this piece of code always run when you change (enable) the option, 
> even if you do it with "meson configure" after the initial setup?

What happens if you run

mkdir build
cd build
meson setup -Ddisable-vcs-versioning=true ..
ninja

-- 
Regards,

Laurent Pinchart