diff mbox series

dtc: pass version as parameter instead of querying git

Message ID 20231213210703.667740-1-peter.marko@siemens.com
State New
Headers show
Series dtc: pass version as parameter instead of querying git | expand

Commit Message

Peter Marko Dec. 13, 2023, 9:07 p.m. UTC
From: Peter Marko <peter.marko@siemens.com>

Since switch from Makefile to meson based build,
the version is no longer hardcoded but queried from git tag.

This works only if git history is available.
When shallow tarballs are used, tag is not available.

Example error for trusted-firmware-a from meta-arm:
dtc version too old (039a994), you need at least version 1.4.4

Signed-off-by: Peter Marko <peter.marko@siemens.com>
---
 ...n-as-parameter-instead-of-querying-g.patch | 49 +++++++++++++++++++
 meta/recipes-kernel/dtc/dtc_1.7.0.bb          |  7 ++-
 2 files changed, 54 insertions(+), 2 deletions(-)
 create mode 100644 meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-instead-of-querying-g.patch

Comments

Alexander Kanavin Dec. 13, 2023, 9:10 p.m. UTC | #1
On Wed, 13 Dec 2023 at 22:07, Peter Marko via lists.openembedded.org
<peter.marko=siemens.com@lists.openembedded.org> wrote:
> +From f32056f825b926b5820d43478d11e92eee1ec91f Mon Sep 17 00:00:00 2001
> +From: Peter Marko <peter.marko@siemens.com>
> +Date: Wed, 13 Dec 2023 21:18:14 +0100
> +Subject: [PATCH] meson: get version as parameter instead of querying git
> +
> +When using shallow tarballs, git tag is not available.
> +This causes version to be git hash.
> +This prevents application from checking minimal required version.
> +
> +Example error for trusted-firmware-a from meta-arm:
> +dtc version too old (039a994), you need at least version 1.4.4
> +
> +Upstream-Status: Inappropriate [oe specific]

This is not really oe-specific, please submit upstream first.

Alex
Richard Purdie Dec. 13, 2023, 9:11 p.m. UTC | #2
On Wed, 2023-12-13 at 22:07 +0100, Peter Marko via
lists.openembedded.org wrote:
> From: Peter Marko <peter.marko@siemens.com>
> 
> Since switch from Makefile to meson based build,
> the version is no longer hardcoded but queried from git tag.
> 
> This works only if git history is available.
> When shallow tarballs are used, tag is not available.
> 
> Example error for trusted-firmware-a from meta-arm:
> dtc version too old (039a994), you need at least version 1.4.4
> 
> Signed-off-by: Peter Marko <peter.marko@siemens.com>
> ---
>  ...n-as-parameter-instead-of-querying-g.patch | 49 +++++++++++++++++++
>  meta/recipes-kernel/dtc/dtc_1.7.0.bb          |  7 ++-
>  2 files changed, 54 insertions(+), 2 deletions(-)
>  create mode 100644 meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-instead-of-querying-g.patch
> 
> diff --git a/meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-instead-of-querying-g.patch b/meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-instead-of-querying-g.patch
> new file mode 100644
> index 0000000000..d7906bd6f9
> --- /dev/null
> +++ b/meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-instead-of-querying-g.patch
> @@ -0,0 +1,49 @@
> +From f32056f825b926b5820d43478d11e92eee1ec91f Mon Sep 17 00:00:00 2001
> +From: Peter Marko <peter.marko@siemens.com>
> +Date: Wed, 13 Dec 2023 21:18:14 +0100
> +Subject: [PATCH] meson: get version as parameter instead of querying git
> +
> +When using shallow tarballs, git tag is not available.
> +This causes version to be git hash.
> +This prevents application from checking minimal required version.
> +
> +Example error for trusted-firmware-a from meta-arm:
> +dtc version too old (039a994), you need at least version 1.4.4
> +
> +Upstream-Status: Inappropriate [oe specific]
> +
> +Signed-off-by: Peter Marko <peter.marko@siemens.com>
> +---
> + meson.build       | 3 ++-
> + meson_options.txt | 2 ++
> + 2 files changed, 4 insertions(+), 1 deletion(-)
> +

I'm not sure that is inappropriate. Has anyone talked to upstream about
builds failing from shallow clones?

Cheers,

