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

Kieran Bingham kieran.bingham at ideasonboard.com
Wed Jul 3 14:19:18 CEST 2019


Hi Laurent,

On 02/07/2019 15:22, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Tue, Jul 02, 2019 at 12:48:39PM +0100, Kieran Bingham wrote:
>> Generate a version string, and provide a global singleton object which
>> allows applications to interogate the current libcamera version

s/interogate/interrogate/


>> 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/
>>
>> The utils/version-gen script 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.
> 
> Could you mention where version-gen comes from ?

It's modelled upon generating the same output as git-version-gen from
autoconf.


> 
>> Finally, a uniquely identifying shortened checksum is provided from git:
> 
> It's not a checksum for a hash.

Good point, it is a hash.

> 
>>
>> 	v0.0.509.c544
> 
> Does the hash get appended only where the current head doesn't match a
> version tag ?
> 
> I think we should increase the number of characters in the hash, as 4
> will generate lots of collisions. How about going for 12 like the kernel
> ? Would it also make sense to separate it from the version number with a
> - instead of . like the kernel does ?


This is modelled to generate the same output as the standard gnu
implementation.

'4' is a minimum, and if collisions occur it will be increased
automatically by git.


> 
>> 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.
>> ---
>>  Documentation/conf.py            |  7 ++----
>>  Documentation/meson.build        |  2 +-
>>  include/libcamera/meson.build    |  1 +
>>  include/libcamera/version.h      | 24 +++++++++++++++++++++
>>  meson.build                      | 12 ++++++-----
>>  src/libcamera/camera_manager.cpp |  3 +++
>>  src/libcamera/meson.build        |  1 +
>>  src/libcamera/version.cpp        | 35 ++++++++++++++++++++++++++++++
>>  utils/version-gen                | 37 ++++++++++++++++++++++++++++++++
>>  version.h.in                     | 14 ++++++++++++
>>  10 files changed, 125 insertions(+), 11 deletions(-)
>>  create mode 100644 include/libcamera/version.h
>>  create mode 100644 src/libcamera/version.cpp
>>  create mode 100755 utils/version-gen
>>  create mode 100644 version.h.in
>>
>> 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
> 
> That's actually not true, with this patch the sphinx documentation isn't
> versioned anymore. To fix this you should pass -D release=... to the
> Sphinx command line.

Perhaps I'm missing something.

could you point out where the version string is lost in the sphinx output?

I think you're right that we should define release= - but I can't
actually see it get used anywhere in sphinx, only in Doxygen.

It's a moot point really though - I just can't tie it to anything.


