[libcamera-devel] [PATCH] libcamera: Auto generate version information
Kieran Bingham
kieran.bingham at ideasonboard.com
Wed Jun 12 00:25:43 CEST 2019
Hi Laurent,
On 11/06/2019 21:32, Laurent Pinchart wrote:
> Hi Kieran,
>
> Thank you for the patch.
>
> On Tue, Jun 11, 2019 at 09:14:11PM +0100, Kieran Bingham wrote:
>> Generate a version string, and provide a global singleton object which
>> allows applications to interogate 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/
>>
>> 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.
>>
>> Finally, a uniquely identifying shortened checksum is provided from git:
>>
>> v0.0.509.c544
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>> Documentation/conf.py | 6 ++++--
>> include/libcamera/meson.build | 1 +
>> include/libcamera/version.h | 23 ++++++++++++++++++++
>> meson.build | 9 +++++++-
>> src/libcamera/meson.build | 1 +
>> src/libcamera/version.cpp | 40 +++++++++++++++++++++++++++++++++++
>> utils/version-gen | 36 +++++++++++++++++++++++++++++++
>> version.h.in | 3 +++
>> 8 files changed, 116 insertions(+), 3 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..e5980b98216d 100644
>> --- a/Documentation/conf.py
>> +++ b/Documentation/conf.py
>> @@ -23,10 +23,12 @@ project = 'libcamera'
>> copyright = '2018-2019, The libcamera documentation authors'
>> author = u'Kieran Bingham, Jacopo Mondi, Laurent Pinchart, Niklas Söderlund'
>>
>> +# Vesion information is provided by the build environment.
>
> s/Vesion/Version/
Eeep. Fixed.
>
>> +#
>> # The short X.Y version
>> -version = ''
>> +# version = ''
>> # The full version, including alpha/beta/rc tags
>> -release = '0.1'
>> +# release = '0.1'
>
> Should we just delete the version and release lines ?
I chose to keep these intentionally. The rest of the conf.py contains
'template' lines.
I can remove if you prefer, but the whole conf.py could probably be
stripped right down too.
>
>> # -- General configuration ---------------------------------------------------
>> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
>> index 1b86fdc7fca4..201832105457 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 = join_paths(meson.current_source_dir(), 'gen-header.sh')
>> diff --git a/include/libcamera/version.h b/include/libcamera/version.h
>> new file mode 100644
>> index 000000000000..ad21f148e748
>> --- /dev/null
>> +++ b/include/libcamera/version.h
>> @@ -0,0 +1,23 @@
>> +/* 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__
>> +
>> +namespace libcamera {
>> +
>> +class LibcameraVersion
>> +{
>> +public:
>> + LibcameraVersion();
>> + std::string toString();
>> +};
>> +
>> +extern LibcameraVersion version;
>> +
>> +} /* namespace libcamera */
>> +
>> +#endif /* __LIBCAMERA_VERSION_H__ */
>
> This seems a bit overkill, can't we just install version.h ?
I wanted to provide a way for an application or otherwise to obtain the
information at runtime.
The static global instance 'version' prints a debug line through our LOG
interface at library start up, and I wanted to be able to expose that
string if needed.
Hrm... seems I messed up the patch anyway, I can't extern a static :-) -
Making it just 'global' to fix.
>> diff --git a/meson.build b/meson.build
>> index 4d3e99d3e58f..5182546382c5 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',
>> @@ -13,6 +15,11 @@ project('libcamera', 'c', 'cpp',
>> # generated from this too.
>> api_version = '0.1'
>>
>> +vcs_tag(command: ['utils/version-gen', '.'],
>> + input: 'version.h.in',
>> + output: 'version.h',
>> + fallback: '')
>
> Mixing tabs and spaces.
Oops! I thought I fixed that.
> Don't you need to add the customtarget object returned by vcs_tag to
> dependencies or sources for something ? Otherwise meson won't know what
> depends on version.h.
I think you're right, but I can't see how to express a dependency on
only a single object file.
I'll have to ask that one in #mesonbuild.
I don't know what magic is happening, but it does seem to be doing the
right thing anyway.
I've tested this with a few iterations of changing files that should not
directly recreate the version.o or libcamera.so - but does affect the
version-gen output, such as changing between a clean tree, and a -dirty
label - and the library does seem to be correctly updated.
I'll call this one 'resolved by magic' for now.
>> +
>> cc = meson.get_compiler('c')
>> config_h = configuration_data()
>>
>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
>> index 1ca1083cf5c7..b9a4153839c9 100644
>> --- a/src/libcamera/meson.build
>> +++ b/src/libcamera/meson.build
>> @@ -25,6 +25,7 @@ libcamera_sources = files([
>> 'utils.cpp',
>> 'v4l2_device.cpp',
>> 'v4l2_subdevice.cpp',
>> + 'version.cpp',
>> ])
>>
>> libcamera_headers = files([
>> diff --git a/src/libcamera/version.cpp b/src/libcamera/version.cpp
>> new file mode 100644
>> index 000000000000..81f692f7cae7
>> --- /dev/null
>> +++ b/src/libcamera/version.cpp
>> @@ -0,0 +1,40 @@
>> +/* 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"
>> +
>> +namespace libcamera {
>> +
>> +LOG_DEFINE_CATEGORY(Version)
>> +
>> +/**
>> + * \class LibcameraVersion
>> + *
>> + * Reports library version information
>> + */
>> +
>> +LibcameraVersion::LibcameraVersion()
>> +{
>> + LOG(Version, Info) << "Libcamera Version " << toString();
>> +}
>> +
>> +/**
>> + * \brief Return the library version as a string
>> + */
>> +std::string LibcameraVersion::toString()
>> +{
>> + return LIBCAMERA_VERSION;
>> +}
>> +
>> +static LibcameraVersion version;
>> +
>> +} /* namespace libcamera */
>> diff --git a/utils/version-gen b/utils/version-gen
>> new file mode 100755
>> index 000000000000..b8e53d77b63e
>> --- /dev/null
>> +++ b/utils/version-gen
>> @@ -0,0 +1,36 @@
>> +#!/bin/sh
>
> Missing SPDX header.
As a component of the build system, is this GPL? or LGPL?
>> +
>> +if test -n "$1"
>
> Please use
>
> if [[ ]] ; then
>
> Our scripts rely on bash, so I think I would specify that explicitly
> instead of /bin/sh.
I am intentionally trying to stay posix shell compliant.
libcamera$ git grep bash
utils/ipu3/ipu3-capture.sh:#!/bin/bash
utils/ipu3/ipu3-process.sh:#!/bin/bash
There's a big difference between non-essential helper scripts for a user
to run - and a script which is executed by the build system...
Thus I'm not sure being reliant on bash is the right thing ...
I should run shellcheck too :)
(Fixed those warnings too)
>
>> +then
>> + cd "$1"
>> +fi
>> +
>> +# No fall back is provided for tarball releases or such, as we do not yet provide releases.
>
> Line break at 80 columns ?
>
Fixed.
> Tarball releases should just ship version.h.
Sure, but the build system will have to know not to regenerate it.
We'll sort that later.
>> +
>> +# Get a short description from the tree.
>> +version=$(git describe --abbrev=4 --match "v[0-9]*" 2>/dev/null)
>> +
>> +if test -z "$version"
>> +then
>
> 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
>> +
>> +
>> +git update-index --refresh > /dev/null 2>&1
>
> What does this do ?
Prevents changed timestamps causing a -dirty label. I've added a comment.
>
>> +dirty=`sh -c 'git diff-index --name-only HEAD' 2>/dev/null` || dirty=
>
> $(...) instead of `...`
>
>> +
>> +# Strip the 'g'
>
> Anything wrong with the 'g' ? :-)
Would you like to keep it?
>> +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 test -n "$dirty"
>> +then
>
> 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..142819645b64
>> --- /dev/null
>> +++ b/version.h.in
>> @@ -0,0 +1,3 @@
>
> Should this have a comment stating that the file is auto-generated ?
Added.
>
>> +#pragma once
>
> We usually use #ifdef #define #endif, is there a specific reason to use
> a #pragma instead ?
>
Probably just because of the sample vcs_tag generator file I looked
at... It was in January so I don't recall anything specific.
(I'm trying to clear my backlog of misc patches)
I'll change to #ifdefs
>> +
>> +#define LIBCAMERA_VERSION "@VCS_TAG@"
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list