Richard
Peter Marko Dec. 13, 2023, 9:53 p.m. UTC | #3
-----Original Message-----
From: Richard Purdie <richard.purdie@linuxfoundation.org> 
Sent: Wednesday, December 13, 2023 22:11
To: Marko, Peter (ADV D EU SK BFS1) <Peter.Marko@siemens.com>; openembedded-core@lists.openembedded.org
Subject: Re: [OE-core][PATCH] dtc: pass version as parameter instead of querying git

> On Wed, 2023-12-13 at 22:07 +0100, Peter Marko via lists.openembedded.org wrote:
> > From: Peter Marko <peter.marko@siemens.com>
> > 
> > Since switch from Makefile to meson based build, the version is no 
> > longer hardcoded but queried from git tag.
> > 
> > This works only if git history is available.
> > When shallow tarballs are used, tag is not available.
> > 
> > Example error for trusted-firmware-a from meta-arm:
> > dtc version too old (039a994), you need at least version 1.4.4
> > 
> > Signed-off-by: Peter Marko <peter.marko@siemens.com>
> > ---
> >  ...n-as-parameter-instead-of-querying-g.patch | 49 +++++++++++++++++++
> >  meta/recipes-kernel/dtc/dtc_1.7.0.bb          |  7 ++-
> >  2 files changed, 54 insertions(+), 2 deletions(-)  create mode 100644 
> > meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-instea
> > d-of-querying-g.patch
> > 
> > diff --git 
> > a/meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-inst
> > ead-of-querying-g.patch 
> > b/meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-inst
> > ead-of-querying-g.patch
> > new file mode 100644
> > index 0000000000..d7906bd6f9
> > --- /dev/null
> > +++ b/meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-
> > +++ instead-of-querying-g.patch
> > @@ -0,0 +1,49 @@
> > +From f32056f825b926b5820d43478d11e92eee1ec91f Mon Sep 17 00:00:00 
> > +2001
> > +From: Peter Marko <peter.marko@siemens.com>
> > +Date: Wed, 13 Dec 2023 21:18:14 +0100
> > +Subject: [PATCH] meson: get version as parameter instead of querying 
> > +git
> > +
> > +When using shallow tarballs, git tag is not available.
> > +This causes version to be git hash.
> > +This prevents application from checking minimal required version.
> > +
> > +Example error for trusted-firmware-a from meta-arm:
> > +dtc version too old (039a994), you need at least version 1.4.4
> > +
> > +Upstream-Status: Inappropriate [oe specific]
> > +
> > +Signed-off-by: Peter Marko <peter.marko@siemens.com>
> > +---
> > + meson.build       | 3 ++-
> > + meson_options.txt | 2 ++
> > + 2 files changed, 4 insertions(+), 1 deletion(-)
> > +
>
> I'm not sure that is inappropriate. Has anyone talked to upstream about builds failing from shallow clones?
>
> Cheers,
>
> Richard
> 

I actually think that building from shallow clones is very non-standard scenario.
Usual builds are either from full clones or from release tarballs.
And meson nicely handles these two scenarios.
Shallow tarball is an incomplete thing which has storage benefits but also misses things that buildsystems are relying on.
And I don't think I can win a discussion with meson maintainers about this.
The vcs functions are standard and have good fallbacks (e.g. to hash)
I could maybe convince them to add option to disable fall back to git hash, but that would be a long-runner.

Maybe I could add an option to bitbake git fetcher to remove .git folder if needed?

