[{"id":2193,"web_url":"https://patchwork.libcamera.org/comment/2193/","msgid":"<5f131a62-6ae7-12a2-4a74-f7b17de0e20d@ideasonboard.com>","date":"2019-07-08T15:05:22","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: Rework automatic\n\tversion generation to avoid rebuilds","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 08/07/2019 14:12, Laurent Pinchart wrote:\n> Commit b817bcec6b53 (\"libcamera: Auto generate version information\")\n> generates version information in order to automatically include it\n> various locations (Sphinx and Doxygen documentation, libcamera::version\n> variable available at runtime, and version.h available at compile time).\n> Unfortunately this causes lots of unnecessary rebuilds when modifying\n> the git tree state, which hinders development.\n> \n> The problem is caused by the generated version.h being listed as a\n> dependency for the whole libcamera. This is required as meson (to the\n> best of my knowledge) doesn't provide a way to explicitly specify the\n> dependency of a single object file (camera_manager.o in this case, as\n> camera_manager.cpp is the only consumer of the generated version string)\n> on the custom target used to generate version.h. The dependency can't be\n> automatically detected at build time, like dependencies on normal\n> headers that are generated by parsing the source, because the version.h\n> header may not exist yet. The build could then fail in a racy way.\n> \n> This change attempts at solving the issue by generating a version.cpp\n> instead of a version.h to set the git-based version. This minimises the\n> number of files that need to be rebuild when then git tree state\n> changes, while retaining the main purpose of the original automatic\n> version generation, the ability to access the git-based version string\n> at runtime. We however lose the ability to access git-based version\n> information at build time in an application building against libcamera,\n> but there is no expected use case for this.\n> \n> On the other hand, major, minor and patch level version numbers are\n> useful at build time. This commit changes the generation of version.h in\n> order to add three macros named LIBCAMERA_VERSION_MAJOR,\n> LIBCAMERA_VERSION_MINOR and LIBCAMERA_VERSION_PATCH for this purpose.\n> version.h is not included by any other libcamera header or source file,\n> and thus doesn't force a rebuild of the library.\n> \n> The Sphinx and Doxygen documentation keep their git-based version\n> information, which is set during the configuration of the build and then\n> doesn't track git commits. We may want to investigate how to improve\n> this, but given that git-based version for the documentation has very\n> few use cases outside of tagging nightly builds, this isn't considered\n> an issue at the moment.\n> \n> The documentation install directory now uses the base version string, in\n> order to avoid increasing the number of documentation directories\n> needlessly. This shouldn't cause any issue as the API should not change\n> without a change to the version number.\n> \n> The version number generation and handling code now also standardises\n> the version variables to not start with a 'v' prefix in meson, in order\n> to simplify their handling. The prefix is added when generating the\n> relevant files.\n> \n> Note that we go back to specifying the fallback version in the main\n> meson.build, in the call to the project() function. For the time being I\n> believe this should be a good compromise to avoid unnecessary\n> recompilation, and moving the fallback version to a different file for\n> tarball releases can be built on top of this.\n> \n> Fixes: b817bcec6b53 (\"libcamera: Auto generate version information\")\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nWhen I apply your patch, or build from your branch I get the following:\n\n\nIn file included from ../src/cam/main.cpp:13:\ninclude/libcamera/libcamera.h:11:10: fatal error: libcamera/config.h: No\nsuch file or directory\n #include <libcamera/config.h>\n\n\nBut that seems unrelated???\n\nPerhaps it's uncovered some race or lack of dependency from a previous\ncommit.\n\nI've removed my entire build directory, and I still get the config.h\nfault ... hrm.\n\nIn fact, gen-header.sh seems to be broken for me:\n\ncat build/include/libcamera/libcamera.h\n/* SPDX-License-Identifier: LGPL-2.1-or-later */\n/* This file is auto-generated, do not edit! */\n/*\n * Copyright (C) 2018-2019, Google Inc.\n *\n * libcamera.h - libcamera public API\n */\n#ifndef __LIBCAMERA_LIBCAMERA_H__\n#define __LIBCAMERA_LIBCAMERA_H__\n\n#include <libcamera/config.h>\n#include <libcamera/version.h>\n\n#endif /* __LIBCAMERA_LIBCAMERA_H__ */\n\n\n\n> ---\n>  Documentation/meson.build          |  6 +++---\n>  include/libcamera/camera_manager.h |  3 +++\n>  include/libcamera/gen-header.sh    |  6 +++++-\n>  include/libcamera/meson.build      | 18 +++++++++++-------\n>  include/libcamera/version.h.in     | 12 +++---------\n>  meson.build                        | 19 ++++++++++++++++---\n>  src/libcamera/camera_manager.cpp   | 14 +++++++-------\n>  src/libcamera/meson.build          | 10 +++++++++-\n>  src/libcamera/version.cpp.in       | 16 ++++++++++++++++\n>  src/qcam/main_window.cpp           |  2 +-\n>  utils/gen-version.sh               | 23 ++++++++++-------------\n>  11 files changed, 84 insertions(+), 45 deletions(-)\n>  create mode 100644 src/libcamera/version.cpp.in\n> \n> diff --git a/Documentation/meson.build b/Documentation/meson.build\n> index c355d5feb504..b1720b05f5ee 100644\n> --- a/Documentation/meson.build\n> +++ b/Documentation/meson.build\n> @@ -1,5 +1,5 @@\n>  doc_install_dir = join_paths(get_option('datadir'), 'doc',\n> -                             'libcamera-@0@'.format(meson.project_version()))\n> +                             'libcamera-@0@'.format(libcamera_version))\n>  \n>  #\n>  # Doxygen\n> @@ -9,7 +9,7 @@ doxygen = find_program('doxygen', required : false)\n>  \n>  if doxygen.found()\n>      cdata = configuration_data()\n> -    cdata.set('VERSION', meson.project_version())\n> +    cdata.set('VERSION', 'v@0@'.format(libcamera_git_version))\n>      cdata.set('TOP_SRCDIR', meson.source_root())\n>      cdata.set('TOP_BUILDDIR', meson.build_root())\n>  \n> @@ -48,7 +48,7 @@ if sphinx.found()\n>          'index.rst',\n>      ]\n>  \n> -    release = 'release=' + meson.project_version()\n> +    release = 'release=v' + libcamera_git_version\n>  \n>      custom_target('documentation',\n>                    command : [sphinx, '-D', release, '-q', '-W', '-b', 'html',\n> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> index cf3a85ae8fe3..633d27d17ebf 100644\n> --- a/include/libcamera/camera_manager.h\n> +++ b/include/libcamera/camera_manager.h\n> @@ -31,6 +31,7 @@ public:\n>  \tvoid removeCamera(Camera *camera);\n>  \n>  \tstatic CameraManager *instance();\n> +\tstatic const std::string &version() { return version_; }\n>  \n>  \tvoid setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher);\n>  \tEventDispatcher *eventDispatcher();\n> @@ -46,6 +47,8 @@ private:\n>  \tstd::vector<std::shared_ptr<Camera>> cameras_;\n>  \n>  \tstd::unique_ptr<EventDispatcher> dispatcher_;\n> +\n> +\tstatic const std::string version_;\n>  };\n>  \n>  } /* namespace libcamera */\n> diff --git a/include/libcamera/gen-header.sh b/include/libcamera/gen-header.sh\n> index e171c08c20b8..ccd3414bd8ca 100755\n> --- a/include/libcamera/gen-header.sh\n> +++ b/include/libcamera/gen-header.sh\n> @@ -16,8 +16,12 @@ cat <<EOF > \"$dst_file\"\n>  \n>  EOF\n>  \n> -for header in \"$src_dir\"/*.h ; do\n> +headers=$(for header in $\"$src_dir\"/*.h ; do\n\nI wonder if that extra $ is to blame...\n\n\n>  \theader=$(basename \"$header\")\n> +\techo $header\n> +done ; echo \"version.h\" | sort)\n> +\n> +for header in $(echo $headers) ; do\n>  \techo \"#include <libcamera/$header>\" >> \"$dst_file\"\n>  done\n>  \n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index 6f81f1117318..972513fc2df5 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -16,13 +16,6 @@ libcamera_api = files([\n>      'timer.h',\n>  ])\n>  \n> -gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')\n> -\n> -version_h = vcs_tag(command : [gen_version, meson.current_source_dir()],\n> -                    input : 'version.h.in',\n> -                    output : 'version.h',\n> -                    fallback : 'v0.0')\n> -\n>  gen_header = files('gen-header.sh')\n>  \n>  libcamera_h = custom_target('gen-header',\n> @@ -32,5 +25,16 @@ libcamera_h = custom_target('gen-header',\n>                              install : true,\n>                              install_dir : 'include/libcamera')\n>  \n> +version = libcamera_version.split('.')\n> +libcamera_version_config = configuration_data()\n> +libcamera_version_config.set('LIBCAMERA_VERSION_MAJOR', version[0])\n> +libcamera_version_config.set('LIBCAMERA_VERSION_MINOR', version[1])\n> +libcamera_version_config.set('LIBCAMERA_VERSION_PATCH', version[2])\n> +\n> +configure_file(input : 'version.h.in',\n> +               output : 'version.h',\n> +               configuration : libcamera_version_config,\n> +               install_dir : 'include/libcamera')\n> +\n>  install_headers(libcamera_api,\n>                  subdir : 'libcamera')\n> diff --git a/include/libcamera/version.h.in b/include/libcamera/version.h.in\n> index e49b36962aed..5e9a30911d12 100644\n> --- a/include/libcamera/version.h.in\n> +++ b/include/libcamera/version.h.in\n> @@ -9,14 +9,8 @@\n>  #ifndef __LIBCAMERA_VERSION_H__\n>  #define __LIBCAMERA_VERSION_H__\n>  \n> -#include <string>\n> -\n> -#define LIBCAMERA_VERSION \"@VCS_TAG@\"\n> -\n> -namespace libcamera {\n> -\n> -extern const std::string version;\n> -\n> -} /* namespace libcamera */\n> +#define LIBCAMERA_VERSION_MAJOR\t\t@LIBCAMERA_VERSION_MAJOR@\n> +#define LIBCAMERA_VERSION_MINOR\t\t@LIBCAMERA_VERSION_MINOR@\n> +#define LIBCAMERA_VERSION_PATCH\t\t@LIBCAMERA_VERSION_PATCH@\n>  \n>  #endif /* __LIBCAMERA_VERSION_H__ */\n> diff --git a/meson.build b/meson.build\n> index 342b3cc76a93..8f3d0ce91cb8 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -1,8 +1,6 @@\n>  project('libcamera', 'c', 'cpp',\n>      meson_version : '>= 0.40',\n> -    version : run_command('utils/gen-version.sh',\n> -                          '@0@'.format(meson.source_root()),\n> -                          check : true).stdout().strip(),\n> +    version : '0.0.0',\n\nI dislike not being able to populate the meson.project_version().\n\nI still think that fallback should be handled in the gen-version.sh script.\n\n\n\n\n>      default_options : [\n>          'werror=true',\n>          'warning_level=2',\n> @@ -10,6 +8,21 @@ project('libcamera', 'c', 'cpp',\n>      ],\n>      license : 'LGPL 2.1+')\n>  \n> +# Generate version information. The libcamera_git_version variable contains the\n> +# full version with git patch count and SHA1 (e.g. 1.2.3+211-c94a24f4), while\n> +# the libcamera_version variable contains the major.minor.patch (e.g. 1.2.3)\n> +# only. If the source tree isn't under git control, or if it matches the last\n> +# git version tag, the build metadata (e.g. +211-c94a24f4) is omitted from\n> +# libcamera_git_version.\n> +libcamera_git_version = run_command('utils/gen-version.sh',\n> +                                    meson.source_root()).stdout().strip()\n> +if libcamera_git_version == ''\n> +    libcamera_git_version = meson.project_version()\n\n...\n\n> +endif\n> +\n> +libcamera_version = libcamera_git_version.split('+')[0]\n> +\n> +# Configure the build environment.\n>  cc = meson.get_compiler('c')\n>  config_h = configuration_data()\n>  \n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index c5da46b4062d..337496c21cfc 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -9,7 +9,6 @@\n>  \n>  #include <libcamera/camera.h>\n>  #include <libcamera/event_dispatcher.h>\n> -#include <libcamera/version.h>\n>  \n>  #include \"device_enumerator.h\"\n>  #include \"event_dispatcher_poll.h\"\n> @@ -26,11 +25,6 @@ namespace libcamera {\n>  \n>  LOG_DEFINE_CATEGORY(Camera)\n>  \n> -/**\n> - * \\brief The library global version string\n> - */\n> -const std::string version(LIBCAMERA_VERSION);\n> -\n>  /**\n>   * \\class CameraManager\n>   * \\brief Provide access and manage all cameras in the system\n> @@ -85,7 +79,7 @@ int CameraManager::start()\n>  \tif (enumerator_)\n>  \t\treturn -EBUSY;\n>  \n> -\tLOG(Camera, Info) << \"libcamera \" << version;\n> +\tLOG(Camera, Info) << \"libcamera \" << version_;\n>  \n>  \tenumerator_ = DeviceEnumerator::create();\n>  \tif (!enumerator_ || enumerator_->enumerate())\n> @@ -232,6 +226,12 @@ CameraManager *CameraManager::instance()\n>  \treturn &manager;\n>  }\n>  \n> +/**\n> + * \\fn const std::string &CameraManager::version()\n> + * \\brief Retrieve the libcamera version string\n> + * \\return The libcamera version string\n> + */\n> +\n>  /**\n>   * \\brief Set the event dispatcher\n>   * \\param[in] dispatcher Pointer to the event dispatcher\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 336f4f066fac..97ff86e2167f 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -79,8 +79,16 @@ control_types_cpp = custom_target('control_types_cpp',\n>  \n>  libcamera_sources += control_types_cpp\n>  \n> +gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')\n> +\n> +version_cpp = vcs_tag(command : [gen_version, meson.source_root()],\n> +                      input : 'version.cpp.in',\n> +                      output : 'version.cpp',\n> +                      fallback : meson.project_version())\n\nNo point specifying the fallback if that's the default fallback?\n\n> +\n> +libcamera_sources += version_cpp\n> +\n>  libcamera_deps = [\n> -    declare_dependency(sources : version_h),>      cc.find_library('dl'),\n>      libudev,\n>  ]\n> diff --git a/src/libcamera/version.cpp.in b/src/libcamera/version.cpp.in\n> new file mode 100644\n> index 000000000000..5aec08a189f3\n> --- /dev/null\n> +++ b/src/libcamera/version.cpp.in\n> @@ -0,0 +1,16 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * version.cpp - libcamera version\n> + *\n> + * This file is auto-generated. Do not edit.\n> + */\n> +\n> +#include <libcamera/camera_manager.h>\n> +\n> +namespace libcamera {\n> +\n> +const std::string CameraManager::version_(\"v@VCS_TAG@\");\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index b2f3a1f324f2..907d2423ed15 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -26,7 +26,7 @@ MainWindow::MainWindow(const OptionsParser::Options &options)\n>  {\n>  \tint ret;\n>  \n> -\ttitle_ = \"QCam \" + QString::fromStdString(libcamera::version);\n> +\ttitle_ = \"QCam \" + QString::fromStdString(CameraManager::version());\n\nWhy do you prefer exposing the version in the CameraManager namespace,\nrather than libcamera?\n\nIt's the version of libcamera, the CameraManager is not versioned\ndifferently from the rest of the library...\n\nNow that you're moving the version (back) to a version.cpp I don't see\nthe need for it to be in the CameraManager namespace. I could have\nunderstood the reasoning while it was 'borrowing' the CameraManager C++\nobject file perhaps...\n\n\nNot objecting to the namespace choice, I just haven't understood the\nrationale for the move.\n\n\n>  \tsetWindowTitle(title_);\n>  \tconnect(&titleTimer_, SIGNAL(timeout()), this, SLOT(updateTitle()));\n>  \n> diff --git a/utils/gen-version.sh b/utils/gen-version.sh\n> index b3003d7a80d3..1800d71642c0 100755\n> --- a/utils/gen-version.sh\n> +++ b/utils/gen-version.sh\n> @@ -8,6 +8,9 @@ then\n>  \tcd \"$1\" 2>/dev/null || exit 1\n>  fi\n>  \n> +# Bail out if the directory isn't under git control\n> +git rev-parse --git-dir >/dev/null 2>&1 || exit 1\n> +\n\nThis is where I believe the script should (in the future, or perhaps\nnear future) identify the appropriate libcamera_version file and return\nthat content.\n\nThe file would be automatically generated when creating a release.\n\n\nI'll submit a patch to do all of this after you've got this one how you\nwant it. I had always planned to do the 'release' stage after the\ninitial versioning.\n\n\n\n>  # Get a short description from the tree.\n>  version=$(git describe --abbrev=8 --match \"v[0-9]*\" 2>/dev/null)\n>  \n> @@ -16,22 +19,16 @@ then\n>  \t# Handle an un-tagged repository\n>  \tsha=$(git describe --abbrev=8 --always 2>/dev/null)\n>  \tcommits=$(git log --oneline | wc -l 2>/dev/null)\n> -\tversion=v0.0.$commits.$sha\n> +\tversion=\"v0.0.0-$commits-g$sha\"\n>  fi\n>  \n> -# Prevent changed timestamps causing -dirty labels\n> +# Append a '-dirty' suffix is the working tree is dirty. Prevent false\n\ns/suffix is/suffix if/\n\n> +# positives due to changed timestamps by running git update-index.\n>  git update-index --refresh > /dev/null 2>&1\n> -dirty=$(git diff-index --name-only HEAD 2>/dev/null) || dirty=\n> +git diff-index --quiet HEAD || version=\"$version-dirty\"\n>  \n> -# Strip the 'g', and replace the preceeding '-' with a '+' to denote a label\n> -version=$(echo \"$version\" | sed -e 's/-g/+/g')\n> -\n> -# Fix the '-' (the patch count) to a '.' as a version increment.\n> -version=$(echo \"$version\" | sed -e 's/-/./g')\n> -\n> -if [ -n \"$dirty\" ]\n> -then\n> -\tversion=$version-dirty\n> -fi\n> +# Replace first '-' with a '+' to denote build metadata, strip the 'g' in from\n> +# of the git SHA1 and remove the initial 'v'.\n> +version=$(echo \"$version\" | sed -e 's/-/+/' | sed -e 's/-g/-/' | cut -c 2-)\n\nAt first this was a bit painful to read. It looks like you've removed\nthe '-' and replaced with '+', which would then be used by the second\nsed, however I see that they only affect the first instance.\n\nNot really anything to change though, just me misreading I guess.\n\n\n>  \n>  echo \"$version\"\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 594DE60C1C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  8 Jul 2019 17:05:26 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9A5AF56A;\n\tMon,  8 Jul 2019 17:05:25 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1562598325;\n\tbh=azoDDlmpqlqc2VofOqHZACUithSg6UpYdjH6epT3KLA=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=Eaz9QBPiV3dkEveJqHaV4n2apkP6xgR/VMqEKspCTaPCFjWBrDJ+5aaErbkaj6GBy\n\tDJ5V0QWydgHJcIrvVmL9l5a6BGu0kh6cMD1GijfHh4FkhPk3CYpoYf5oM1u/GGGB2m\n\tp25TWXVSYlD01HJ6jaH/vwqHCEAy+8lbHh+vEKcY=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20190708131212.15275-1-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<5f131a62-6ae7-12a2-4a74-f7b17de0e20d@ideasonboard.com>","Date":"Mon, 8 Jul 2019 16:05:22 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.1","MIME-Version":"1.0","In-Reply-To":"<20190708131212.15275-1-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: Rework automatic\n\tversion generation to avoid rebuilds","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 08 Jul 2019 15:05:26 -0000"}},{"id":2195,"web_url":"https://patchwork.libcamera.org/comment/2195/","msgid":"<81d3b110-b62f-3840-ad60-927387265226@ideasonboard.com>","date":"2019-07-09T08:43:39","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: Rework automatic\n\tversion generation to avoid rebuilds","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nPlease also note the following shell-check warnings:\n\n> shellcheck ./include/libcamera/gen-header.sh \n> \n> In ./include/libcamera/gen-header.sh line 19:\n> headers=$(for header in $\"$src_dir\"/*.h ; do\n>                         ^-- SC2039: In POSIX sh, $\"..\" is undefined.\n> \n> \n> In ./include/libcamera/gen-header.sh line 21:\n> \techo $header\n>              ^-- SC2086: Double quote to prevent globbing and word splitting.\n> \n> \n> In ./include/libcamera/gen-header.sh line 24:\n> for header in $(echo $headers) ; do\n>               ^-- SC2116: Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'.\n>                      ^-- SC2086: Double quote to prevent globbing and word splitting.\n> \n\n--\nRegards\n\nKieran\n\n\nOn 08/07/2019 16:05, Kieran Bingham wrote:\n> Hi Laurent,\n> \n> On 08/07/2019 14:12, Laurent Pinchart wrote:\n>> Commit b817bcec6b53 (\"libcamera: Auto generate version information\")\n>> generates version information in order to automatically include it\n>> various locations (Sphinx and Doxygen documentation, libcamera::version\n>> variable available at runtime, and version.h available at compile time).\n>> Unfortunately this causes lots of unnecessary rebuilds when modifying\n>> the git tree state, which hinders development.\n>>\n>> The problem is caused by the generated version.h being listed as a\n>> dependency for the whole libcamera. This is required as meson (to the\n>> best of my knowledge) doesn't provide a way to explicitly specify the\n>> dependency of a single object file (camera_manager.o in this case, as\n>> camera_manager.cpp is the only consumer of the generated version string)\n>> on the custom target used to generate version.h. The dependency can't be\n>> automatically detected at build time, like dependencies on normal\n>> headers that are generated by parsing the source, because the version.h\n>> header may not exist yet. The build could then fail in a racy way.\n>>\n>> This change attempts at solving the issue by generating a version.cpp\n>> instead of a version.h to set the git-based version. This minimises the\n>> number of files that need to be rebuild when then git tree state\n>> changes, while retaining the main purpose of the original automatic\n>> version generation, the ability to access the git-based version string\n>> at runtime. We however lose the ability to access git-based version\n>> information at build time in an application building against libcamera,\n>> but there is no expected use case for this.\n>>\n>> On the other hand, major, minor and patch level version numbers are\n>> useful at build time. This commit changes the generation of version.h in\n>> order to add three macros named LIBCAMERA_VERSION_MAJOR,\n>> LIBCAMERA_VERSION_MINOR and LIBCAMERA_VERSION_PATCH for this purpose.\n>> version.h is not included by any other libcamera header or source file,\n>> and thus doesn't force a rebuild of the library.\n>>\n>> The Sphinx and Doxygen documentation keep their git-based version\n>> information, which is set during the configuration of the build and then\n>> doesn't track git commits. We may want to investigate how to improve\n>> this, but given that git-based version for the documentation has very\n>> few use cases outside of tagging nightly builds, this isn't considered\n>> an issue at the moment.\n>>\n>> The documentation install directory now uses the base version string, in\n>> order to avoid increasing the number of documentation directories\n>> needlessly. This shouldn't cause any issue as the API should not change\n>> without a change to the version number.\n>>\n>> The version number generation and handling code now also standardises\n>> the version variables to not start with a 'v' prefix in meson, in order\n>> to simplify their handling. The prefix is added when generating the\n>> relevant files.\n>>\n>> Note that we go back to specifying the fallback version in the main\n>> meson.build, in the call to the project() function. For the time being I\n>> believe this should be a good compromise to avoid unnecessary\n>> recompilation, and moving the fallback version to a different file for\n>> tarball releases can be built on top of this.\n>>\n>> Fixes: b817bcec6b53 (\"libcamera: Auto generate version information\")\n>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> When I apply your patch, or build from your branch I get the following:\n> \n> \n> In file included from ../src/cam/main.cpp:13:\n> include/libcamera/libcamera.h:11:10: fatal error: libcamera/config.h: No\n> such file or directory\n>  #include <libcamera/config.h>\n> \n> \n> But that seems unrelated???\n> \n> Perhaps it's uncovered some race or lack of dependency from a previous\n> commit.\n> \n> I've removed my entire build directory, and I still get the config.h\n> fault ... hrm.\n> \n> In fact, gen-header.sh seems to be broken for me:\n> \n> cat build/include/libcamera/libcamera.h\n> /* SPDX-License-Identifier: LGPL-2.1-or-later */\n> /* This file is auto-generated, do not edit! */\n> /*\n>  * Copyright (C) 2018-2019, Google Inc.\n>  *\n>  * libcamera.h - libcamera public API\n>  */\n> #ifndef __LIBCAMERA_LIBCAMERA_H__\n> #define __LIBCAMERA_LIBCAMERA_H__\n> \n> #include <libcamera/config.h>\n> #include <libcamera/version.h>\n> \n> #endif /* __LIBCAMERA_LIBCAMERA_H__ */\n> \n> \n> \n>> ---\n>>  Documentation/meson.build          |  6 +++---\n>>  include/libcamera/camera_manager.h |  3 +++\n>>  include/libcamera/gen-header.sh    |  6 +++++-\n>>  include/libcamera/meson.build      | 18 +++++++++++-------\n>>  include/libcamera/version.h.in     | 12 +++---------\n>>  meson.build                        | 19 ++++++++++++++++---\n>>  src/libcamera/camera_manager.cpp   | 14 +++++++-------\n>>  src/libcamera/meson.build          | 10 +++++++++-\n>>  src/libcamera/version.cpp.in       | 16 ++++++++++++++++\n>>  src/qcam/main_window.cpp           |  2 +-\n>>  utils/gen-version.sh               | 23 ++++++++++-------------\n>>  11 files changed, 84 insertions(+), 45 deletions(-)\n>>  create mode 100644 src/libcamera/version.cpp.in\n>>\n>> diff --git a/Documentation/meson.build b/Documentation/meson.build\n>> index c355d5feb504..b1720b05f5ee 100644\n>> --- a/Documentation/meson.build\n>> +++ b/Documentation/meson.build\n>> @@ -1,5 +1,5 @@\n>>  doc_install_dir = join_paths(get_option('datadir'), 'doc',\n>> -                             'libcamera-@0@'.format(meson.project_version()))\n>> +                             'libcamera-@0@'.format(libcamera_version))\n>>  \n>>  #\n>>  # Doxygen\n>> @@ -9,7 +9,7 @@ doxygen = find_program('doxygen', required : false)\n>>  \n>>  if doxygen.found()\n>>      cdata = configuration_data()\n>> -    cdata.set('VERSION', meson.project_version())\n>> +    cdata.set('VERSION', 'v@0@'.format(libcamera_git_version))\n>>      cdata.set('TOP_SRCDIR', meson.source_root())\n>>      cdata.set('TOP_BUILDDIR', meson.build_root())\n>>  \n>> @@ -48,7 +48,7 @@ if sphinx.found()\n>>          'index.rst',\n>>      ]\n>>  \n>> -    release = 'release=' + meson.project_version()\n>> +    release = 'release=v' + libcamera_git_version\n>>  \n>>      custom_target('documentation',\n>>                    command : [sphinx, '-D', release, '-q', '-W', '-b', 'html',\n>> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n>> index cf3a85ae8fe3..633d27d17ebf 100644\n>> --- a/include/libcamera/camera_manager.h\n>> +++ b/include/libcamera/camera_manager.h\n>> @@ -31,6 +31,7 @@ public:\n>>  \tvoid removeCamera(Camera *camera);\n>>  \n>>  \tstatic CameraManager *instance();\n>> +\tstatic const std::string &version() { return version_; }\n>>  \n>>  \tvoid setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher);\n>>  \tEventDispatcher *eventDispatcher();\n>> @@ -46,6 +47,8 @@ private:\n>>  \tstd::vector<std::shared_ptr<Camera>> cameras_;\n>>  \n>>  \tstd::unique_ptr<EventDispatcher> dispatcher_;\n>> +\n>> +\tstatic const std::string version_;\n>>  };\n>>  \n>>  } /* namespace libcamera */\n>> diff --git a/include/libcamera/gen-header.sh b/include/libcamera/gen-header.sh\n>> index e171c08c20b8..ccd3414bd8ca 100755\n>> --- a/include/libcamera/gen-header.sh\n>> +++ b/include/libcamera/gen-header.sh\n>> @@ -16,8 +16,12 @@ cat <<EOF > \"$dst_file\"\n>>  \n>>  EOF\n>>  \n>> -for header in \"$src_dir\"/*.h ; do\n>> +headers=$(for header in $\"$src_dir\"/*.h ; do\n> \n> I wonder if that extra $ is to blame...\n> \n> \n>>  \theader=$(basename \"$header\")\n>> +\techo $header\n>> +done ; echo \"version.h\" | sort)\n>> +\n>> +for header in $(echo $headers) ; do\n>>  \techo \"#include <libcamera/$header>\" >> \"$dst_file\"\n>>  done\n>>  \n>> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n>> index 6f81f1117318..972513fc2df5 100644\n>> --- a/include/libcamera/meson.build\n>> +++ b/include/libcamera/meson.build\n>> @@ -16,13 +16,6 @@ libcamera_api = files([\n>>      'timer.h',\n>>  ])\n>>  \n>> -gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')\n>> -\n>> -version_h = vcs_tag(command : [gen_version, meson.current_source_dir()],\n>> -                    input : 'version.h.in',\n>> -                    output : 'version.h',\n>> -                    fallback : 'v0.0')\n>> -\n>>  gen_header = files('gen-header.sh')\n>>  \n>>  libcamera_h = custom_target('gen-header',\n>> @@ -32,5 +25,16 @@ libcamera_h = custom_target('gen-header',\n>>                              install : true,\n>>                              install_dir : 'include/libcamera')\n>>  \n>> +version = libcamera_version.split('.')\n>> +libcamera_version_config = configuration_data()\n>> +libcamera_version_config.set('LIBCAMERA_VERSION_MAJOR', version[0])\n>> +libcamera_version_config.set('LIBCAMERA_VERSION_MINOR', version[1])\n>> +libcamera_version_config.set('LIBCAMERA_VERSION_PATCH', version[2])\n>> +\n>> +configure_file(input : 'version.h.in',\n>> +               output : 'version.h',\n>> +               configuration : libcamera_version_config,\n>> +               install_dir : 'include/libcamera')\n>> +\n>>  install_headers(libcamera_api,\n>>                  subdir : 'libcamera')\n>> diff --git a/include/libcamera/version.h.in b/include/libcamera/version.h.in\n>> index e49b36962aed..5e9a30911d12 100644\n>> --- a/include/libcamera/version.h.in\n>> +++ b/include/libcamera/version.h.in\n>> @@ -9,14 +9,8 @@\n>>  #ifndef __LIBCAMERA_VERSION_H__\n>>  #define __LIBCAMERA_VERSION_H__\n>>  \n>> -#include <string>\n>> -\n>> -#define LIBCAMERA_VERSION \"@VCS_TAG@\"\n>> -\n>> -namespace libcamera {\n>> -\n>> -extern const std::string version;\n>> -\n>> -} /* namespace libcamera */\n>> +#define LIBCAMERA_VERSION_MAJOR\t\t@LIBCAMERA_VERSION_MAJOR@\n>> +#define LIBCAMERA_VERSION_MINOR\t\t@LIBCAMERA_VERSION_MINOR@\n>> +#define LIBCAMERA_VERSION_PATCH\t\t@LIBCAMERA_VERSION_PATCH@\n>>  \n>>  #endif /* __LIBCAMERA_VERSION_H__ */\n>> diff --git a/meson.build b/meson.build\n>> index 342b3cc76a93..8f3d0ce91cb8 100644\n>> --- a/meson.build\n>> +++ b/meson.build\n>> @@ -1,8 +1,6 @@\n>>  project('libcamera', 'c', 'cpp',\n>>      meson_version : '>= 0.40',\n>> -    version : run_command('utils/gen-version.sh',\n>> -                          '@0@'.format(meson.source_root()),\n>> -                          check : true).stdout().strip(),\n>> +    version : '0.0.0',\n> \n> I dislike not being able to populate the meson.project_version().\n> \n> I still think that fallback should be handled in the gen-version.sh script.\n> \n> \n> \n> \n>>      default_options : [\n>>          'werror=true',\n>>          'warning_level=2',\n>> @@ -10,6 +8,21 @@ project('libcamera', 'c', 'cpp',\n>>      ],\n>>      license : 'LGPL 2.1+')\n>>  \n>> +# Generate version information. The libcamera_git_version variable contains the\n>> +# full version with git patch count and SHA1 (e.g. 1.2.3+211-c94a24f4), while\n>> +# the libcamera_version variable contains the major.minor.patch (e.g. 1.2.3)\n>> +# only. If the source tree isn't under git control, or if it matches the last\n>> +# git version tag, the build metadata (e.g. +211-c94a24f4) is omitted from\n>> +# libcamera_git_version.\n>> +libcamera_git_version = run_command('utils/gen-version.sh',\n>> +                                    meson.source_root()).stdout().strip()\n>> +if libcamera_git_version == ''\n>> +    libcamera_git_version = meson.project_version()\n> \n> ...\n> \n>> +endif\n>> +\n>> +libcamera_version = libcamera_git_version.split('+')[0]\n>> +\n>> +# Configure the build environment.\n>>  cc = meson.get_compiler('c')\n>>  config_h = configuration_data()\n>>  \n>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n>> index c5da46b4062d..337496c21cfc 100644\n>> --- a/src/libcamera/camera_manager.cpp\n>> +++ b/src/libcamera/camera_manager.cpp\n>> @@ -9,7 +9,6 @@\n>>  \n>>  #include <libcamera/camera.h>\n>>  #include <libcamera/event_dispatcher.h>\n>> -#include <libcamera/version.h>\n>>  \n>>  #include \"device_enumerator.h\"\n>>  #include \"event_dispatcher_poll.h\"\n>> @@ -26,11 +25,6 @@ namespace libcamera {\n>>  \n>>  LOG_DEFINE_CATEGORY(Camera)\n>>  \n>> -/**\n>> - * \\brief The library global version string\n>> - */\n>> -const std::string version(LIBCAMERA_VERSION);\n>> -\n>>  /**\n>>   * \\class CameraManager\n>>   * \\brief Provide access and manage all cameras in the system\n>> @@ -85,7 +79,7 @@ int CameraManager::start()\n>>  \tif (enumerator_)\n>>  \t\treturn -EBUSY;\n>>  \n>> -\tLOG(Camera, Info) << \"libcamera \" << version;\n>> +\tLOG(Camera, Info) << \"libcamera \" << version_;\n>>  \n>>  \tenumerator_ = DeviceEnumerator::create();\n>>  \tif (!enumerator_ || enumerator_->enumerate())\n>> @@ -232,6 +226,12 @@ CameraManager *CameraManager::instance()\n>>  \treturn &manager;\n>>  }\n>>  \n>> +/**\n>> + * \\fn const std::string &CameraManager::version()\n>> + * \\brief Retrieve the libcamera version string\n>> + * \\return The libcamera version string\n>> + */\n>> +\n>>  /**\n>>   * \\brief Set the event dispatcher\n>>   * \\param[in] dispatcher Pointer to the event dispatcher\n>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>> index 336f4f066fac..97ff86e2167f 100644\n>> --- a/src/libcamera/meson.build\n>> +++ b/src/libcamera/meson.build\n>> @@ -79,8 +79,16 @@ control_types_cpp = custom_target('control_types_cpp',\n>>  \n>>  libcamera_sources += control_types_cpp\n>>  \n>> +gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')\n>> +\n>> +version_cpp = vcs_tag(command : [gen_version, meson.source_root()],\n>> +                      input : 'version.cpp.in',\n>> +                      output : 'version.cpp',\n>> +                      fallback : meson.project_version())\n> \n> No point specifying the fallback if that's the default fallback?\n> \n>> +\n>> +libcamera_sources += version_cpp\n>> +\n>>  libcamera_deps = [\n>> -    declare_dependency(sources : version_h),>      cc.find_library('dl'),\n>>      libudev,\n>>  ]\n>> diff --git a/src/libcamera/version.cpp.in b/src/libcamera/version.cpp.in\n>> new file mode 100644\n>> index 000000000000..5aec08a189f3\n>> --- /dev/null\n>> +++ b/src/libcamera/version.cpp.in\n>> @@ -0,0 +1,16 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2019, Google Inc.\n>> + *\n>> + * version.cpp - libcamera version\n>> + *\n>> + * This file is auto-generated. Do not edit.\n>> + */\n>> +\n>> +#include <libcamera/camera_manager.h>\n>> +\n>> +namespace libcamera {\n>> +\n>> +const std::string CameraManager::version_(\"v@VCS_TAG@\");\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n>> index b2f3a1f324f2..907d2423ed15 100644\n>> --- a/src/qcam/main_window.cpp\n>> +++ b/src/qcam/main_window.cpp\n>> @@ -26,7 +26,7 @@ MainWindow::MainWindow(const OptionsParser::Options &options)\n>>  {\n>>  \tint ret;\n>>  \n>> -\ttitle_ = \"QCam \" + QString::fromStdString(libcamera::version);\n>> +\ttitle_ = \"QCam \" + QString::fromStdString(CameraManager::version());\n> \n> Why do you prefer exposing the version in the CameraManager namespace,\n> rather than libcamera?\n> \n> It's the version of libcamera, the CameraManager is not versioned\n> differently from the rest of the library...\n> \n> Now that you're moving the version (back) to a version.cpp I don't see\n> the need for it to be in the CameraManager namespace. I could have\n> understood the reasoning while it was 'borrowing' the CameraManager C++\n> object file perhaps...\n> \n> \n> Not objecting to the namespace choice, I just haven't understood the\n> rationale for the move.\n> \n> \n>>  \tsetWindowTitle(title_);\n>>  \tconnect(&titleTimer_, SIGNAL(timeout()), this, SLOT(updateTitle()));\n>>  \n>> diff --git a/utils/gen-version.sh b/utils/gen-version.sh\n>> index b3003d7a80d3..1800d71642c0 100755\n>> --- a/utils/gen-version.sh\n>> +++ b/utils/gen-version.sh\n>> @@ -8,6 +8,9 @@ then\n>>  \tcd \"$1\" 2>/dev/null || exit 1\n>>  fi\n>>  \n>> +# Bail out if the directory isn't under git control\n>> +git rev-parse --git-dir >/dev/null 2>&1 || exit 1\n>> +\n> \n> This is where I believe the script should (in the future, or perhaps\n> near future) identify the appropriate libcamera_version file and return\n> that content.\n> \n> The file would be automatically generated when creating a release.\n> \n> \n> I'll submit a patch to do all of this after you've got this one how you\n> want it. I had always planned to do the 'release' stage after the\n> initial versioning.\n> \n> \n> \n>>  # Get a short description from the tree.\n>>  version=$(git describe --abbrev=8 --match \"v[0-9]*\" 2>/dev/null)\n>>  \n>> @@ -16,22 +19,16 @@ then\n>>  \t# Handle an un-tagged repository\n>>  \tsha=$(git describe --abbrev=8 --always 2>/dev/null)\n>>  \tcommits=$(git log --oneline | wc -l 2>/dev/null)\n>> -\tversion=v0.0.$commits.$sha\n>> +\tversion=\"v0.0.0-$commits-g$sha\"\n>>  fi\n>>  \n>> -# Prevent changed timestamps causing -dirty labels\n>> +# Append a '-dirty' suffix is the working tree is dirty. Prevent false\n> \n> s/suffix is/suffix if/\n> \n>> +# positives due to changed timestamps by running git update-index.\n>>  git update-index --refresh > /dev/null 2>&1\n>> -dirty=$(git diff-index --name-only HEAD 2>/dev/null) || dirty=\n>> +git diff-index --quiet HEAD || version=\"$version-dirty\"\n>>  \n>> -# Strip the 'g', and replace the preceeding '-' with a '+' to denote a label\n>> -version=$(echo \"$version\" | sed -e 's/-g/+/g')\n>> -\n>> -# Fix the '-' (the patch count) to a '.' as a version increment.\n>> -version=$(echo \"$version\" | sed -e 's/-/./g')\n>> -\n>> -if [ -n \"$dirty\" ]\n>> -then\n>> -\tversion=$version-dirty\n>> -fi\n>> +# Replace first '-' with a '+' to denote build metadata, strip the 'g' in from\n>> +# of the git SHA1 and remove the initial 'v'.\n>> +version=$(echo \"$version\" | sed -e 's/-/+/' | sed -e 's/-g/-/' | cut -c 2-)\n> \n> At first this was a bit painful to read. It looks like you've removed\n> the '-' and replaced with '+', which would then be used by the second\n> sed, however I see that they only affect the first instance.\n> \n> Not really anything to change though, just me misreading I guess.\n> \n> \n>>  \n>>  echo \"$version\"\n>>\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8B76660BE1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  9 Jul 2019 10:43:42 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D859A56A;\n\tTue,  9 Jul 2019 10:43:41 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1562661822;\n\tbh=VeRMcerDrOZznQyOgXHFKACZUcdvAUD20EF6OrEmAQA=;\n\th=Reply-To:Subject:From:To:References:Date:In-Reply-To:From;\n\tb=nK+ywKj4m3OYzVdwnlPm6mDumheQBNTtGepqJ60AJmHdv9dDqwkuw7H0CwJCax4pS\n\thZyRO/+Etw2SetKHvjYppS6JvGSlAcS3KbJnRpWro02que2ak98Lyj4STTEC3moyeB\n\tGVdRBpSDoQ2zyV2uDqekkRnJQiFXarqt/tjqh/48=","Reply-To":"kieran.bingham@ideasonboard.com","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20190708131212.15275-1-laurent.pinchart@ideasonboard.com>\n\t<5f131a62-6ae7-12a2-4a74-f7b17de0e20d@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<81d3b110-b62f-3840-ad60-927387265226@ideasonboard.com>","Date":"Tue, 9 Jul 2019 09:43:39 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.1","MIME-Version":"1.0","In-Reply-To":"<5f131a62-6ae7-12a2-4a74-f7b17de0e20d@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: Rework automatic\n\tversion generation to avoid rebuilds","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 09 Jul 2019 08:43:42 -0000"}},{"id":2196,"web_url":"https://patchwork.libcamera.org/comment/2196/","msgid":"<20190709084837.GF4819@pendragon.ideasonboard.com>","date":"2019-07-09T08:48:37","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: Rework automatic\n\tversion generation to avoid rebuilds","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, Jul 08, 2019 at 04:05:22PM +0100, Kieran Bingham wrote:\n> On 08/07/2019 14:12, Laurent Pinchart wrote:\n> > Commit b817bcec6b53 (\"libcamera: Auto generate version information\")\n> > generates version information in order to automatically include it\n> > various locations (Sphinx and Doxygen documentation, libcamera::version\n> > variable available at runtime, and version.h available at compile time).\n> > Unfortunately this causes lots of unnecessary rebuilds when modifying\n> > the git tree state, which hinders development.\n> > \n> > The problem is caused by the generated version.h being listed as a\n> > dependency for the whole libcamera. This is required as meson (to the\n> > best of my knowledge) doesn't provide a way to explicitly specify the\n> > dependency of a single object file (camera_manager.o in this case, as\n> > camera_manager.cpp is the only consumer of the generated version string)\n> > on the custom target used to generate version.h. The dependency can't be\n> > automatically detected at build time, like dependencies on normal\n> > headers that are generated by parsing the source, because the version.h\n> > header may not exist yet. The build could then fail in a racy way.\n> > \n> > This change attempts at solving the issue by generating a version.cpp\n> > instead of a version.h to set the git-based version. This minimises the\n> > number of files that need to be rebuild when then git tree state\n> > changes, while retaining the main purpose of the original automatic\n> > version generation, the ability to access the git-based version string\n> > at runtime. We however lose the ability to access git-based version\n> > information at build time in an application building against libcamera,\n> > but there is no expected use case for this.\n> > \n> > On the other hand, major, minor and patch level version numbers are\n> > useful at build time. This commit changes the generation of version.h in\n> > order to add three macros named LIBCAMERA_VERSION_MAJOR,\n> > LIBCAMERA_VERSION_MINOR and LIBCAMERA_VERSION_PATCH for this purpose.\n> > version.h is not included by any other libcamera header or source file,\n> > and thus doesn't force a rebuild of the library.\n> > \n> > The Sphinx and Doxygen documentation keep their git-based version\n> > information, which is set during the configuration of the build and then\n> > doesn't track git commits. We may want to investigate how to improve\n> > this, but given that git-based version for the documentation has very\n> > few use cases outside of tagging nightly builds, this isn't considered\n> > an issue at the moment.\n> > \n> > The documentation install directory now uses the base version string, in\n> > order to avoid increasing the number of documentation directories\n> > needlessly. This shouldn't cause any issue as the API should not change\n> > without a change to the version number.\n> > \n> > The version number generation and handling code now also standardises\n> > the version variables to not start with a 'v' prefix in meson, in order\n> > to simplify their handling. The prefix is added when generating the\n> > relevant files.\n> > \n> > Note that we go back to specifying the fallback version in the main\n> > meson.build, in the call to the project() function. For the time being I\n> > believe this should be a good compromise to avoid unnecessary\n> > recompilation, and moving the fallback version to a different file for\n> > tarball releases can be built on top of this.\n> > \n> > Fixes: b817bcec6b53 (\"libcamera: Auto generate version information\")\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> When I apply your patch, or build from your branch I get the following:\n> \n> \n> In file included from ../src/cam/main.cpp:13:\n> include/libcamera/libcamera.h:11:10: fatal error: libcamera/config.h: No\n> such file or directory\n>  #include <libcamera/config.h>\n> \n> \n> But that seems unrelated???\n> \n> Perhaps it's uncovered some race or lack of dependency from a previous\n> commit.\n> \n> I've removed my entire build directory, and I still get the config.h\n> fault ... hrm.\n> \n> In fact, gen-header.sh seems to be broken for me:\n> \n> cat build/include/libcamera/libcamera.h\n> /* SPDX-License-Identifier: LGPL-2.1-or-later */\n> /* This file is auto-generated, do not edit! */\n> /*\n>  * Copyright (C) 2018-2019, Google Inc.\n>  *\n>  * libcamera.h - libcamera public API\n>  */\n> #ifndef __LIBCAMERA_LIBCAMERA_H__\n> #define __LIBCAMERA_LIBCAMERA_H__\n> \n> #include <libcamera/config.h>\n> #include <libcamera/version.h>\n> \n> #endif /* __LIBCAMERA_LIBCAMERA_H__ */\n> \n> > ---\n> >  Documentation/meson.build          |  6 +++---\n> >  include/libcamera/camera_manager.h |  3 +++\n> >  include/libcamera/gen-header.sh    |  6 +++++-\n> >  include/libcamera/meson.build      | 18 +++++++++++-------\n> >  include/libcamera/version.h.in     | 12 +++---------\n> >  meson.build                        | 19 ++++++++++++++++---\n> >  src/libcamera/camera_manager.cpp   | 14 +++++++-------\n> >  src/libcamera/meson.build          | 10 +++++++++-\n> >  src/libcamera/version.cpp.in       | 16 ++++++++++++++++\n> >  src/qcam/main_window.cpp           |  2 +-\n> >  utils/gen-version.sh               | 23 ++++++++++-------------\n> >  11 files changed, 84 insertions(+), 45 deletions(-)\n> >  create mode 100644 src/libcamera/version.cpp.in\n> > \n> > diff --git a/Documentation/meson.build b/Documentation/meson.build\n> > index c355d5feb504..b1720b05f5ee 100644\n> > --- a/Documentation/meson.build\n> > +++ b/Documentation/meson.build\n> > @@ -1,5 +1,5 @@\n> >  doc_install_dir = join_paths(get_option('datadir'), 'doc',\n> > -                             'libcamera-@0@'.format(meson.project_version()))\n> > +                             'libcamera-@0@'.format(libcamera_version))\n> >  \n> >  #\n> >  # Doxygen\n> > @@ -9,7 +9,7 @@ doxygen = find_program('doxygen', required : false)\n> >  \n> >  if doxygen.found()\n> >      cdata = configuration_data()\n> > -    cdata.set('VERSION', meson.project_version())\n> > +    cdata.set('VERSION', 'v@0@'.format(libcamera_git_version))\n> >      cdata.set('TOP_SRCDIR', meson.source_root())\n> >      cdata.set('TOP_BUILDDIR', meson.build_root())\n> >  \n> > @@ -48,7 +48,7 @@ if sphinx.found()\n> >          'index.rst',\n> >      ]\n> >  \n> > -    release = 'release=' + meson.project_version()\n> > +    release = 'release=v' + libcamera_git_version\n> >  \n> >      custom_target('documentation',\n> >                    command : [sphinx, '-D', release, '-q', '-W', '-b', 'html',\n> > diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> > index cf3a85ae8fe3..633d27d17ebf 100644\n> > --- a/include/libcamera/camera_manager.h\n> > +++ b/include/libcamera/camera_manager.h\n> > @@ -31,6 +31,7 @@ public:\n> >  \tvoid removeCamera(Camera *camera);\n> >  \n> >  \tstatic CameraManager *instance();\n> > +\tstatic const std::string &version() { return version_; }\n> >  \n> >  \tvoid setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher);\n> >  \tEventDispatcher *eventDispatcher();\n> > @@ -46,6 +47,8 @@ private:\n> >  \tstd::vector<std::shared_ptr<Camera>> cameras_;\n> >  \n> >  \tstd::unique_ptr<EventDispatcher> dispatcher_;\n> > +\n> > +\tstatic const std::string version_;\n> >  };\n> >  \n> >  } /* namespace libcamera */\n> > diff --git a/include/libcamera/gen-header.sh b/include/libcamera/gen-header.sh\n> > index e171c08c20b8..ccd3414bd8ca 100755\n> > --- a/include/libcamera/gen-header.sh\n> > +++ b/include/libcamera/gen-header.sh\n> > @@ -16,8 +16,12 @@ cat <<EOF > \"$dst_file\"\n> >  \n> >  EOF\n> >  \n> > -for header in \"$src_dir\"/*.h ; do\n> > +headers=$(for header in $\"$src_dir\"/*.h ; do\n> \n> I wonder if that extra $ is to blame...\n\nIt indeed seems incorrect. I've removed the extra $ and it still works\nfor me, but it didn't break in the first place :-)\n\n> >  \theader=$(basename \"$header\")\n> > +\techo $header\n> > +done ; echo \"version.h\" | sort)\n> > +\n> > +for header in $(echo $headers) ; do\n> >  \techo \"#include <libcamera/$header>\" >> \"$dst_file\"\n> >  done\n> >  \n> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> > index 6f81f1117318..972513fc2df5 100644\n> > --- a/include/libcamera/meson.build\n> > +++ b/include/libcamera/meson.build\n> > @@ -16,13 +16,6 @@ libcamera_api = files([\n> >      'timer.h',\n> >  ])\n> >  \n> > -gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')\n> > -\n> > -version_h = vcs_tag(command : [gen_version, meson.current_source_dir()],\n> > -                    input : 'version.h.in',\n> > -                    output : 'version.h',\n> > -                    fallback : 'v0.0')\n> > -\n> >  gen_header = files('gen-header.sh')\n> >  \n> >  libcamera_h = custom_target('gen-header',\n> > @@ -32,5 +25,16 @@ libcamera_h = custom_target('gen-header',\n> >                              install : true,\n> >                              install_dir : 'include/libcamera')\n> >  \n> > +version = libcamera_version.split('.')\n> > +libcamera_version_config = configuration_data()\n> > +libcamera_version_config.set('LIBCAMERA_VERSION_MAJOR', version[0])\n> > +libcamera_version_config.set('LIBCAMERA_VERSION_MINOR', version[1])\n> > +libcamera_version_config.set('LIBCAMERA_VERSION_PATCH', version[2])\n> > +\n> > +configure_file(input : 'version.h.in',\n> > +               output : 'version.h',\n> > +               configuration : libcamera_version_config,\n> > +               install_dir : 'include/libcamera')\n> > +\n> >  install_headers(libcamera_api,\n> >                  subdir : 'libcamera')\n> > diff --git a/include/libcamera/version.h.in b/include/libcamera/version.h.in\n> > index e49b36962aed..5e9a30911d12 100644\n> > --- a/include/libcamera/version.h.in\n> > +++ b/include/libcamera/version.h.in\n> > @@ -9,14 +9,8 @@\n> >  #ifndef __LIBCAMERA_VERSION_H__\n> >  #define __LIBCAMERA_VERSION_H__\n> >  \n> > -#include <string>\n> > -\n> > -#define LIBCAMERA_VERSION \"@VCS_TAG@\"\n> > -\n> > -namespace libcamera {\n> > -\n> > -extern const std::string version;\n> > -\n> > -} /* namespace libcamera */\n> > +#define LIBCAMERA_VERSION_MAJOR\t\t@LIBCAMERA_VERSION_MAJOR@\n> > +#define LIBCAMERA_VERSION_MINOR\t\t@LIBCAMERA_VERSION_MINOR@\n> > +#define LIBCAMERA_VERSION_PATCH\t\t@LIBCAMERA_VERSION_PATCH@\n> >  \n> >  #endif /* __LIBCAMERA_VERSION_H__ */\n> > diff --git a/meson.build b/meson.build\n> > index 342b3cc76a93..8f3d0ce91cb8 100644\n> > --- a/meson.build\n> > +++ b/meson.build\n> > @@ -1,8 +1,6 @@\n> >  project('libcamera', 'c', 'cpp',\n> >      meson_version : '>= 0.40',\n> > -    version : run_command('utils/gen-version.sh',\n> > -                          '@0@'.format(meson.source_root()),\n> > -                          check : true).stdout().strip(),\n> > +    version : '0.0.0',\n> \n> I dislike not being able to populate the meson.project_version().\n> \n> I still think that fallback should be handled in the gen-version.sh script.\n\nI'm not opposed to that, but I think we should do so on top of this\npatch.\n\n> >      default_options : [\n> >          'werror=true',\n> >          'warning_level=2',\n> > @@ -10,6 +8,21 @@ project('libcamera', 'c', 'cpp',\n> >      ],\n> >      license : 'LGPL 2.1+')\n> >  \n> > +# Generate version information. The libcamera_git_version variable contains the\n> > +# full version with git patch count and SHA1 (e.g. 1.2.3+211-c94a24f4), while\n> > +# the libcamera_version variable contains the major.minor.patch (e.g. 1.2.3)\n> > +# only. If the source tree isn't under git control, or if it matches the last\n> > +# git version tag, the build metadata (e.g. +211-c94a24f4) is omitted from\n> > +# libcamera_git_version.\n> > +libcamera_git_version = run_command('utils/gen-version.sh',\n> > +                                    meson.source_root()).stdout().strip()\n> > +if libcamera_git_version == ''\n> > +    libcamera_git_version = meson.project_version()\n> \n> ...\n> \n> > +endif\n> > +\n> > +libcamera_version = libcamera_git_version.split('+')[0]\n> > +\n> > +# Configure the build environment.\n> >  cc = meson.get_compiler('c')\n> >  config_h = configuration_data()\n> >  \n> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > index c5da46b4062d..337496c21cfc 100644\n> > --- a/src/libcamera/camera_manager.cpp\n> > +++ b/src/libcamera/camera_manager.cpp\n> > @@ -9,7 +9,6 @@\n> >  \n> >  #include <libcamera/camera.h>\n> >  #include <libcamera/event_dispatcher.h>\n> > -#include <libcamera/version.h>\n> >  \n> >  #include \"device_enumerator.h\"\n> >  #include \"event_dispatcher_poll.h\"\n> > @@ -26,11 +25,6 @@ namespace libcamera {\n> >  \n> >  LOG_DEFINE_CATEGORY(Camera)\n> >  \n> > -/**\n> > - * \\brief The library global version string\n> > - */\n> > -const std::string version(LIBCAMERA_VERSION);\n> > -\n> >  /**\n> >   * \\class CameraManager\n> >   * \\brief Provide access and manage all cameras in the system\n> > @@ -85,7 +79,7 @@ int CameraManager::start()\n> >  \tif (enumerator_)\n> >  \t\treturn -EBUSY;\n> >  \n> > -\tLOG(Camera, Info) << \"libcamera \" << version;\n> > +\tLOG(Camera, Info) << \"libcamera \" << version_;\n> >  \n> >  \tenumerator_ = DeviceEnumerator::create();\n> >  \tif (!enumerator_ || enumerator_->enumerate())\n> > @@ -232,6 +226,12 @@ CameraManager *CameraManager::instance()\n> >  \treturn &manager;\n> >  }\n> >  \n> > +/**\n> > + * \\fn const std::string &CameraManager::version()\n> > + * \\brief Retrieve the libcamera version string\n> > + * \\return The libcamera version string\n> > + */\n> > +\n> >  /**\n> >   * \\brief Set the event dispatcher\n> >   * \\param[in] dispatcher Pointer to the event dispatcher\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 336f4f066fac..97ff86e2167f 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -79,8 +79,16 @@ control_types_cpp = custom_target('control_types_cpp',\n> >  \n> >  libcamera_sources += control_types_cpp\n> >  \n> > +gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')\n> > +\n> > +version_cpp = vcs_tag(command : [gen_version, meson.source_root()],\n> > +                      input : 'version.cpp.in',\n> > +                      output : 'version.cpp',\n> > +                      fallback : meson.project_version())\n> \n> No point specifying the fallback if that's the default fallback?\n\nThe default fallback was introduced in meson > 0.40.\n\n> > +\n> > +libcamera_sources += version_cpp\n> > +\n> >  libcamera_deps = [\n> > -    declare_dependency(sources : version_h),>      cc.find_library('dl'),\n> >      libudev,\n> >  ]\n> > diff --git a/src/libcamera/version.cpp.in b/src/libcamera/version.cpp.in\n> > new file mode 100644\n> > index 000000000000..5aec08a189f3\n> > --- /dev/null\n> > +++ b/src/libcamera/version.cpp.in\n> > @@ -0,0 +1,16 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * version.cpp - libcamera version\n> > + *\n> > + * This file is auto-generated. Do not edit.\n> > + */\n> > +\n> > +#include <libcamera/camera_manager.h>\n> > +\n> > +namespace libcamera {\n> > +\n> > +const std::string CameraManager::version_(\"v@VCS_TAG@\");\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> > index b2f3a1f324f2..907d2423ed15 100644\n> > --- a/src/qcam/main_window.cpp\n> > +++ b/src/qcam/main_window.cpp\n> > @@ -26,7 +26,7 @@ MainWindow::MainWindow(const OptionsParser::Options &options)\n> >  {\n> >  \tint ret;\n> >  \n> > -\ttitle_ = \"QCam \" + QString::fromStdString(libcamera::version);\n> > +\ttitle_ = \"QCam \" + QString::fromStdString(CameraManager::version());\n> \n> Why do you prefer exposing the version in the CameraManager namespace,\n> rather than libcamera?\n>\n> It's the version of libcamera, the CameraManager is not versioned\n> differently from the rest of the library...\n> \n> Now that you're moving the version (back) to a version.cpp I don't see\n> the need for it to be in the CameraManager namespace. I could have\n> understood the reasoning while it was 'borrowing' the CameraManager C++\n> object file perhaps...\n> \n> Not objecting to the namespace choice, I just haven't understood the\n> rationale for the move.\n\nYou're right, I'll change that back. I think I made the change in an\nintermediate refactoring step, and it's not needed anymore.\n\n> >  \tsetWindowTitle(title_);\n> >  \tconnect(&titleTimer_, SIGNAL(timeout()), this, SLOT(updateTitle()));\n> >  \n> > diff --git a/utils/gen-version.sh b/utils/gen-version.sh\n> > index b3003d7a80d3..1800d71642c0 100755\n> > --- a/utils/gen-version.sh\n> > +++ b/utils/gen-version.sh\n> > @@ -8,6 +8,9 @@ then\n> >  \tcd \"$1\" 2>/dev/null || exit 1\n> >  fi\n> >  \n> > +# Bail out if the directory isn't under git control\n> > +git rev-parse --git-dir >/dev/null 2>&1 || exit 1\n> > +\n> \n> This is where I believe the script should (in the future, or perhaps\n> near future) identify the appropriate libcamera_version file and return\n> that content.\n> \n> The file would be automatically generated when creating a release.\n> \n> I'll submit a patch to do all of this after you've got this one how you\n> want it. I had always planned to do the 'release' stage after the\n> initial versioning.\n\nSure. I think it will be a small change if applied on top of this one.\n\n> >  # Get a short description from the tree.\n> >  version=$(git describe --abbrev=8 --match \"v[0-9]*\" 2>/dev/null)\n> >  \n> > @@ -16,22 +19,16 @@ then\n> >  \t# Handle an un-tagged repository\n> >  \tsha=$(git describe --abbrev=8 --always 2>/dev/null)\n> >  \tcommits=$(git log --oneline | wc -l 2>/dev/null)\n> > -\tversion=v0.0.$commits.$sha\n> > +\tversion=\"v0.0.0-$commits-g$sha\"\n> >  fi\n> >  \n> > -# Prevent changed timestamps causing -dirty labels\n> > +# Append a '-dirty' suffix is the working tree is dirty. Prevent false\n> \n> s/suffix is/suffix if/\n> \n> > +# positives due to changed timestamps by running git update-index.\n> >  git update-index --refresh > /dev/null 2>&1\n> > -dirty=$(git diff-index --name-only HEAD 2>/dev/null) || dirty=\n> > +git diff-index --quiet HEAD || version=\"$version-dirty\"\n> >  \n> > -# Strip the 'g', and replace the preceeding '-' with a '+' to denote a label\n> > -version=$(echo \"$version\" | sed -e 's/-g/+/g')\n> > -\n> > -# Fix the '-' (the patch count) to a '.' as a version increment.\n> > -version=$(echo \"$version\" | sed -e 's/-/./g')\n> > -\n> > -if [ -n \"$dirty\" ]\n> > -then\n> > -\tversion=$version-dirty\n> > -fi\n> > +# Replace first '-' with a '+' to denote build metadata, strip the 'g' in from\n> > +# of the git SHA1 and remove the initial 'v'.\n> > +version=$(echo \"$version\" | sed -e 's/-/+/' | sed -e 's/-g/-/' | cut -c 2-)\n> \n> At first this was a bit painful to read. It looks like you've removed\n> the '-' and replaced with '+', which would then be used by the second\n> sed, however I see that they only affect the first instance.\n> \n> Not really anything to change though, just me misreading I guess.\n> \n> >  \n> >  echo \"$version\"\n> >","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E029E60BE1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  9 Jul 2019 10:49:04 +0200 (CEST)","from pendragon.ideasonboard.com (softbank126163157105.bbtec.net\n\t[126.163.157.105])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3CBFE56A;\n\tTue,  9 Jul 2019 10:49:02 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1562662144;\n\tbh=iX8Zkl8/GYPxx38of1oGmDeHerulB/obN7Wg+jhACzA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rp+rjdNPGO/jKlLP+MkQEeaEZF4HxYRIQrw+REO7KAqw2kESfJ8KTs1XkUGl8cPTC\n\ta8Ib2vMb38M2QD2z8o+KJ/jIwj3yLGHQX34g74Q81HSzyjiXh8SdZdkc/bfuD5prod\n\t9IdgXyWGXuCA2PFamQLQOijcyPvIOBDyBbw2lZH4=","Date":"Tue, 9 Jul 2019 11:48:37 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190709084837.GF4819@pendragon.ideasonboard.com>","References":"<20190708131212.15275-1-laurent.pinchart@ideasonboard.com>\n\t<5f131a62-6ae7-12a2-4a74-f7b17de0e20d@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<5f131a62-6ae7-12a2-4a74-f7b17de0e20d@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: Rework automatic\n\tversion generation to avoid rebuilds","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 09 Jul 2019 08:49:05 -0000"}},{"id":2197,"web_url":"https://patchwork.libcamera.org/comment/2197/","msgid":"<20190709091154.GG4819@pendragon.ideasonboard.com>","date":"2019-07-09T09:11:54","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: Rework automatic\n\tversion generation to avoid rebuilds","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tue, Jul 09, 2019 at 09:43:39AM +0100, Kieran Bingham wrote:\n> Hi Laurent,\n> \n> Please also note the following shell-check warnings:\n> \n> > shellcheck ./include/libcamera/gen-header.sh \n> > \n> > In ./include/libcamera/gen-header.sh line 19:\n> > headers=$(for header in $\"$src_dir\"/*.h ; do\n> >                         ^-- SC2039: In POSIX sh, $\"..\" is undefined.\n> > \n> > \n> > In ./include/libcamera/gen-header.sh line 21:\n> > \techo $header\n> >              ^-- SC2086: Double quote to prevent globbing and word splitting.\n\n> > In ./include/libcamera/gen-header.sh line 24:\n> > for header in $(echo $headers) ; do\n> >               ^-- SC2116: Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'.\n> >                      ^-- SC2086: Double quote to prevent globbing and word splitting.\n\nThis was on purpose to fix an issue I experienced with globbing and word\nsplitting, but it turns out I must have fixed something and it's not\nneeded anymore. I'll happily fix this, thanks :-)\n\n> On 08/07/2019 16:05, Kieran Bingham wrote:\n> > On 08/07/2019 14:12, Laurent Pinchart wrote:\n> >> Commit b817bcec6b53 (\"libcamera: Auto generate version information\")\n> >> generates version information in order to automatically include it\n> >> various locations (Sphinx and Doxygen documentation, libcamera::version\n> >> variable available at runtime, and version.h available at compile time).\n> >> Unfortunately this causes lots of unnecessary rebuilds when modifying\n> >> the git tree state, which hinders development.\n> >>\n> >> The problem is caused by the generated version.h being listed as a\n> >> dependency for the whole libcamera. This is required as meson (to the\n> >> best of my knowledge) doesn't provide a way to explicitly specify the\n> >> dependency of a single object file (camera_manager.o in this case, as\n> >> camera_manager.cpp is the only consumer of the generated version string)\n> >> on the custom target used to generate version.h. The dependency can't be\n> >> automatically detected at build time, like dependencies on normal\n> >> headers that are generated by parsing the source, because the version.h\n> >> header may not exist yet. The build could then fail in a racy way.\n> >>\n> >> This change attempts at solving the issue by generating a version.cpp\n> >> instead of a version.h to set the git-based version. This minimises the\n> >> number of files that need to be rebuild when then git tree state\n> >> changes, while retaining the main purpose of the original automatic\n> >> version generation, the ability to access the git-based version string\n> >> at runtime. We however lose the ability to access git-based version\n> >> information at build time in an application building against libcamera,\n> >> but there is no expected use case for this.\n> >>\n> >> On the other hand, major, minor and patch level version numbers are\n> >> useful at build time. This commit changes the generation of version.h in\n> >> order to add three macros named LIBCAMERA_VERSION_MAJOR,\n> >> LIBCAMERA_VERSION_MINOR and LIBCAMERA_VERSION_PATCH for this purpose.\n> >> version.h is not included by any other libcamera header or source file,\n> >> and thus doesn't force a rebuild of the library.\n> >>\n> >> The Sphinx and Doxygen documentation keep their git-based version\n> >> information, which is set during the configuration of the build and then\n> >> doesn't track git commits. We may want to investigate how to improve\n> >> this, but given that git-based version for the documentation has very\n> >> few use cases outside of tagging nightly builds, this isn't considered\n> >> an issue at the moment.\n> >>\n> >> The documentation install directory now uses the base version string, in\n> >> order to avoid increasing the number of documentation directories\n> >> needlessly. This shouldn't cause any issue as the API should not change\n> >> without a change to the version number.\n> >>\n> >> The version number generation and handling code now also standardises\n> >> the version variables to not start with a 'v' prefix in meson, in order\n> >> to simplify their handling. The prefix is added when generating the\n> >> relevant files.\n> >>\n> >> Note that we go back to specifying the fallback version in the main\n> >> meson.build, in the call to the project() function. For the time being I\n> >> believe this should be a good compromise to avoid unnecessary\n> >> recompilation, and moving the fallback version to a different file for\n> >> tarball releases can be built on top of this.\n> >>\n> >> Fixes: b817bcec6b53 (\"libcamera: Auto generate version information\")\n> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > \n> > When I apply your patch, or build from your branch I get the following:\n> > \n> > \n> > In file included from ../src/cam/main.cpp:13:\n> > include/libcamera/libcamera.h:11:10: fatal error: libcamera/config.h: No\n> > such file or directory\n> >  #include <libcamera/config.h>\n> > \n> > \n> > But that seems unrelated???\n> > \n> > Perhaps it's uncovered some race or lack of dependency from a previous\n> > commit.\n> > \n> > I've removed my entire build directory, and I still get the config.h\n> > fault ... hrm.\n> > \n> > In fact, gen-header.sh seems to be broken for me:\n> > \n> > cat build/include/libcamera/libcamera.h\n> > /* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > /* This file is auto-generated, do not edit! */\n> > /*\n> >  * Copyright (C) 2018-2019, Google Inc.\n> >  *\n> >  * libcamera.h - libcamera public API\n> >  */\n> > #ifndef __LIBCAMERA_LIBCAMERA_H__\n> > #define __LIBCAMERA_LIBCAMERA_H__\n> > \n> > #include <libcamera/config.h>\n> > #include <libcamera/version.h>\n> > \n> > #endif /* __LIBCAMERA_LIBCAMERA_H__ */\n> > \n> > \n> > \n> >> ---\n> >>  Documentation/meson.build          |  6 +++---\n> >>  include/libcamera/camera_manager.h |  3 +++\n> >>  include/libcamera/gen-header.sh    |  6 +++++-\n> >>  include/libcamera/meson.build      | 18 +++++++++++-------\n> >>  include/libcamera/version.h.in     | 12 +++---------\n> >>  meson.build                        | 19 ++++++++++++++++---\n> >>  src/libcamera/camera_manager.cpp   | 14 +++++++-------\n> >>  src/libcamera/meson.build          | 10 +++++++++-\n> >>  src/libcamera/version.cpp.in       | 16 ++++++++++++++++\n> >>  src/qcam/main_window.cpp           |  2 +-\n> >>  utils/gen-version.sh               | 23 ++++++++++-------------\n> >>  11 files changed, 84 insertions(+), 45 deletions(-)\n> >>  create mode 100644 src/libcamera/version.cpp.in\n> >>\n> >> diff --git a/Documentation/meson.build b/Documentation/meson.build\n> >> index c355d5feb504..b1720b05f5ee 100644\n> >> --- a/Documentation/meson.build\n> >> +++ b/Documentation/meson.build\n> >> @@ -1,5 +1,5 @@\n> >>  doc_install_dir = join_paths(get_option('datadir'), 'doc',\n> >> -                             'libcamera-@0@'.format(meson.project_version()))\n> >> +                             'libcamera-@0@'.format(libcamera_version))\n> >>  \n> >>  #\n> >>  # Doxygen\n> >> @@ -9,7 +9,7 @@ doxygen = find_program('doxygen', required : false)\n> >>  \n> >>  if doxygen.found()\n> >>      cdata = configuration_data()\n> >> -    cdata.set('VERSION', meson.project_version())\n> >> +    cdata.set('VERSION', 'v@0@'.format(libcamera_git_version))\n> >>      cdata.set('TOP_SRCDIR', meson.source_root())\n> >>      cdata.set('TOP_BUILDDIR', meson.build_root())\n> >>  \n> >> @@ -48,7 +48,7 @@ if sphinx.found()\n> >>          'index.rst',\n> >>      ]\n> >>  \n> >> -    release = 'release=' + meson.project_version()\n> >> +    release = 'release=v' + libcamera_git_version\n> >>  \n> >>      custom_target('documentation',\n> >>                    command : [sphinx, '-D', release, '-q', '-W', '-b', 'html',\n> >> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> >> index cf3a85ae8fe3..633d27d17ebf 100644\n> >> --- a/include/libcamera/camera_manager.h\n> >> +++ b/include/libcamera/camera_manager.h\n> >> @@ -31,6 +31,7 @@ public:\n> >>  \tvoid removeCamera(Camera *camera);\n> >>  \n> >>  \tstatic CameraManager *instance();\n> >> +\tstatic const std::string &version() { return version_; }\n> >>  \n> >>  \tvoid setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher);\n> >>  \tEventDispatcher *eventDispatcher();\n> >> @@ -46,6 +47,8 @@ private:\n> >>  \tstd::vector<std::shared_ptr<Camera>> cameras_;\n> >>  \n> >>  \tstd::unique_ptr<EventDispatcher> dispatcher_;\n> >> +\n> >> +\tstatic const std::string version_;\n> >>  };\n> >>  \n> >>  } /* namespace libcamera */\n> >> diff --git a/include/libcamera/gen-header.sh b/include/libcamera/gen-header.sh\n> >> index e171c08c20b8..ccd3414bd8ca 100755\n> >> --- a/include/libcamera/gen-header.sh\n> >> +++ b/include/libcamera/gen-header.sh\n> >> @@ -16,8 +16,12 @@ cat <<EOF > \"$dst_file\"\n> >>  \n> >>  EOF\n> >>  \n> >> -for header in \"$src_dir\"/*.h ; do\n> >> +headers=$(for header in $\"$src_dir\"/*.h ; do\n> > \n> > I wonder if that extra $ is to blame...\n> > \n> > \n> >>  \theader=$(basename \"$header\")\n> >> +\techo $header\n> >> +done ; echo \"version.h\" | sort)\n> >> +\n> >> +for header in $(echo $headers) ; do\n> >>  \techo \"#include <libcamera/$header>\" >> \"$dst_file\"\n> >>  done\n> >>  \n> >> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> >> index 6f81f1117318..972513fc2df5 100644\n> >> --- a/include/libcamera/meson.build\n> >> +++ b/include/libcamera/meson.build\n> >> @@ -16,13 +16,6 @@ libcamera_api = files([\n> >>      'timer.h',\n> >>  ])\n> >>  \n> >> -gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')\n> >> -\n> >> -version_h = vcs_tag(command : [gen_version, meson.current_source_dir()],\n> >> -                    input : 'version.h.in',\n> >> -                    output : 'version.h',\n> >> -                    fallback : 'v0.0')\n> >> -\n> >>  gen_header = files('gen-header.sh')\n> >>  \n> >>  libcamera_h = custom_target('gen-header',\n> >> @@ -32,5 +25,16 @@ libcamera_h = custom_target('gen-header',\n> >>                              install : true,\n> >>                              install_dir : 'include/libcamera')\n> >>  \n> >> +version = libcamera_version.split('.')\n> >> +libcamera_version_config = configuration_data()\n> >> +libcamera_version_config.set('LIBCAMERA_VERSION_MAJOR', version[0])\n> >> +libcamera_version_config.set('LIBCAMERA_VERSION_MINOR', version[1])\n> >> +libcamera_version_config.set('LIBCAMERA_VERSION_PATCH', version[2])\n> >> +\n> >> +configure_file(input : 'version.h.in',\n> >> +               output : 'version.h',\n> >> +               configuration : libcamera_version_config,\n> >> +               install_dir : 'include/libcamera')\n> >> +\n> >>  install_headers(libcamera_api,\n> >>                  subdir : 'libcamera')\n> >> diff --git a/include/libcamera/version.h.in b/include/libcamera/version.h.in\n> >> index e49b36962aed..5e9a30911d12 100644\n> >> --- a/include/libcamera/version.h.in\n> >> +++ b/include/libcamera/version.h.in\n> >> @@ -9,14 +9,8 @@\n> >>  #ifndef __LIBCAMERA_VERSION_H__\n> >>  #define __LIBCAMERA_VERSION_H__\n> >>  \n> >> -#include <string>\n> >> -\n> >> -#define LIBCAMERA_VERSION \"@VCS_TAG@\"\n> >> -\n> >> -namespace libcamera {\n> >> -\n> >> -extern const std::string version;\n> >> -\n> >> -} /* namespace libcamera */\n> >> +#define LIBCAMERA_VERSION_MAJOR\t\t@LIBCAMERA_VERSION_MAJOR@\n> >> +#define LIBCAMERA_VERSION_MINOR\t\t@LIBCAMERA_VERSION_MINOR@\n> >> +#define LIBCAMERA_VERSION_PATCH\t\t@LIBCAMERA_VERSION_PATCH@\n> >>  \n> >>  #endif /* __LIBCAMERA_VERSION_H__ */\n> >> diff --git a/meson.build b/meson.build\n> >> index 342b3cc76a93..8f3d0ce91cb8 100644\n> >> --- a/meson.build\n> >> +++ b/meson.build\n> >> @@ -1,8 +1,6 @@\n> >>  project('libcamera', 'c', 'cpp',\n> >>      meson_version : '>= 0.40',\n> >> -    version : run_command('utils/gen-version.sh',\n> >> -                          '@0@'.format(meson.source_root()),\n> >> -                          check : true).stdout().strip(),\n> >> +    version : '0.0.0',\n> > \n> > I dislike not being able to populate the meson.project_version().\n> > \n> > I still think that fallback should be handled in the gen-version.sh script.\n> > \n> > \n> > \n> > \n> >>      default_options : [\n> >>          'werror=true',\n> >>          'warning_level=2',\n> >> @@ -10,6 +8,21 @@ project('libcamera', 'c', 'cpp',\n> >>      ],\n> >>      license : 'LGPL 2.1+')\n> >>  \n> >> +# Generate version information. The libcamera_git_version variable contains the\n> >> +# full version with git patch count and SHA1 (e.g. 1.2.3+211-c94a24f4), while\n> >> +# the libcamera_version variable contains the major.minor.patch (e.g. 1.2.3)\n> >> +# only. If the source tree isn't under git control, or if it matches the last\n> >> +# git version tag, the build metadata (e.g. +211-c94a24f4) is omitted from\n> >> +# libcamera_git_version.\n> >> +libcamera_git_version = run_command('utils/gen-version.sh',\n> >> +                                    meson.source_root()).stdout().strip()\n> >> +if libcamera_git_version == ''\n> >> +    libcamera_git_version = meson.project_version()\n> > \n> > ...\n> > \n> >> +endif\n> >> +\n> >> +libcamera_version = libcamera_git_version.split('+')[0]\n> >> +\n> >> +# Configure the build environment.\n> >>  cc = meson.get_compiler('c')\n> >>  config_h = configuration_data()\n> >>  \n> >> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> >> index c5da46b4062d..337496c21cfc 100644\n> >> --- a/src/libcamera/camera_manager.cpp\n> >> +++ b/src/libcamera/camera_manager.cpp\n> >> @@ -9,7 +9,6 @@\n> >>  \n> >>  #include <libcamera/camera.h>\n> >>  #include <libcamera/event_dispatcher.h>\n> >> -#include <libcamera/version.h>\n> >>  \n> >>  #include \"device_enumerator.h\"\n> >>  #include \"event_dispatcher_poll.h\"\n> >> @@ -26,11 +25,6 @@ namespace libcamera {\n> >>  \n> >>  LOG_DEFINE_CATEGORY(Camera)\n> >>  \n> >> -/**\n> >> - * \\brief The library global version string\n> >> - */\n> >> -const std::string version(LIBCAMERA_VERSION);\n> >> -\n> >>  /**\n> >>   * \\class CameraManager\n> >>   * \\brief Provide access and manage all cameras in the system\n> >> @@ -85,7 +79,7 @@ int CameraManager::start()\n> >>  \tif (enumerator_)\n> >>  \t\treturn -EBUSY;\n> >>  \n> >> -\tLOG(Camera, Info) << \"libcamera \" << version;\n> >> +\tLOG(Camera, Info) << \"libcamera \" << version_;\n> >>  \n> >>  \tenumerator_ = DeviceEnumerator::create();\n> >>  \tif (!enumerator_ || enumerator_->enumerate())\n> >> @@ -232,6 +226,12 @@ CameraManager *CameraManager::instance()\n> >>  \treturn &manager;\n> >>  }\n> >>  \n> >> +/**\n> >> + * \\fn const std::string &CameraManager::version()\n> >> + * \\brief Retrieve the libcamera version string\n> >> + * \\return The libcamera version string\n> >> + */\n> >> +\n> >>  /**\n> >>   * \\brief Set the event dispatcher\n> >>   * \\param[in] dispatcher Pointer to the event dispatcher\n> >> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> >> index 336f4f066fac..97ff86e2167f 100644\n> >> --- a/src/libcamera/meson.build\n> >> +++ b/src/libcamera/meson.build\n> >> @@ -79,8 +79,16 @@ control_types_cpp = custom_target('control_types_cpp',\n> >>  \n> >>  libcamera_sources += control_types_cpp\n> >>  \n> >> +gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')\n> >> +\n> >> +version_cpp = vcs_tag(command : [gen_version, meson.source_root()],\n> >> +                      input : 'version.cpp.in',\n> >> +                      output : 'version.cpp',\n> >> +                      fallback : meson.project_version())\n> > \n> > No point specifying the fallback if that's the default fallback?\n> > \n> >> +\n> >> +libcamera_sources += version_cpp\n> >> +\n> >>  libcamera_deps = [\n> >> -    declare_dependency(sources : version_h),>      cc.find_library('dl'),\n> >>      libudev,\n> >>  ]\n> >> diff --git a/src/libcamera/version.cpp.in b/src/libcamera/version.cpp.in\n> >> new file mode 100644\n> >> index 000000000000..5aec08a189f3\n> >> --- /dev/null\n> >> +++ b/src/libcamera/version.cpp.in\n> >> @@ -0,0 +1,16 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2019, Google Inc.\n> >> + *\n> >> + * version.cpp - libcamera version\n> >> + *\n> >> + * This file is auto-generated. Do not edit.\n> >> + */\n> >> +\n> >> +#include <libcamera/camera_manager.h>\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +const std::string CameraManager::version_(\"v@VCS_TAG@\");\n> >> +\n> >> +} /* namespace libcamera */\n> >> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> >> index b2f3a1f324f2..907d2423ed15 100644\n> >> --- a/src/qcam/main_window.cpp\n> >> +++ b/src/qcam/main_window.cpp\n> >> @@ -26,7 +26,7 @@ MainWindow::MainWindow(const OptionsParser::Options &options)\n> >>  {\n> >>  \tint ret;\n> >>  \n> >> -\ttitle_ = \"QCam \" + QString::fromStdString(libcamera::version);\n> >> +\ttitle_ = \"QCam \" + QString::fromStdString(CameraManager::version());\n> > \n> > Why do you prefer exposing the version in the CameraManager namespace,\n> > rather than libcamera?\n> > \n> > It's the version of libcamera, the CameraManager is not versioned\n> > differently from the rest of the library...\n> > \n> > Now that you're moving the version (back) to a version.cpp I don't see\n> > the need for it to be in the CameraManager namespace. I could have\n> > understood the reasoning while it was 'borrowing' the CameraManager C++\n> > object file perhaps...\n> > \n> > \n> > Not objecting to the namespace choice, I just haven't understood the\n> > rationale for the move.\n> > \n> > \n> >>  \tsetWindowTitle(title_);\n> >>  \tconnect(&titleTimer_, SIGNAL(timeout()), this, SLOT(updateTitle()));\n> >>  \n> >> diff --git a/utils/gen-version.sh b/utils/gen-version.sh\n> >> index b3003d7a80d3..1800d71642c0 100755\n> >> --- a/utils/gen-version.sh\n> >> +++ b/utils/gen-version.sh\n> >> @@ -8,6 +8,9 @@ then\n> >>  \tcd \"$1\" 2>/dev/null || exit 1\n> >>  fi\n> >>  \n> >> +# Bail out if the directory isn't under git control\n> >> +git rev-parse --git-dir >/dev/null 2>&1 || exit 1\n> >> +\n> > \n> > This is where I believe the script should (in the future, or perhaps\n> > near future) identify the appropriate libcamera_version file and return\n> > that content.\n> > \n> > The file would be automatically generated when creating a release.\n> > \n> > \n> > I'll submit a patch to do all of this after you've got this one how you\n> > want it. I had always planned to do the 'release' stage after the\n> > initial versioning.\n> > \n> > \n> > \n> >>  # Get a short description from the tree.\n> >>  version=$(git describe --abbrev=8 --match \"v[0-9]*\" 2>/dev/null)\n> >>  \n> >> @@ -16,22 +19,16 @@ then\n> >>  \t# Handle an un-tagged repository\n> >>  \tsha=$(git describe --abbrev=8 --always 2>/dev/null)\n> >>  \tcommits=$(git log --oneline | wc -l 2>/dev/null)\n> >> -\tversion=v0.0.$commits.$sha\n> >> +\tversion=\"v0.0.0-$commits-g$sha\"\n> >>  fi\n> >>  \n> >> -# Prevent changed timestamps causing -dirty labels\n> >> +# Append a '-dirty' suffix is the working tree is dirty. Prevent false\n> > \n> > s/suffix is/suffix if/\n> > \n> >> +# positives due to changed timestamps by running git update-index.\n> >>  git update-index --refresh > /dev/null 2>&1\n> >> -dirty=$(git diff-index --name-only HEAD 2>/dev/null) || dirty=\n> >> +git diff-index --quiet HEAD || version=\"$version-dirty\"\n> >>  \n> >> -# Strip the 'g', and replace the preceeding '-' with a '+' to denote a label\n> >> -version=$(echo \"$version\" | sed -e 's/-g/+/g')\n> >> -\n> >> -# Fix the '-' (the patch count) to a '.' as a version increment.\n> >> -version=$(echo \"$version\" | sed -e 's/-/./g')\n> >> -\n> >> -if [ -n \"$dirty\" ]\n> >> -then\n> >> -\tversion=$version-dirty\n> >> -fi\n> >> +# Replace first '-' with a '+' to denote build metadata, strip the 'g' in from\n> >> +# of the git SHA1 and remove the initial 'v'.\n> >> +version=$(echo \"$version\" | sed -e 's/-/+/' | sed -e 's/-g/-/' | cut -c 2-)\n> > \n> > At first this was a bit painful to read. It looks like you've removed\n> > the '-' and replaced with '+', which would then be used by the second\n> > sed, however I see that they only affect the first instance.\n> > \n> > Not really anything to change though, just me misreading I guess.\n> > \n> >>  \n> >>  echo \"$version\"\n> >>","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9F89060BE1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  9 Jul 2019 11:12:22 +0200 (CEST)","from pendragon.ideasonboard.com (softbank126163157105.bbtec.net\n\t[126.163.157.105])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DE5D856A;\n\tTue,  9 Jul 2019 11:12:20 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1562663542;\n\tbh=e62EXYaosgqVi3Up1sVDrp+g5jIbwJ6VfAPnimGXOOo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IGTlTF6zXFfFpha0ww90A2IfuPq3yHjzXF7QRuC4JsNEXFOLdxMNTArkRn4Q80LXu\n\tY/zJAYkM7axr/rU1VdRwUQ1/9uiLj+dVJ8JhIa2gJ1O8c4lB+t7Ab72H68OT/TMHvo\n\tKbNOIPjYA65Vl6aho0Rz3VstH7odB5KK5RxWuPWk=","Date":"Tue, 9 Jul 2019 12:11:54 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190709091154.GG4819@pendragon.ideasonboard.com>","References":"<20190708131212.15275-1-laurent.pinchart@ideasonboard.com>\n\t<5f131a62-6ae7-12a2-4a74-f7b17de0e20d@ideasonboard.com>\n\t<81d3b110-b62f-3840-ad60-927387265226@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<81d3b110-b62f-3840-ad60-927387265226@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: Rework automatic\n\tversion generation to avoid rebuilds","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 09 Jul 2019 09:12:22 -0000"}}]