[libcamera-devel] [PATCH v2 2/4] libcamera: Auto generate version information
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jul 3 14:26:52 CEST 2019
Hi Kieran,
On Wed, Jul 03, 2019 at 01:19:18PM +0100, Kieran Bingham wrote:
> 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.
But it doesn't match semver.org :-) A dash or plus separator would be
better.
> '4' is a minimum, and if collisions occur it will be increased
> automatically by git.
Chances of collisions being really high with 4, should be already
increase the minimum to avoid generating random-length version strings ?
I'm fine if the length occasionally gets bigger, but if it changes all
the time it's a big annoying.
> >> 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?
Look at the browser title bar in libcamera.org :-)
> 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,
Laurent Pinchart
More information about the libcamera-devel
mailing list