[libcamera-devel] [PATCH] libcamera: Auto generate version information
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jun 12 10:39:50 CEST 2019
Hi Kieran,
On Tue, Jun 11, 2019 at 11:25:43PM +0100, Kieran Bingham wrote:
> On 11/06/2019 21:32, Laurent Pinchart wrote:
> > 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.
If you want to keep them please make sure they match the stock template
file.
By the way, where does the build environment provide version information
to sphinx ?
> >> # -- 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.
Sure, I think that's useful, but I think just moving version.h to
include/libcamera/ and installing it would be better. Applications would
have access to the raw data, and can do whatever they want with it. I'd
go one step further, and provide numerical version information too, but
that can be for later.
> 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.
If you want to log the version I would do so in CameraManager. That's
better than in a global constructor that would likely be called before
the logging infrastructure is initialised.
> 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.
Until we hit the right race :-)
> 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.
Note that once you have compiled the project, ninja will have generated
dependency information, and will rebuild the .h and .cpp files
correctly. To test for races you need to wipe the build directory. You
can then use 'ninja target' with the target being set to a file that
includes version.h, and watch it fail when building that target only.
> 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?
I'd say GPL-2+. I just realised that gen-headers doesn't have an SPDX
header either. Feel free to fix both, or I can send a patch.
> >> +
> >> +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 believe we'll introduce bash-isms even if we try not to, and they will
result in failures later. If we mandate usage of bash it should be less
of an issue. If you're confident about being able to write 100%
posix-compliant shell script, I would use [ instead of [[.
> 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?
I don't mind either way, it's included in the Linux kernel version so I
got used to it, I think it would confuse me a bit not to have 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,
Laurent Pinchart
More information about the libcamera-devel
mailing list