>>  # -- General configuration ---------------------------------------------------
>>  
>> diff --git a/Documentation/meson.build b/Documentation/meson.build
>> index 629e853120cb..91539b2afd66 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
>> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
>> index 15484724df01..c925af7b3f96 100644
>> --- a/include/libcamera/meson.build
>> +++ b/include/libcamera/meson.build
>> @@ -12,6 +12,7 @@ libcamera_api = files([
>>      'signal.h',
>>      'stream.h',
>>      'timer.h',
>> +    'version.h',
>>  ])
>>  
>>  gen_header = files('gen-header.sh')
>> diff --git a/include/libcamera/version.h b/include/libcamera/version.h
>> new file mode 100644
>> index 000000000000..3075cfb7a6e3
>> --- /dev/null
>> +++ b/include/libcamera/version.h
>> @@ -0,0 +1,24 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2019, Google Inc.
>> + *
>> + * version.h - Library version information
>> + */
>> +#ifndef __LIBCAMERA_VERSION_H__
>> +#define __LIBCAMERA_VERSION_H__
>> +
>> +#include <string>
>> +
>> +namespace libcamera {
>> +
>> +class LibcameraVersion
>> +{
>> +public:
>> +	std::string toString() const;
>> +};
>> +
>> +extern const LibcameraVersion version;
>> +
>> +} /* namespace libcamera */
>> +
>> +#endif /* __LIBCAMERA_VERSION_H__ */
>> diff --git a/meson.build b/meson.build
>> index a3b0bc820072..c34763162d7d 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/version-gen',
>> +                          '@0@'.format(meson.source_root()),
>> +                          check : true).stdout().strip(),
>>      default_options : [
>>          'werror=true',
>>          'warning_level=2',
>> @@ -8,10 +10,10 @@ 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'
>> +vcs = vcs_tag(command: ['utils/version-gen', '.'],
>> +              input: 'version.h.in',
>> +              output: 'version.h',
>> +              fallback: 'v0.0')
>>  
>>  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..09be4b03af45 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"
>> @@ -79,6 +80,8 @@ int CameraManager::start()
>>  	if (enumerator_)
>>  		return -EBUSY;
>>  
>> +	LOG(Camera, Info) << "libcamera version : " << version.toString();
> 
> As the version starts with a v, should we simply print "libcamera " <<
> version ?

Yes, perhaps we should.


> 
>> +
>>  	enumerator_ = DeviceEnumerator::create();
>>  	if (!enumerator_ || enumerator_->enumerate())
>>  		return -ENODEV;
>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
>> index 45bd9d1793aa..0110f1906bc3 100644
>> --- a/src/libcamera/meson.build
>> +++ b/src/libcamera/meson.build
>> @@ -28,6 +28,7 @@ libcamera_sources = files([
>>      'v4l2_device.cpp',
>>      'v4l2_subdevice.cpp',
>>      'v4l2_videodevice.cpp',
>> +    'version.cpp',
>>  ])
>>  
>>  libcamera_headers = files([
>> diff --git a/src/libcamera/version.cpp b/src/libcamera/version.cpp
>> new file mode 100644
>> index 000000000000..d46985c88da5
>> --- /dev/null
>> +++ b/src/libcamera/version.cpp
>> @@ -0,0 +1,35 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2019, Google Inc.
>> + *
>> + * version.cpp - Version information
>> + */
>> +
>> +#include <libcamera/version.h>
>> +
>> +#include "log.h"
>> +
>> +/* The version header is automatically generated at the base of the project. */
>> +#include "../../version.h"
> 
> I still think that the generated version.h header should be installed,
> as it can be useful for application to know which version they have been
> compiled against.

Ok.

> 
>> +
>> +namespace libcamera {
>> +
>> +LOG_DEFINE_CATEGORY(Version)
> 
> Not needed.

Yes, sorry - refactoring artefact. I'll remove.

> 
>> +
>> +/**
>> + * \class LibcameraVersion
>> + *
>> + * Reports library version information
>> + */
>> +
>> +/**
>> + * \brief Return the library version as a string
>> + */
>> +std::string LibcameraVersion::toString() const
>> +{
>> +	return LIBCAMERA_VERSION;
>> +}
>> +
>> +const LibcameraVersion version;
> 
> You can make this much simpler:
> 
> const std::string version(LIBCAMERA_VERSION);
> 
> The version can then be accessed with libcamera::version.

Yes, that's much better.

> 
>> +
>> +} /* namespace libcamera */
>> diff --git a/utils/version-gen b/utils/version-gen
>> new file mode 100755
>> index 000000000000..481616b45c70
>> --- /dev/null
>> +++ b/utils/version-gen
> 
> Let's rename this to version-gen.sh, or even better, gen-version.sh to
> match the naming scheme of our other scripts.


Sure.


> 
>> @@ -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
>> +
>> +# Get a short description from the tree.
>> +version=$(git describe --abbrev=4 --match "v[0-9]*" 2>/dev/null)
>> +
>> +if [ -z "$version" ]
>> +then
>> +	# Handle an un-tagged repository
>> +	sha=$(git describe --abbrev=4 --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'
>> +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"
>> diff --git a/version.h.in b/version.h.in
>> new file mode 100644
>> index 000000000000..ad9134682df6
>> --- /dev/null
>> +++ b/version.h.in
>> @@ -0,0 +1,14 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2019, Google Inc.
>> + *
>> + * version.h - VCS TAG generated file.
>> + *
>> + * This file is auto-generated. Do not modify.
> 
> We use "Do not edit" in other auto-generated files.

Ok.


> 
>> + */
>> +#ifndef __LIBCAMERA_VERSION_TAG_H__
>> +#define __LIBCAMERA_VERSION_TAG_H__
>> +
>> +#define LIBCAMERA_VERSION "@VCS_TAG@"
>> +
>> +#endif // __LIBCAMERA_VERSION_TAG_H__
> 
> /* */ instead of //
> 
> My preference would be for version.h.in to contain both the
> LIBCAMERA_VERSION and the version variable.
> 
> ------------------------------------------------------------------------
> /* SPDX-License-Identifier: LGPL-2.1-or-later */
> /*
>  * Copyright (C) 2019, Google Inc.
>  *
>  * version.h - 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__ */
> ------------------------------------------------------------------------
> 

-- 
Regards
--
Kieran


More information about the libcamera-devel mailing list