[{"id":20183,"web_url":"https://patchwork.libcamera.org/comment/20183/","msgid":"","date":"2021-10-14T00:46:38","subject":"Re: [libcamera-devel] [PATCH v2 1/1] build: Preserve upstream git\n\tversioning using meson dist","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\n(CC'ing Kieran)\n\nThank you for the patch.\n\nOn Wed, Oct 13, 2021 at 01:23:12PM +0100, Naushir Patuck wrote:\n> When distributions build and package libcamera libraries, they may not\n> necessarily run the build in the upstream source tree. In these cases, the git\n> SHA1 versioning information will be lost.\n> \n> This change addresses that problem by requiring package managers to run\n> 'meson dist' to create a tarball of the source files and build from there.\n> On runing 'meson dist', the utils/run-dist.sh script will create a version.gen\n> file in the release tarball with the version string generated from the existing\n> utils/gen-version.sh script.\n> \n> The utils/gen-version.sh script has been updated to check for the presence of\n> this version.gen file and read the version string from it instead of creating\n> one.\n\nI came across\nhttps://sources.debian.org/src/libsmpp34/1.14.0-2/git-version-gen/ which\nseems to store the version in a file named .tarball-version. Should we\nfollow the same convention ?\n\n> Signed-off-by: Naushir Patuck \n> ---\n> meson.build | 3 +++\n> src/libcamera/meson.build | 11 +++++------\n> utils/gen-version.sh | 9 +++++++++\n> utils/run-dist.sh | 11 +++++++++++\n> 4 files changed, 28 insertions(+), 6 deletions(-)\n> create mode 100644 utils/run-dist.sh\n> \n> diff --git a/meson.build b/meson.build\n> index a49c484fe64e..85ca0013733e 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -24,6 +24,9 @@ endif\n> \n> libcamera_version = libcamera_git_version.split('+')[0]\n> \n> +# This script gererates the version.gen file on a 'meson dist' command.\n> +meson.add_dist_script('utils/run-dist.sh')\n> +\n> # Configure the build environment.\n> cc = meson.get_compiler('c')\n> cxx = meson.get_compiler('cpp')\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 243dd3c180eb..360eaf80ecf1 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -93,12 +93,11 @@ endforeach\n> \n> libcamera_sources += control_sources\n> \n> -gen_version = meson.source_root() / 'utils' / 'gen-version.sh'\n> -\n> -version_cpp = vcs_tag(command : [gen_version, meson.build_root()],\n> - input : 'version.cpp.in',\n> - output : 'version.cpp',\n> - fallback : meson.project_version())\n> +version_data = configuration_data()\n> +version_data.set('VCS_TAG', libcamera_git_version)\n> +version_cpp = configure_file(input : 'version.cpp.in',\n> + output : 'version.cpp',\n> + configuration : version_data)\n\nThis doesn't seem strictly needed, but it avoids running gen-version.sh\na second time, which is nice. I however have a nagging feeling that\nthere was a reason why we used vcs_tag and not configure_file, but I\ncan't recall it. We'll find out if it causes issues :-)\n\n> \n> libcamera_sources += version_cpp\n> \n> diff --git a/utils/gen-version.sh b/utils/gen-version.sh\n> index b09ad495f86a..09cede84c25e 100755\n> --- a/utils/gen-version.sh\n> +++ b/utils/gen-version.sh\n> @@ -5,6 +5,15 @@\n> \n> build_dir=\"$1\"\n> \n> +# If version.gen exists, output the version string from the file and exit.\n> +# This file is auto-generated on a 'meson dist' command from the run-dist.sh\n> +# script.\n> +if [ -f \"${MESON_SOURCE_ROOT}\"/version.gen ]\n> +then\n> +\tcat \"${MESON_SOURCE_ROOT}\"/version.gen\n> +\texit 0\n> +fi\n> +\n\nShouldn't we do this only if we're not under git control ? There should\nbe no version.gen file in that case of course, but isn't still a better\ndefault ?\n\n> # Bail out if the directory isn't under git control\n> src_dir=$(git rev-parse --git-dir 2>&1) || exit 1\n> src_dir=$(readlink -f \"$src_dir/..\")\n> diff --git a/utils/run-dist.sh b/utils/run-dist.sh\n> new file mode 100644\n> index 000000000000..3b6c0adb05ed\n> --- /dev/null\n> +++ b/utils/run-dist.sh\n> @@ -0,0 +1,11 @@\n> +#!/bin/sh\n> +\n> +# SPDX-License-Identifier: GPL-2.0-or-later\n> +#\n> +# On a meson dist run, generate the version string and store it in a file.\n> +# This will later be picked up by the utils/gen-version.sh script and used\n> +# instead of re-generating it. This way, if we are not building in the upstream\n> +# git source tree, the upstream version information will be preserved.\n> +\n> +cd \"$MESON_SOURCE_ROOT\" || return\n> +./utils/gen-version.sh > \"$MESON_DIST_ROOT\"/version.gen\n\nDo we need to handle the case of meson dist being run from a tarball\ninstead of git ? It seems like a corner case to me, so probably not very\nuseful, but we could possibly support it by copying\n$MESON_SOURCE_ROOT/version.gen to $MESON_DIST_ROOT/ if the source file\nexists. I'm fine either way.","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1F371C323E\n\tfor ;\n\tThu, 14 Oct 2021 00:46:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7171268F4F;\n\tThu, 14 Oct 2021 02:46:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ACC5468F4B\n\tfor ;\n\tThu, 14 Oct 2021 02:46:53 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2339B1B48;\n\tThu, 14 Oct 2021 02:46:53 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"JTDZZHw5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634172413;\n\tbh=FizWtlhqnrjZ6IQ/DTImB8KqnTusVpkZSMAyfiqsENM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JTDZZHw5ax/G8TaPU2r8hYVfF6Jje6NMRF2PZNEH1qSN/NsnxoNfAi8uDag5vx9aL\n\tqQ9gqRXjzBhnr16yK0hkkn7askATM9CQlyfI1yMpXmvBNSTCyy6hjwiK5Ftk7ZEXDN\n\tDlUuibkWKrveBclj+YjtCng5LkY+GNv+XtGTawLg=","Date":"Thu, 14 Oct 2021 03:46:38 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20211013122312.1943362-1-naush@raspberrypi.com>\n\t<20211013122312.1943362-2-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211013122312.1943362-2-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 1/1] build: Preserve upstream git\n\tversioning using meson dist","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20184,"web_url":"https://patchwork.libcamera.org/comment/20184/","msgid":"","date":"2021-10-14T06:04:40","subject":"Re: [libcamera-devel] [PATCH v2 1/1] build: Preserve upstream git\n\tversioning using meson dist","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your feedback.\n\nOn Thu, 14 Oct 2021 at 01:46, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> (CC'ing Kieran)\n>\n> Thank you for the patch.\n>\n> On Wed, Oct 13, 2021 at 01:23:12PM +0100, Naushir Patuck wrote:\n> > When distributions build and package libcamera libraries, they may not\n> > necessarily run the build in the upstream source tree. In these cases,\n> the git\n> > SHA1 versioning information will be lost.\n> >\n> > This change addresses that problem by requiring package managers to run\n> > 'meson dist' to create a tarball of the source files and build from\n> there.\n> > On runing 'meson dist', the utils/run-dist.sh script will create a\n> version.gen\n> > file in the release tarball with the version string generated from the\n> existing\n> > utils/gen-version.sh script.\n> >\n> > The utils/gen-version.sh script has been updated to check for the\n> presence of\n> > this version.gen file and read the version string from it instead of\n> creating\n> > one.\n>\n> I came across\n> https://sources.debian.org/src/libsmpp34/1.14.0-2/git-version-gen/ which\n> seems to store the version in a file named .tarball-version. Should we\n> follow the same convention ?\n>\n\nSure, I can rename it to .tarball-version.\n\n\n>\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > meson.build | 3 +++\n> > src/libcamera/meson.build | 11 +++++------\n> > utils/gen-version.sh | 9 +++++++++\n> > utils/run-dist.sh | 11 +++++++++++\n> > 4 files changed, 28 insertions(+), 6 deletions(-)\n> > create mode 100644 utils/run-dist.sh\n> >\n> > diff --git a/meson.build b/meson.build\n> > index a49c484fe64e..85ca0013733e 100644\n> > --- a/meson.build\n> > +++ b/meson.build\n> > @@ -24,6 +24,9 @@ endif\n> >\n> > libcamera_version = libcamera_git_version.split('+')[0]\n> >\n> > +# This script gererates the version.gen file on a 'meson dist' command.\n> > +meson.add_dist_script('utils/run-dist.sh')\n> > +\n> > # Configure the build environment.\n> > cc = meson.get_compiler('c')\n> > cxx = meson.get_compiler('cpp')\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 243dd3c180eb..360eaf80ecf1 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -93,12 +93,11 @@ endforeach\n> >\n> > libcamera_sources += control_sources\n> >\n> > -gen_version = meson.source_root() / 'utils' / 'gen-version.sh'\n> > -\n> > -version_cpp = vcs_tag(command : [gen_version, meson.build_root()],\n> > - input : 'version.cpp.in',\n> > - output : 'version.cpp',\n> > - fallback : meson.project_version())\n> > +version_data = configuration_data()\n> > +version_data.set('VCS_TAG', libcamera_git_version)\n> > +version_cpp = configure_file(input : 'version.cpp.in',\n> > + output : 'version.cpp',\n> > + configuration : version_data)\n>\n> This doesn't seem strictly needed, but it avoids running gen-version.sh\n> a second time, which is nice. I however have a nagging feeling that\n> there was a reason why we used vcs_tag and not configure_file, but I\n> can't recall it. We'll find out if it causes issues :-)\n>\n\nThere seems to be a subtle difference from my brief messing around with\nthis. configure_file() will execute on a meson config/setup command, and\nvcs_tag will run on the ninja build command. I did have a problem with my\nchange using the latter (I cannot recall exactly right now, but it was\nsomething\nto do with the $MESON_SOURCE_ROOT env variable not setup when running\nthe gen-version script). I thought this change would be good as you do not\nrun\nthe script twice either.\n\n\n>\n> >\n> > libcamera_sources += version_cpp\n> >\n> > diff --git a/utils/gen-version.sh b/utils/gen-version.sh\n> > index b09ad495f86a..09cede84c25e 100755\n> > --- a/utils/gen-version.sh\n> > +++ b/utils/gen-version.sh\n> > @@ -5,6 +5,15 @@\n> >\n> > build_dir=\"$1\"\n> >\n> > +# If version.gen exists, output the version string from the file and\n> exit.\n> > +# This file is auto-generated on a 'meson dist' command from the\n> run-dist.sh\n> > +# script.\n> > +if [ -f \"${MESON_SOURCE_ROOT}\"/version.gen ]\n> > +then\n> > + cat \"${MESON_SOURCE_ROOT}\"/version.gen\n> > + exit 0\n> > +fi\n> > +\n>\n> Shouldn't we do this only if we're not under git control ? There should\n> be no version.gen file in that case of course, but isn't still a better\n> default ?\n>\n\nIn the original github issue, the reporter had a flow where the tarball\nwould be initialised as a new git repo, and so the sha values extracted\nwere valid values, but had no relation to the upstream sha values. So for\nthose cases, we have to check the file unconditionally.\n\n\n>\n> > # Bail out if the directory isn't under git control\n> > src_dir=$(git rev-parse --git-dir 2>&1) || exit 1\n> > src_dir=$(readlink -f \"$src_dir/..\")\n> > diff --git a/utils/run-dist.sh b/utils/run-dist.sh\n> > new file mode 100644\n> > index 000000000000..3b6c0adb05ed\n> > --- /dev/null\n> > +++ b/utils/run-dist.sh\n> > @@ -0,0 +1,11 @@\n> > +#!/bin/sh\n> > +\n> > +# SPDX-License-Identifier: GPL-2.0-or-later\n> > +#\n> > +# On a meson dist run, generate the version string and store it in a\n> file.\n> > +# This will later be picked up by the utils/gen-version.sh script and\n> used\n> > +# instead of re-generating it. This way, if we are not building in the\n> upstream\n> > +# git source tree, the upstream version information will be preserved.\n> > +\n> > +cd \"$MESON_SOURCE_ROOT\" || return\n> > +./utils/gen-version.sh > \"$MESON_DIST_ROOT\"/version.gen\n>\n> Do we need to handle the case of meson dist being run from a tarball\n> instead of git ? It seems like a corner case to me, so probably not very\n> useful, but we could possibly support it by copying\n> $MESON_SOURCE_ROOT/version.gen to $MESON_DIST_ROOT/ if the source file\n> exists. I'm fine either way.\n>\n\nGood point. Although, I cannot see why meson dist should be run from a\ntarball and not the upstream git tree :-)\n\nThanks,\nNaush\n\n\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4A7AEBDC71\n\tfor ;\n\tThu, 14 Oct 2021 06:04:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A54B768F4D;\n\tThu, 14 Oct 2021 08:04:58 +0200 (CEST)","from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com\n\t[IPv6:2a00:1450:4864:20::12e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0B217604FE\n\tfor ;\n\tThu, 14 Oct 2021 08:04:57 +0200 (CEST)","by mail-lf1-x12e.google.com with SMTP id x27so22254553lfu.5\n\tfor ;\n\tWed, 13 Oct 2021 23:04:56 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"G41khLZm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=dPT8PQjLh5fg49aUhEzLuDETOZJL7j83lRSDGZj5za4=;\n\tb=G41khLZmGtN5HetZju5C8tFNNA8tZXFgi5KvPUmcmw8zTLyGCrZdXj6mbj+WviIJNo\n\tlq6OLJ5kNbXFEQamwj6/lRHMcm/uN+3Pw2hhqxChAOM2+FE6PmJf/4MQcQe6tvjbPzjj\n\tncGMdG4IFSAaRDt5jV/4kF8rqb4nYKFrN8LrQwVQQzLVuNMdwEpjk9BJ5928IkvRfOi6\n\t4yD2OxK34pyUyg1NI+p0AZ9f7/l1V0UNEo4idPWSs15j9u2o6WZdM4tIa4eRpwEhb+0p\n\tcviSGWLqZm8dfllFF+TogMpw24lQNYNTkrr1V7ztMpH2oKsQ1TFviWV+vJX2aXrHc+ZL\n\tMH5A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=dPT8PQjLh5fg49aUhEzLuDETOZJL7j83lRSDGZj5za4=;\n\tb=YL4MaYa377sDJUgsvgxAgZU3NtFfnf3zXc6efGphSeXRoAfak7pulopNZHDqs026Ml\n\t6x/i/wSwM9eR8NIA1lXWuSEFk31QaB+KGdkCXE3W6t9VjeaTQGLpfLaYPNKKj36KrICc\n\tBoEBwHjT75MSeT9K2bE6Wi90Ttp4DGZUrStyBliNGs7a0D0++vqFecExemuCM5fm0JaK\n\tkDU3iSS5PKco9QGlySPXMSKNdDWEq+d+YrJzQkKJSeJpTP5EE+TvsMxDcw3gn2jM2aWy\n\tOkqg7hSQ/fY4zL9pbqv5Gh1iFiu3DFoWvizbu5ctQWGvqgc9CtCW6zMzeJYP+EgKelJX\n\tr2EQ==","X-Gm-Message-State":"AOAM53268Rmn2UtGNR9+02jIdRAlGBk9MCzA45YBaWgxHKSlA8GnR92i\n\tJCPYwlZQh3G14h0QPrtHerldl8ZVPmLUetgC/Q+Ruoz3SAtftg==","X-Google-Smtp-Source":"ABdhPJwaCz7MLHqgWE9ZeypdIfnNVW2kyM+pk8YGX3lGvXNuK2qVKxBIJDKeK1w9B+9hyRX6a+XFcteyDVR/+Fyy3kA=","X-Received":"by 2002:a05:6512:3407:: with SMTP id\n\ti7mr3286368lfr.108.1634191495947; \n\tWed, 13 Oct 2021 23:04:55 -0700 (PDT)","MIME-Version":"1.0","References":"<20211013122312.1943362-1-naush@raspberrypi.com>\n\t<20211013122312.1943362-2-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Thu, 14 Oct 2021 07:04:40 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"0000000000009c7bfa05ce49dbd0\"","Subject":"Re: [libcamera-devel] [PATCH v2 1/1] build: Preserve upstream git\n\tversioning using meson dist","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20191,"web_url":"https://patchwork.libcamera.org/comment/20191/","msgid":"","date":"2021-10-14T09:58:49","subject":"Re: [libcamera-devel] [PATCH v2 1/1] build: Preserve upstream git\n\tversioning using meson dist","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Thu, 14 Oct 2021 at 07:04, Naushir Patuck wrote:\n\n> Hi Laurent,\n>\n> Thank you for your feedback.\n>\n> On Thu, 14 Oct 2021 at 01:46, Laurent Pinchart <\n> laurent.pinchart@ideasonboard.com> wrote:\n>\n>> Hi Naush,\n>>\n>> (CC'ing Kieran)\n>>\n>> Thank you for the patch.\n>>\n>> On Wed, Oct 13, 2021 at 01:23:12PM +0100, Naushir Patuck wrote:\n>> > When distributions build and package libcamera libraries, they may not\n>> > necessarily run the build in the upstream source tree. In these cases,\n>> the git\n>> > SHA1 versioning information will be lost.\n>> >\n>> > This change addresses that problem by requiring package managers to run\n>> > 'meson dist' to create a tarball of the source files and build from\n>> there.\n>> > On runing 'meson dist', the utils/run-dist.sh script will create a\n>> version.gen\n>> > file in the release tarball with the version string generated from the\n>> existing\n>> > utils/gen-version.sh script.\n>> >\n>> > The utils/gen-version.sh script has been updated to check for the\n>> presence of\n>> > this version.gen file and read the version string from it instead of\n>> creating\n>> > one.\n>>\n>> I came across\n>> https://sources.debian.org/src/libsmpp34/1.14.0-2/git-version-gen/ which\n>> seems to store the version in a file named .tarball-version. Should we\n>> follow the same convention ?\n>>\n>\n> Sure, I can rename it to .tarball-version.\n>\n>\n>>\n>> > Signed-off-by: Naushir Patuck \n>> > ---\n>> > meson.build | 3 +++\n>> > src/libcamera/meson.build | 11 +++++------\n>> > utils/gen-version.sh | 9 +++++++++\n>> > utils/run-dist.sh | 11 +++++++++++\n>> > 4 files changed, 28 insertions(+), 6 deletions(-)\n>> > create mode 100644 utils/run-dist.sh\n>> >\n>> > diff --git a/meson.build b/meson.build\n>> > index a49c484fe64e..85ca0013733e 100644\n>> > --- a/meson.build\n>> > +++ b/meson.build\n>> > @@ -24,6 +24,9 @@ endif\n>> >\n>> > libcamera_version = libcamera_git_version.split('+')[0]\n>> >\n>> > +# This script gererates the version.gen file on a 'meson dist' command.\n>> > +meson.add_dist_script('utils/run-dist.sh')\n>> > +\n>> > # Configure the build environment.\n>> > cc = meson.get_compiler('c')\n>> > cxx = meson.get_compiler('cpp')\n>> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>> > index 243dd3c180eb..360eaf80ecf1 100644\n>> > --- a/src/libcamera/meson.build\n>> > +++ b/src/libcamera/meson.build\n>> > @@ -93,12 +93,11 @@ endforeach\n>> >\n>> > libcamera_sources += control_sources\n>> >\n>> > -gen_version = meson.source_root() / 'utils' / 'gen-version.sh'\n>> > -\n>> > -version_cpp = vcs_tag(command : [gen_version, meson.build_root()],\n>> > - input : 'version.cpp.in',\n>> > - output : 'version.cpp',\n>> > - fallback : meson.project_version())\n>> > +version_data = configuration_data()\n>> > +version_data.set('VCS_TAG', libcamera_git_version)\n>> > +version_cpp = configure_file(input : 'version.cpp.in',\n>> > + output : 'version.cpp',\n>> > + configuration : version_data)\n>>\n>> This doesn't seem strictly needed, but it avoids running gen-version.sh\n>> a second time, which is nice. I however have a nagging feeling that\n>> there was a reason why we used vcs_tag and not configure_file, but I\n>> can't recall it. We'll find out if it causes issues :-)\n>>\n>\n> There seems to be a subtle difference from my brief messing around with\n> this. configure_file() will execute on a meson config/setup command, and\n> vcs_tag will run on the ninja build command. I did have a problem with my\n> change using the latter (I cannot recall exactly right now, but it was\n> something\n> to do with the $MESON_SOURCE_ROOT env variable not setup when running\n> the gen-version script). I thought this change would be good as you do\n> not run\n> the script twice either.\n>\n>\n>>\n>> >\n>> > libcamera_sources += version_cpp\n>> >\n>> > diff --git a/utils/gen-version.sh b/utils/gen-version.sh\n>> > index b09ad495f86a..09cede84c25e 100755\n>> > --- a/utils/gen-version.sh\n>> > +++ b/utils/gen-version.sh\n>> > @@ -5,6 +5,15 @@\n>> >\n>> > build_dir=\"$1\"\n>> >\n>> > +# If version.gen exists, output the version string from the file and\n>> exit.\n>> > +# This file is auto-generated on a 'meson dist' command from the\n>> run-dist.sh\n>> > +# script.\n>> > +if [ -f \"${MESON_SOURCE_ROOT}\"/version.gen ]\n>> > +then\n>> > + cat \"${MESON_SOURCE_ROOT}\"/version.gen\n>> > + exit 0\n>> > +fi\n>> > +\n>>\n>> Shouldn't we do this only if we're not under git control ? There should\n>> be no version.gen file in that case of course, but isn't still a better\n>> default ?\n>>\n>\n> In the original github issue, the reporter had a flow where the tarball\n> would be initialised as a new git repo, and so the sha values extracted\n> were valid values, but had no relation to the upstream sha values. So for\n> those cases, we have to check the file unconditionally.\n>\n>\n>>\n>> > # Bail out if the directory isn't under git control\n>> > src_dir=$(git rev-parse --git-dir 2>&1) || exit 1\n>> > src_dir=$(readlink -f \"$src_dir/..\")\n>> > diff --git a/utils/run-dist.sh b/utils/run-dist.sh\n>> > new file mode 100644\n>> > index 000000000000..3b6c0adb05ed\n>> > --- /dev/null\n>> > +++ b/utils/run-dist.sh\n>> > @@ -0,0 +1,11 @@\n>> > +#!/bin/sh\n>> > +\n>> > +# SPDX-License-Identifier: GPL-2.0-or-later\n>> > +#\n>> > +# On a meson dist run, generate the version string and store it in a\n>> file.\n>> > +# This will later be picked up by the utils/gen-version.sh script and\n>> used\n>> > +# instead of re-generating it. This way, if we are not building in the\n>> upstream\n>> > +# git source tree, the upstream version information will be preserved.\n>> > +\n>> > +cd \"$MESON_SOURCE_ROOT\" || return\n>> > +./utils/gen-version.sh > \"$MESON_DIST_ROOT\"/version.gen\n>>\n>> Do we need to handle the case of meson dist being run from a tarball\n>> instead of git ? It seems like a corner case to me, so probably not very\n>> useful, but we could possibly support it by copying\n>> $MESON_SOURCE_ROOT/version.gen to $MESON_DIST_ROOT/ if the source file\n>> exists. I'm fine either way.\n>>\n>\n> Good point. Although, I cannot see why meson dist should be run from a\n> tarball and not the upstream git tree :-)\n>\n\nSo it turns out the above change is not needed after all:\n\n$ meson dist --no-tests\nDist currently only works with Git or Mercurial repos\n\nRegards,\nNaush","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id F06DBBDC71\n\tfor ;\n\tThu, 14 Oct 2021 09:59:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 60B8E68F51;\n\tThu, 14 Oct 2021 11:59:13 +0200 (CEST)","from mail-lf1-x132.google.com (mail-lf1-x132.google.com\n\t[IPv6:2a00:1450:4864:20::132])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 352C068F4A\n\tfor ;\n\tThu, 14 Oct 2021 11:59:12 +0200 (CEST)","by mail-lf1-x132.google.com with SMTP id x27so24308271lfa.9\n\tfor ;\n\tThu, 14 Oct 2021 02:59:12 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"bxkLbU2a\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=0oY7agTZiYnp3kZSzxMMkC+EU86Z0JOOBKOcZLJl3JU=;\n\tb=bxkLbU2awKUx4BbDraVr5Tz8MIW7DFoM2iBLTXaS5hQAS9mbFECnMnIew0TGuxVMaK\n\tc7OwlDaCHp3KzSc7DR15Wh0wjEfvNPsujyC+KZnrMLA3H1IrDKmBe9iQGHE+KAz6y6Xe\n\t3H2on6qaaxjMwNLnolTU1UdTlPcbnlo1wBxCzR28cLPpZvLgYNn+nF5+1ec4i5WTeMPM\n\tfF2/62OAB1VE+njARhAX9zhbeywCvaRJ1jzvi/KGthlHU981Pjjq0CNg2/JGB2Ys4ipT\n\tHWYm9YxgtQtO6ihQGKExX6wrGbBh+6D9saSNyZjb0UZ7xmYtb7Y2wxqYUtX7pSrnop4f\n\tO3AA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=0oY7agTZiYnp3kZSzxMMkC+EU86Z0JOOBKOcZLJl3JU=;\n\tb=1hM7MeQmMA9Jz3MhHaYDgIZzNl25rW7uyq8GwZODCCr/FWskfJ9dcqzdY2rwxCHz87\n\tAwuki7gLf9gJduu7KIoBBKtHOMzyjX2g4oQ3aD0sEVhfaeI8Yk4yV3yQesfs4kKTpPuS\n\tMk0e1e01nZSlRioBWIEMVwOJE+yENw3vvkkYQ2fNiCDLB3MsxFAfDenCmT4Nhh09qDVM\n\tDYkWtsjcTl3pL2Nocti/irUPIkeYJlX0wE3BmlFJVq6kMLOA7+G/eQvMLK8z2ugMnaX1\n\tHRFxjNFV0FD+AFr8hD9p8wgPXhB6svkb8BZoua+Pc+Sl5CmlsgFzgMs9QsN1ydYUDwhm\n\tKbvg==","X-Gm-Message-State":"AOAM531eAy9EMEi22vF7tB6jwbGDlHoEUal0AsK1vCiygUscrBcHFN3+\n\toIlSvX+hKr7b4gJU5IDQ8aBSNAjak/fkF2J6uGLgQ8e6/b+KrXL0","X-Google-Smtp-Source":"ABdhPJw8TnMGxFhNbwL1CEjyu4M1Jm4sir3upbQtba9CO72cEb6MdFuX10QN/dvlUEKbDl0PDNl9cEwg97PIvRHGE0o=","X-Received":"by 2002:a19:ad0d:: with SMTP id\n\tt13mr4282782lfc.161.1634205545435; \n\tThu, 14 Oct 2021 02:59:05 -0700 (PDT)","MIME-Version":"1.0","References":"<20211013122312.1943362-1-naush@raspberrypi.com>\n\t<20211013122312.1943362-2-naush@raspberrypi.com>\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Thu, 14 Oct 2021 10:58:49 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"00000000000006a8dc05ce4d2179\"","Subject":"Re: [libcamera-devel] [PATCH v2 1/1] build: Preserve upstream git\n\tversioning using meson dist","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20192,"web_url":"https://patchwork.libcamera.org/comment/20192/","msgid":"","date":"2021-10-14T10:20:59","subject":"Re: [libcamera-devel] [PATCH v2 1/1] build: Preserve upstream git\n\tversioning using meson dist","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Thu, Oct 14, 2021 at 07:04:40AM +0100, Naushir Patuck wrote:\n> On Thu, 14 Oct 2021 at 01:46, Laurent Pinchart wrote:\n> \n> > Hi Naush,\n> >\n> > (CC'ing Kieran)\n> >\n> > Thank you for the patch.\n> >\n> > On Wed, Oct 13, 2021 at 01:23:12PM +0100, Naushir Patuck wrote:\n> > > When distributions build and package libcamera libraries, they may not\n> > > necessarily run the build in the upstream source tree. In these cases, the git\n> > > SHA1 versioning information will be lost.\n> > >\n> > > This change addresses that problem by requiring package managers to run\n> > > 'meson dist' to create a tarball of the source files and build from there.\n> > > On runing 'meson dist', the utils/run-dist.sh script will create a version.gen\n> > > file in the release tarball with the version string generated from the existing\n> > > utils/gen-version.sh script.\n> > >\n> > > The utils/gen-version.sh script has been updated to check for the presence of\n> > > this version.gen file and read the version string from it instead of creating\n> > > one.\n> >\n> > I came across\n> > https://sources.debian.org/src/libsmpp34/1.14.0-2/git-version-gen/ which\n> > seems to store the version in a file named .tarball-version. Should we\n> > follow the same convention ?\n> \n> Sure, I can rename it to .tarball-version.\n> \n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > meson.build | 3 +++\n> > > src/libcamera/meson.build | 11 +++++------\n> > > utils/gen-version.sh | 9 +++++++++\n> > > utils/run-dist.sh | 11 +++++++++++\n> > > 4 files changed, 28 insertions(+), 6 deletions(-)\n> > > create mode 100644 utils/run-dist.sh\n> > >\n> > > diff --git a/meson.build b/meson.build\n> > > index a49c484fe64e..85ca0013733e 100644\n> > > --- a/meson.build\n> > > +++ b/meson.build\n> > > @@ -24,6 +24,9 @@ endif\n> > >\n> > > libcamera_version = libcamera_git_version.split('+')[0]\n> > >\n> > > +# This script gererates the version.gen file on a 'meson dist' command.\n> > > +meson.add_dist_script('utils/run-dist.sh')\n> > > +\n> > > # Configure the build environment.\n> > > cc = meson.get_compiler('c')\n> > > cxx = meson.get_compiler('cpp')\n> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > index 243dd3c180eb..360eaf80ecf1 100644\n> > > --- a/src/libcamera/meson.build\n> > > +++ b/src/libcamera/meson.build\n> > > @@ -93,12 +93,11 @@ endforeach\n> > >\n> > > libcamera_sources += control_sources\n> > >\n> > > -gen_version = meson.source_root() / 'utils' / 'gen-version.sh'\n> > > -\n> > > -version_cpp = vcs_tag(command : [gen_version, meson.build_root()],\n> > > - input : 'version.cpp.in',\n> > > - output : 'version.cpp',\n> > > - fallback : meson.project_version())\n> > > +version_data = configuration_data()\n> > > +version_data.set('VCS_TAG', libcamera_git_version)\n> > > +version_cpp = configure_file(input : 'version.cpp.in',\n> > > + output : 'version.cpp',\n> > > + configuration : version_data)\n> >\n> > This doesn't seem strictly needed, but it avoids running gen-version.sh\n> > a second time, which is nice. I however have a nagging feeling that\n> > there was a reason why we used vcs_tag and not configure_file, but I\n> > can't recall it. We'll find out if it causes issues :-)\n> \n> There seems to be a subtle difference from my brief messing around with\n> this. configure_file() will execute on a meson config/setup command, and\n> vcs_tag will run on the ninja build command. I did have a problem with my\n> change using the latter (I cannot recall exactly right now, but it was something\n> to do with the $MESON_SOURCE_ROOT env variable not setup when running\n> the gen-version script). I thought this change would be good as you do not run\n> the script twice either.\n\nDoesn't this mean that we'll record the wrong version if one adds a new\ncommit and recompiles without reconfiguring ? Adding the SHA1 to\nversion.cpp, and printing it in the log, is meant to help catching\nissues when libcamera is being deployed to the wrong directory by\nmistake during development, so I think this is a very important use\ncase.\n\n> > > libcamera_sources += version_cpp\n> > >\n> > > diff --git a/utils/gen-version.sh b/utils/gen-version.sh\n> > > index b09ad495f86a..09cede84c25e 100755\n> > > --- a/utils/gen-version.sh\n> > > +++ b/utils/gen-version.sh\n> > > @@ -5,6 +5,15 @@\n> > >\n> > > build_dir=\"$1\"\n> > >\n> > > +# If version.gen exists, output the version string from the file and exit.\n> > > +# This file is auto-generated on a 'meson dist' command from the run-dist.sh\n> > > +# script.\n> > > +if [ -f \"${MESON_SOURCE_ROOT}\"/version.gen ]\n> > > +then\n> > > + cat \"${MESON_SOURCE_ROOT}\"/version.gen\n> > > + exit 0\n> > > +fi\n> > > +\n> >\n> > Shouldn't we do this only if we're not under git control ? There should\n> > be no version.gen file in that case of course, but isn't still a better\n> > default ?\n> \n> In the original github issue, the reporter had a flow where the tarball\n> would be initialised as a new git repo, and so the sha values extracted\n> were valid values, but had no relation to the upstream sha values. So for\n> those cases, we have to check the file unconditionally.\n> \n> > > # Bail out if the directory isn't under git control\n> > > src_dir=$(git rev-parse --git-dir 2>&1) || exit 1\n> > > src_dir=$(readlink -f \"$src_dir/..\")\n> > > diff --git a/utils/run-dist.sh b/utils/run-dist.sh\n> > > new file mode 100644\n> > > index 000000000000..3b6c0adb05ed\n> > > --- /dev/null\n> > > +++ b/utils/run-dist.sh\n> > > @@ -0,0 +1,11 @@\n> > > +#!/bin/sh\n> > > +\n> > > +# SPDX-License-Identifier: GPL-2.0-or-later\n> > > +#\n> > > +# On a meson dist run, generate the version string and store it in a file.\n> > > +# This will later be picked up by the utils/gen-version.sh script and used\n> > > +# instead of re-generating it. This way, if we are not building in the upstream\n> > > +# git source tree, the upstream version information will be preserved.\n> > > +\n> > > +cd \"$MESON_SOURCE_ROOT\" || return\n> > > +./utils/gen-version.sh > \"$MESON_DIST_ROOT\"/version.gen\n> >\n> > Do we need to handle the case of meson dist being run from a tarball\n> > instead of git ? It seems like a corner case to me, so probably not very\n> > useful, but we could possibly support it by copying\n> > $MESON_SOURCE_ROOT/version.gen to $MESON_DIST_ROOT/ if the source file\n> > exists. I'm fine either way.\n> \n> Good point. Although, I cannot see why meson dist should be run from a\n> tarball and not the upstream git tree :-)","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C1D92C323E\n\tfor ;\n\tThu, 14 Oct 2021 10:21:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 44FC268541;\n\tThu, 14 Oct 2021 12:21:16 +0200 (CEST)","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 B877268541\n\tfor ;\n\tThu, 14 Oct 2021 12:21:14 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1885F2F3;\n\tThu, 14 Oct 2021 12:21:14 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Aai2Y/vd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634206874;\n\tbh=4NwGZ8cUGW0iMRxaMiZsuw5DtalSxskvCXIAh3eGdfw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Aai2Y/vd7l75LuL511T6GP3BdoEYcHQjWlmf2nkpwTTE0aLfkLPzhq19azPsZ9kBT\n\tT9+VP667sKzV1m9g68BlHkmBVpeVqJirn05RwhoVQWBpGSSwxKMpnDXYmkxQPCZudr\n\tFkhAxoag4h+PdAoE9DnOLli7OQvlbWyeBF403O8Q=","Date":"Thu, 14 Oct 2021 13:20:59 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20211013122312.1943362-1-naush@raspberrypi.com>\n\t<20211013122312.1943362-2-naush@raspberrypi.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 1/1] build: Preserve upstream git\n\tversioning using meson dist","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20195,"web_url":"https://patchwork.libcamera.org/comment/20195/","msgid":"","date":"2021-10-14T10:26:30","subject":"Re: [libcamera-devel] [PATCH v2 1/1] build: Preserve upstream git\n\tversioning using meson dist","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Thu, 14 Oct 2021 at 11:21, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> On Thu, Oct 14, 2021 at 07:04:40AM +0100, Naushir Patuck wrote:\n> > On Thu, 14 Oct 2021 at 01:46, Laurent Pinchart wrote:\n> >\n> > > Hi Naush,\n> > >\n> > > (CC'ing Kieran)\n> > >\n> > > Thank you for the patch.\n> > >\n> > > On Wed, Oct 13, 2021 at 01:23:12PM +0100, Naushir Patuck wrote:\n> > > > When distributions build and package libcamera libraries, they may\n> not\n> > > > necessarily run the build in the upstream source tree. In these\n> cases, the git\n> > > > SHA1 versioning information will be lost.\n> > > >\n> > > > This change addresses that problem by requiring package managers to\n> run\n> > > > 'meson dist' to create a tarball of the source files and build from\n> there.\n> > > > On runing 'meson dist', the utils/run-dist.sh script will create a\n> version.gen\n> > > > file in the release tarball with the version string generated from\n> the existing\n> > > > utils/gen-version.sh script.\n> > > >\n> > > > The utils/gen-version.sh script has been updated to check for the\n> presence of\n> > > > this version.gen file and read the version string from it instead of\n> creating\n> > > > one.\n> > >\n> > > I came across\n> > > https://sources.debian.org/src/libsmpp34/1.14.0-2/git-version-gen/\n> which\n> > > seems to store the version in a file named .tarball-version. Should we\n> > > follow the same convention ?\n> >\n> > Sure, I can rename it to .tarball-version.\n> >\n> > > > Signed-off-by: Naushir Patuck \n> > > > ---\n> > > > meson.build | 3 +++\n> > > > src/libcamera/meson.build | 11 +++++------\n> > > > utils/gen-version.sh | 9 +++++++++\n> > > > utils/run-dist.sh | 11 +++++++++++\n> > > > 4 files changed, 28 insertions(+), 6 deletions(-)\n> > > > create mode 100644 utils/run-dist.sh\n> > > >\n> > > > diff --git a/meson.build b/meson.build\n> > > > index a49c484fe64e..85ca0013733e 100644\n> > > > --- a/meson.build\n> > > > +++ b/meson.build\n> > > > @@ -24,6 +24,9 @@ endif\n> > > >\n> > > > libcamera_version = libcamera_git_version.split('+')[0]\n> > > >\n> > > > +# This script gererates the version.gen file on a 'meson dist'\n> command.\n> > > > +meson.add_dist_script('utils/run-dist.sh')\n> > > > +\n> > > > # Configure the build environment.\n> > > > cc = meson.get_compiler('c')\n> > > > cxx = meson.get_compiler('cpp')\n> > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > > index 243dd3c180eb..360eaf80ecf1 100644\n> > > > --- a/src/libcamera/meson.build\n> > > > +++ b/src/libcamera/meson.build\n> > > > @@ -93,12 +93,11 @@ endforeach\n> > > >\n> > > > libcamera_sources += control_sources\n> > > >\n> > > > -gen_version = meson.source_root() / 'utils' / 'gen-version.sh'\n> > > > -\n> > > > -version_cpp = vcs_tag(command : [gen_version, meson.build_root()],\n> > > > - input : 'version.cpp.in',\n> > > > - output : 'version.cpp',\n> > > > - fallback : meson.project_version())\n> > > > +version_data = configuration_data()\n> > > > +version_data.set('VCS_TAG', libcamera_git_version)\n> > > > +version_cpp = configure_file(input : 'version.cpp.in',\n> > > > + output : 'version.cpp',\n> > > > + configuration : version_data)\n> > >\n> > > This doesn't seem strictly needed, but it avoids running gen-version.sh\n> > > a second time, which is nice. I however have a nagging feeling that\n> > > there was a reason why we used vcs_tag and not configure_file, but I\n> > > can't recall it. We'll find out if it causes issues :-)\n> >\n> > There seems to be a subtle difference from my brief messing around with\n> > this. configure_file() will execute on a meson config/setup command, and\n> > vcs_tag will run on the ninja build command. I did have a problem with\n> my\n> > change using the latter (I cannot recall exactly right now, but it was\n> something\n> > to do with the $MESON_SOURCE_ROOT env variable not setup when running\n> > the gen-version script). I thought this change would be good as you do\n> not run\n> > the script twice either.\n>\n> Doesn't this mean that we'll record the wrong version if one adds a new\n> commit and recompiles without reconfiguring ? Adding the SHA1 to\n> version.cpp, and printing it in the log, is meant to help catching\n> issues when libcamera is being deployed to the wrong directory by\n> mistake during development, so I think this is a very important use\n> case.\n>\n\nYou are correct, that is a common use case that must be accounted for!\nPerhaps this was the reason the original code used vcs_tag :-)\n\nI'll revert this back and try to work through my issue I was seeing in\nanother\nway.\n\n\n\n>\n> > > > libcamera_sources += version_cpp\n> > > >\n> > > > diff --git a/utils/gen-version.sh b/utils/gen-version.sh\n> > > > index b09ad495f86a..09cede84c25e 100755\n> > > > --- a/utils/gen-version.sh\n> > > > +++ b/utils/gen-version.sh\n> > > > @@ -5,6 +5,15 @@\n> > > >\n> > > > build_dir=\"$1\"\n> > > >\n> > > > +# If version.gen exists, output the version string from the file\n> and exit.\n> > > > +# This file is auto-generated on a 'meson dist' command from the\n> run-dist.sh\n> > > > +# script.\n> > > > +if [ -f \"${MESON_SOURCE_ROOT}\"/version.gen ]\n> > > > +then\n> > > > + cat \"${MESON_SOURCE_ROOT}\"/version.gen\n> > > > + exit 0\n> > > > +fi\n> > > > +\n> > >\n> > > Shouldn't we do this only if we're not under git control ? There should\n> > > be no version.gen file in that case of course, but isn't still a better\n> > > default ?\n> >\n> > In the original github issue, the reporter had a flow where the tarball\n> > would be initialised as a new git repo, and so the sha values extracted\n> > were valid values, but had no relation to the upstream sha values. So for\n> > those cases, we have to check the file unconditionally.\n> >\n> > > > # Bail out if the directory isn't under git control\n> > > > src_dir=$(git rev-parse --git-dir 2>&1) || exit 1\n> > > > src_dir=$(readlink -f \"$src_dir/..\")\n> > > > diff --git a/utils/run-dist.sh b/utils/run-dist.sh\n> > > > new file mode 100644\n> > > > index 000000000000..3b6c0adb05ed\n> > > > --- /dev/null\n> > > > +++ b/utils/run-dist.sh\n> > > > @@ -0,0 +1,11 @@\n> > > > +#!/bin/sh\n> > > > +\n> > > > +# SPDX-License-Identifier: GPL-2.0-or-later\n> > > > +#\n> > > > +# On a meson dist run, generate the version string and store it in\n> a file.\n> > > > +# This will later be picked up by the utils/gen-version.sh script\n> and used\n> > > > +# instead of re-generating it. This way, if we are not building in\n> the upstream\n> > > > +# git source tree, the upstream version information will be\n> preserved.\n> > > > +\n> > > > +cd \"$MESON_SOURCE_ROOT\" || return\n> > > > +./utils/gen-version.sh > \"$MESON_DIST_ROOT\"/version.gen\n> > >\n> > > Do we need to handle the case of meson dist being run from a tarball\n> > > instead of git ? It seems like a corner case to me, so probably not\n> very\n> > > useful, but we could possibly support it by copying\n> > > $MESON_SOURCE_ROOT/version.gen to $MESON_DIST_ROOT/ if the source file\n> > > exists. I'm fine either way.\n> >\n> > Good point. Although, I cannot see why meson dist should be run from a\n> > tarball and not the upstream git tree :-)\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 17720BDC71\n\tfor ;\n\tThu, 14 Oct 2021 10:26:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D77A868F4F;\n\tThu, 14 Oct 2021 12:26:48 +0200 (CEST)","from mail-lf1-x12b.google.com (mail-lf1-x12b.google.com\n\t[IPv6:2a00:1450:4864:20::12b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4F4A168541\n\tfor ;\n\tThu, 14 Oct 2021 12:26:47 +0200 (CEST)","by mail-lf1-x12b.google.com with SMTP id u21so21602034lff.8\n\tfor ;\n\tThu, 14 Oct 2021 03:26:47 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"itu+D3H4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=BuSZsXrL16ubZxzpheFmfJHHQ04jCUzKY5D0iFPHRgM=;\n\tb=itu+D3H4qCLh/8ek53ksSVumY4gGR/KDs1oFRb2KUZeMyD7rbAk7aYv31aZpuyQm7g\n\t/c3Bfyi7lNBEk2Ew2aotyh1lBI6M2lhev/EK7yQ1O/khn+EUoyt0unqkc0mEksTovxZM\n\tiY1c2EK78dT01q3ta41r2u7qSuhhiErfEiayMqhZIMMHuXpw90F04JKASEKydX3qk5YB\n\tw5Vkt4VydQ7gSKPB3uJQw8a1XaHAJkwj2+ShJvBd2GxrUhoW2TvV3jnxRub/+Bj10+2Q\n\trzpFKbCKnmh/hTNruMeSSK5bQnu1XkbtxZyu7tV4FZ3AnE+d3H7nW1P+z/id3oNlqvcI\n\toajw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=BuSZsXrL16ubZxzpheFmfJHHQ04jCUzKY5D0iFPHRgM=;\n\tb=4IpZk3yIMGytb+O7f6dG8ygd5mL4h56nestgva1FPWXGerjMqmeQHSYU3nPsklAgyI\n\tf0BEzoQ1dL6hqnxoPHUSJY7syT4J1UqTQYJrq9ldO6JlgDZFfqovyxT+bPDIpqik/M81\n\tET/Ud865Joe2JQlkpljSYyN5GwuIwxFBVKqNRYpDLAieBjEyGf6q+KDNjVQpfHeYJfb1\n\tk/uKz9+w60+1/jaHpInuLfLJJ+H/kt4W9jp/1z9MRwXi/ndW8P8jPl5PYOg0T1+79mcZ\n\tNqjzrMfBnxfQg2IXKZfrg2VBYjtam16bYp/vdiLSvS0Tu3FOKzX7WRueeNOYlGv7d8/Q\n\te9vg==","X-Gm-Message-State":"AOAM530r6D+cZX4sjsz4/sKwxb6FaEY+yDk9enZ04vpQy9bFvfMb0xyd\n\t/PJXMny8gHiy2WmSTkGl8jerNYSEMGs3gMVesEaW7Y1qaQPjUlza","X-Google-Smtp-Source":"ABdhPJyle4f8a9jp7BI7LQtGJbdiKqsiPo+e3u77mgIaUkOoXuOwfbsILVBqVKyR/Toplm74O57Sg3m0eksfQJTC4wY=","X-Received":"by 2002:ac2:5dfc:: with SMTP id z28mr4427038lfq.79.1634207206646;\n\tThu, 14 Oct 2021 03:26:46 -0700 (PDT)","MIME-Version":"1.0","References":"<20211013122312.1943362-1-naush@raspberrypi.com>\n\t<20211013122312.1943362-2-naush@raspberrypi.com>\n\t\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Thu, 14 Oct 2021 11:26:30 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"0000000000000ab78905ce4d841c\"","Subject":"Re: [libcamera-devel] [PATCH v2 1/1] build: Preserve upstream git\n\tversioning using meson dist","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]