[libcamera-devel] [PATCH v2 2/5] libcamera: Auto generate version information

[Kieran Bingham]

Hi Laurent,

On 04/07/2019 14:40, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thu, Jul 04, 2019 at 02:03:44PM +0100, Kieran Bingham wrote:
>> Generate a version string, and provide a global singleton object which
>> allows applications to interrogate the current libcamera version
>> information.
>>
>> The version header is automatically updated by meson on each build.
>> The string roughly follows the semver [0] conventions of
>> major.minor.patch-label as a value.
>>
>> [0] https://semver.org/
>>
>> A script (utils/gen-version.sh) is provided which is modelled upon the
>> processing from autoconf's git-versin-gen. The gen-version.sh script
> 
> s/versin/version/

Fixed,

> 
>> will look for tags in the form vX.Y as starting points for the version
>> string. While the repository does not have any matching tags, v0.0 will
>> be assumed, resulting in versions with both major and minor being set to
>> '0', and the patch count resulting from the number of patches in the
>> history to that point.
>>
>> Finally, a uniquely identifying shortened hash is provided from git:
>>
>> 	v0.0.509+c544
> 
> The script now uses 8 characters by default for the abbreviated SHA1.

I'll update.

>>
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>> ---
>> v2:
>>   - Store VCS_Tag return reference for dependancy linking
>>   - Fix indentation on meson.build
>>   - fix shell usage and shellcheck warnings in version-gen
>>   - Make LibcameraVersion version global
>>   - Sphinx version now explicit as to it's source from
>>     Documentation/meson.build
>>   - Pass project_version() through api_version
>>   - Use Camera object to report version rather than global constructor.
>>
>> v3:
>>  - Add dependency to the vcs tag to ensure libcamera builds the header
>>  - Fix up commit title
>>  - Rename version-gen to gen-version.sh
>>  - Append git sha and any -dirty flag with a '+' separator
>>  - Move version generation to single libcamera/version.h file.
>>  - Remove version.cpp and store the const std::string version in
>>    camera_manager.cpp
>> ---
>>  Documentation/conf.py            |  7 ++----
>>  Documentation/meson.build        |  6 ++++--
>>  include/libcamera/meson.build    |  5 +++++
>>  include/libcamera/version.h.in   | 22 +++++++++++++++++++
>>  meson.build                      |  9 +++-----
>>  src/libcamera/camera_manager.cpp |  8 +++++++
>>  src/libcamera/meson.build        |  1 +
>>  utils/gen-version.sh             | 37 ++++++++++++++++++++++++++++++++
>>  8 files changed, 82 insertions(+), 13 deletions(-)
>>  create mode 100644 include/libcamera/version.h.in
>>  create mode 100755 utils/gen-version.sh
>>
>> diff --git a/Documentation/conf.py b/Documentation/conf.py
>> index 970edf3d7298..3ac61a208145 100644
>> --- a/Documentation/conf.py
>> +++ b/Documentation/conf.py
>> @@ -23,11 +23,8 @@ project = 'libcamera'
>>  copyright = '2018-2019, The libcamera documentation authors'
>>  author = u'Kieran Bingham, Jacopo Mondi, Laurent Pinchart, Niklas Söderlund'
>>  
>> -# The short X.Y version
>> -version = ''
>> -# The full version, including alpha/beta/rc tags
>> -release = '0.1'
>> -
>> +# Version information is provided by the build environment, through the
>> +# configuration_data (cdata) in Documentation/meson.build
> 
> "through the sphinx command line" ? configuration_data() is an internal
> meson concept.

Also - that's how the Doxygen version information is passed, not the
sphinx...

Changed,

> 
>>  
>>  # -- General configuration ---------------------------------------------------
>>  
>> diff --git a/Documentation/meson.build b/Documentation/meson.build
>> index 629e853120cb..ea7db4eaeab0 100644
>> --- a/Documentation/meson.build
>> +++ b/Documentation/meson.build
>> @@ -1,4 +1,4 @@
>> -doc_install_dir = join_paths(get_option('datadir'), 'doc', 'libcamera- at 0@'.format(api_version))
>> +doc_install_dir = join_paths(get_option('datadir'), 'doc', 'libcamera- at 0@'.format(meson.project_version()))
>>  
>>  #
>>  # Doxygen
>> @@ -47,8 +47,10 @@ if sphinx.found()
>>          'index.rst',
>>      ]
>>  
>> +    release = 'release=' + meson.project_version()
>> +
>>      custom_target('documentation',
>> -                  command : [sphinx, '-q', '-W', '-b', 'html', meson.current_source_dir(), '@OUTPUT@'],
>> +                  command : [sphinx, '-D', release, '-q', '-W', '-b', 'html', meson.current_source_dir(), '@OUTPUT@'],
> 
> Time to split this on multiple lines ? :-)