Thanks for review,
Peter
Richard Purdie Dec. 13, 2023, 9:58 p.m. UTC | #4
On Wed, 2023-12-13 at 21:53 +0000, Marko, Peter wrote:
> -----Original Message-----
> From: Richard Purdie <richard.purdie@linuxfoundation.org> 
> Sent: Wednesday, December 13, 2023 22:11
> To: Marko, Peter (ADV D EU SK BFS1) <Peter.Marko@siemens.com>; openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core][PATCH] dtc: pass version as parameter instead of querying git
> 
> > On Wed, 2023-12-13 at 22:07 +0100, Peter Marko via lists.openembedded.org wrote:
> > > From: Peter Marko <peter.marko@siemens.com>
> > > 
> > > Since switch from Makefile to meson based build, the version is no 
> > > longer hardcoded but queried from git tag.
> > > 
> > > This works only if git history is available.
> > > When shallow tarballs are used, tag is not available.
> > > 
> > > Example error for trusted-firmware-a from meta-arm:
> > > dtc version too old (039a994), you need at least version 1.4.4
> > > 
> > > Signed-off-by: Peter Marko <peter.marko@siemens.com>
> > > ---
> > >  ...n-as-parameter-instead-of-querying-g.patch | 49 +++++++++++++++++++
> > >  meta/recipes-kernel/dtc/dtc_1.7.0.bb          |  7 ++-
> > >  2 files changed, 54 insertions(+), 2 deletions(-)  create mode 100644 
> > > meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-instea
> > > d-of-querying-g.patch
> > > 
> > > diff --git 
> > > a/meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-inst
> > > ead-of-querying-g.patch 
> > > b/meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-inst
> > > ead-of-querying-g.patch
> > > new file mode 100644
> > > index 0000000000..d7906bd6f9
> > > --- /dev/null
> > > +++ b/meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-
> > > +++ instead-of-querying-g.patch
> > > @@ -0,0 +1,49 @@
> > > +From f32056f825b926b5820d43478d11e92eee1ec91f Mon Sep 17 00:00:00 
> > > +2001
> > > +From: Peter Marko <peter.marko@siemens.com>
> > > +Date: Wed, 13 Dec 2023 21:18:14 +0100
> > > +Subject: [PATCH] meson: get version as parameter instead of querying 
> > > +git
> > > +
> > > +When using shallow tarballs, git tag is not available.
> > > +This causes version to be git hash.
> > > +This prevents application from checking minimal required version.
> > > +
> > > +Example error for trusted-firmware-a from meta-arm:
> > > +dtc version too old (039a994), you need at least version 1.4.4
> > > +
> > > +Upstream-Status: Inappropriate [oe specific]
> > > +
> > > +Signed-off-by: Peter Marko <peter.marko@siemens.com>
> > > +---
> > > + meson.build       | 3 ++-
> > > + meson_options.txt | 2 ++
> > > + 2 files changed, 4 insertions(+), 1 deletion(-)
> > > +
> > 
> > I'm not sure that is inappropriate. Has anyone talked to upstream about builds failing from shallow clones?
> > 
> > Cheers,
> > 
> > Richard
> > 
> 
> I actually think that building from shallow clones is very non-standard scenario.
> Usual builds are either from full clones or from release tarballs.
> And meson nicely handles these two scenarios.
> Shallow tarball is an incomplete thing which has storage benefits but also misses things that buildsystems are relying on.
> And I don't think I can win a discussion with meson maintainers about this.
> The vcs functions are standard and have good fallbacks (e.g. to hash)
> I could maybe convince them to add option to disable fall back to git hash, but that would be a long-runner.

They might be more receptive to adding an argument to allow the value
to be forced though, particularly if you can explain scenarios where
this is the only option and the code get it wrong?

I do think we need to raise these issues with the upstreams and only
carry patches if we can't convince them of these challenges.

> Maybe I could add an option to bitbake git fetcher to remove .git folder if needed?

I'm not sure we want to encourage it. That is actually more
maintainable even in the recipe though.

Cheers,

Richard
Peter Marko Dec. 16, 2023, 7:07 p.m. UTC | #5
-----Original Message-----
From: Richard Purdie <richard.purdie@linuxfoundation.org> 
Sent: Wednesday, December 13, 2023 22:58
To: Marko, Peter (ADV D EU SK BFS1) <Peter.Marko@siemens.com>; openembedded-core@lists.openembedded.org; Kanavin, Alexander (EXT) (Linutronix GmbH) <alex@linutronix.de>
Subject: Re: [OE-core][PATCH] dtc: pass version as parameter instead of querying git

> On Wed, 2023-12-13 at 21:53 +0000, Marko, Peter wrote:
> > 
> > I actually think that building from shallow clones is very non-standard scenario.
> > Usual builds are either from full clones or from release tarballs.
> > And meson nicely handles these two scenarios.
> > Shallow tarball is an incomplete thing which has storage benefits but also misses things that buildsystems are relying on.
> > And I don't think I can win a discussion with meson maintainers about this.
> > The vcs functions are standard and have good fallbacks (e.g. to hash) 
> > I could maybe convince them to add option to disable fall back to git hash, but that would be a long-runner.
>
> They might be more receptive to adding an argument to allow the value to be forced though, particularly if you can explain scenarios where this is the only option and the code get it wrong?
>
> I do think we need to raise these issues with the upstreams and only carry patches if we can't convince them of these challenges.
>
> > Maybe I could add an option to bitbake git fetcher to remove .git folder if needed?
>
> I'm not sure we want to encourage it. That is actually more maintainable even in the recipe though.
>
> Cheers,
>
> Richard