Wrapped,

The doc_install_dir at the top of this file is also long, which is the
precedent I followed for not wrapping this.

Additional patch wrapping that line added to this series.


> 
>>                    input : docs_sources,
>>                    output : 'html',
>>                    build_by_default : true,
>> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
>> index 3067120a1598..69823dd1da63 100644
>> --- a/include/libcamera/meson.build
>> +++ b/include/libcamera/meson.build
>> @@ -18,6 +18,11 @@ libcamera_api = files([
>>  
>>  gen_header = files('gen-header.sh')
>>  
>> +version_h = vcs_tag(command: ['../../utils/gen-version.sh', meson.current_source_dir()],
> 
> Maybe join_paths(meson.source_root(), 'utils', 'gen-version.sh') ?

Yes. I had been trying to use files() - but vcs_tag() does not support
it, because it generates a list...

The join_paths is better than hardcoding the ../../ though.


> 
>> +                    input: 'version.h.in',
>> +                    output: 'version.h',
>> +                    fallback: 'v0.0')
> 
> s/:/ :/ in all the lines above.

Updated.

> 
>> +
>>  libcamera_h = custom_target('gen-header',
>>                              input : 'meson.build',
>>                              output : 'libcamera.h',
>> diff --git a/include/libcamera/version.h.in b/include/libcamera/version.h.in
>> new file mode 100644
>> index 000000000000..e49b36962aed
>> --- /dev/null
>> +++ b/include/libcamera/version.h.in
>> @@ -0,0 +1,22 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2019, Google Inc.
>> + *
>> + * version.h - Library version information
>> + *
>> + * This file is auto-generated. Do not edit.
>> + */
>> +#ifndef __LIBCAMERA_VERSION_H__
>> +#define __LIBCAMERA_VERSION_H__
>> +
>> +#include <string>
>> +
>> +#define LIBCAMERA_VERSION "@VCS_TAG@"
>> +
>> +namespace libcamera {
>> +
>> +extern const std::string version;
>> +
>> +} /* namespace libcamera */
>> +
>> +#endif /* __LIBCAMERA_VERSION_H__ */
>> diff --git a/meson.build b/meson.build
>> index a3b0bc820072..342b3cc76a93 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -1,6 +1,8 @@
>>  project('libcamera', 'c', 'cpp',
>>      meson_version : '>= 0.40',
>> -    version : '0.1',
>> +    version : run_command('utils/gen-version.sh',
>> +                          '@0@'.format(meson.source_root()),
>> +                          check : true).stdout().strip(),
> 
> I don't think we should run the script here. This value becomes
> accessible in meson.project_version(), which is the default fallback for
> the vcs_tag() function, and should be used when no version control is

I override the vcs_tag fallback through the fallback parameter to v0.0.


> available (for instance in the case of release tarballs). Here what I
> propose doing instead:

In the case of release tarballs, the script should be updated to return
that.

And meson should create a release version file on dist/install. But
that's for another patch series, as it's tough enough to get this part
in first :)



> - In this file:
> 
> libcamera_version = run_command('utils/gen-version.sh',
>                                 '@0@'.format(meson.source_root()),
> 				check : true).stdout().strip()
> if libcamera_version == ''
>     libcamera_version = 'v0.0'
> endif
> 
> project(
>     ...
>     version : libcamera_version,
>     ...
> )

You can't do that - I've tried before. (And I've just retried)

ERROR: First statement must be a call to project



> - In include/libcamera/meson.build:
> 
> version_h = vcs_tag(command : ['echo', libcamera_version],
>                     input: 'version.h.in',
>                     output: 'version.h')


And that would not run the vcs command on every build... which is the
purpose of the vcs_tag.



> That we we only run the script once, and have a canonical location for
> the tarball release version in the root meson.build. What do you think ?


See above.


> 
>>      default_options : [
>>          'werror=true',
>>          'warning_level=2',
>> @@ -8,11 +10,6 @@ project('libcamera', 'c', 'cpp',
>>      ],
>>      license : 'LGPL 2.1+')
>>  
>> -# TODO: Extract this from project.version.
>> -#       Ideally the version at Documentation/conf.py should be
>> -#       generated from this too.
>> -api_version = '0.1'
>> -
>>  cc = meson.get_compiler('c')
>>  config_h = configuration_data()
>>  
>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
>> index cf881ce2e641..5ced4ad76ba0 100644
>> --- a/src/libcamera/camera_manager.cpp
>> +++ b/src/libcamera/camera_manager.cpp
>> @@ -9,6 +9,7 @@
>>  
>>  #include <libcamera/camera.h>
>>  #include <libcamera/event_dispatcher.h>
>> +#include <libcamera/version.h>
>>  
>>  #include "device_enumerator.h"
>>  #include "event_dispatcher_poll.h"
>> @@ -64,6 +65,11 @@ CameraManager::~CameraManager()
>>  {
>>  }
>>  
>> +/**
>> + * \brief Declare the library global version string.
> 
> s/\.//

Removed.

> 
>> + */
>> +const std::string version(LIBCAMERA_VERSION);
>> +
>>  /**
>>   * \brief Start the camera manager
>>   *
>> @@ -79,6 +85,8 @@ int CameraManager::start()
>>  	if (enumerator_)
>>  		return -EBUSY;
>>  
>> +	LOG(Camera, Info) << "libcamera " << version;
>> +
>>  	enumerator_ = DeviceEnumerator::create();
>>  	if (!enumerator_ || enumerator_->enumerate())
>>  		return -ENODEV;
>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
>> index 8075b1f696f5..336f4f066fac 100644
>> --- a/src/libcamera/meson.build
>> +++ b/src/libcamera/meson.build
>> @@ -80,6 +80,7 @@ control_types_cpp = custom_target('control_types_cpp',
>>  libcamera_sources += control_types_cpp
>>  
>>  libcamera_deps = [
>> +    declare_dependency(sources : version_h),
>>      cc.find_library('dl'),
>>      libudev,
>>  ]
>> diff --git a/utils/gen-version.sh b/utils/gen-version.sh
>> new file mode 100755
>> index 000000000000..b3003d7a80d3
>> --- /dev/null
>> +++ b/utils/gen-version.sh
>> @@ -0,0 +1,37 @@
>> +#!/bin/sh
>> +
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +# Generate a version string using git describe
>> +
>> +if [ -n "$1" ]
>> +then
>> +	cd "$1" 2>/dev/null || exit 1
>> +fi
>> +
> 
> If you can exit here when the tree isn't under git control then we'll be
> ready for tarball releases.


No, I think it will be up to this script to return the tarball release
version.

That's currently unimplemented and can be built on top.

It will also need hooks adding into meson as a dist rule or such.



>> +# Get a short description from the tree.
>> +version=$(git describe --abbrev=8 --match "v[0-9]*" 2>/dev/null)
>> +
>> +if [ -z "$version" ]
>> +then
>> +	# Handle an un-tagged repository
>> +	sha=$(git describe --abbrev=8 --always 2>/dev/null)
>> +	commits=$(git log --oneline | wc -l 2>/dev/null)
>> +	version=v0.0.$commits.$sha
>> +fi
>> +
>> +# Prevent changed timestamps causing -dirty labels
>> +git update-index --refresh > /dev/null 2>&1
>> +dirty=$(git diff-index --name-only HEAD 2>/dev/null) || dirty=
>> +
>> +# Strip the 'g', and replace the preceeding '-' with a '+' to denote a label
>> +version=$(echo "$version" | sed -e 's/-g/+/g')
>> +
>> +# Fix the '-' (the patch count) to a '.' as a version increment.
>> +version=$(echo "$version" | sed -e 's/-/./g')
>> +
>> +if [ -n "$dirty" ]
>> +then
>> +	version=$version-dirty
>> +fi
>> +
>> +echo "$version"
> 

-- 
Regards
--
Kieran