I reworked this completely, sent it to upstream and submitted as new patch to oe-core.
Let's see what is the reaction.

Peter
diff mbox series

Patch

diff --git a/meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-instead-of-querying-g.patch b/meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-instead-of-querying-g.patch
new file mode 100644
index 0000000000..d7906bd6f9
--- /dev/null
+++ b/meta/recipes-kernel/dtc/dtc/0001-meson-get-version-as-parameter-instead-of-querying-g.patch
@@ -0,0 +1,49 @@ 
+From f32056f825b926b5820d43478d11e92eee1ec91f Mon Sep 17 00:00:00 2001
+From: Peter Marko <peter.marko@siemens.com>
+Date: Wed, 13 Dec 2023 21:18:14 +0100
+Subject: [PATCH] meson: get version as parameter instead of querying git
+
+When using shallow tarballs, git tag is not available.
+This causes version to be git hash.
+This prevents application from checking minimal required version.
+
+Example error for trusted-firmware-a from meta-arm:
+dtc version too old (039a994), you need at least version 1.4.4
+
+Upstream-Status: Inappropriate [oe specific]
+
+Signed-off-by: Peter Marko <peter.marko@siemens.com>
+---
+ meson.build       | 3 ++-
+ meson_options.txt | 2 ++
+ 2 files changed, 4 insertions(+), 1 deletion(-)
+
+diff --git a/meson.build b/meson.build
+index 78251eb..8a25163 100644
+--- a/meson.build
++++ b/meson.build
+@@ -55,9 +55,10 @@ py = import('python')
+ py = py.find_installation(required: get_option('python'))
+ swig = find_program('swig', required: get_option('python'))
+ 
+-version_gen_h = vcs_tag(
++version_gen_h = configure_file(
+   input: 'version_gen.h.in',
+   output: 'version_gen.h',
++  configuration: { 'VCS_TAG': get_option('PV')},
+ )
+ 
+ subdir('libfdt')
+diff --git a/meson_options.txt b/meson_options.txt
+index 82621c3..c313d32 100644
+--- a/meson_options.txt
++++ b/meson_options.txt
+@@ -10,3 +10,5 @@ option('python', type: 'feature', value: 'auto',
+        description: 'Build pylibfdt Python library')
+ option('static-build', type: 'boolean', value: false,
+        description: 'Build static binaries')
++option('PV', type: 'string', value: 'undefined',
++       description: 'version string to be used instead of getting it from git')
+-- 
+2.30.2
+
diff --git a/meta/recipes-kernel/dtc/dtc_1.7.0.bb b/meta/recipes-kernel/dtc/dtc_1.7.0.bb
index 1a78a0c079..586a5f91bd 100644
--- a/meta/recipes-kernel/dtc/dtc_1.7.0.bb
+++ b/meta/recipes-kernel/dtc/dtc_1.7.0.bb
@@ -8,7 +8,10 @@  LIC_FILES_CHKSUM = "file://GPL;md5=b234ee4d69f5fce4486a80fdaf4a4263 \
                     file://BSD-2-Clause;md5=5d6306d1b08f8df623178dfd81880927 \
                     file://README.license;md5=a1eb22e37f09df5b5511b8a278992d0e"
 
-SRC_URI = "git://git.kernel.org/pub/scm/utils/dtc/dtc.git;branch=main;protocol=https" 
+SRC_URI = " \
+    git://git.kernel.org/pub/scm/utils/dtc/dtc.git;branch=main;protocol=https \
+    file://0001-meson-get-version-as-parameter-instead-of-querying-g.patch \
+"
 SRCREV = "039a99414e778332d8f9c04cbd3072e1dcc62798"
 
 UPSTREAM_CHECK_GITTAGREGEX = "v(?P<pver>\d+(\.\d+)+)"
@@ -17,7 +20,7 @@  S = "${WORKDIR}/git"
 
 inherit meson pkgconfig
 
-EXTRA_OEMESON = "-Dpython=disabled -Dvalgrind=disabled"
+EXTRA_OEMESON = "-Dpython=disabled -Dvalgrind=disabled -DPV=v${PV}"
 
 PACKAGECONFIG ??= "tools"
 PACKAGECONFIG[tools] = "-Dtools=true,-Dtools=false,flex-native bison-native"