[RFC][PATCH] tcmode-default.inc: Add LLVM_PREFERRED_VERSION

[RFC][PATCH] tcmode-default.inc: Add LLVM_PREFERRED_VERSION

[Martin Jansa]

* Allow to independently set P_V for llvm* recipes and actual llvm version used in other places

* This is needed for meta-clang which provides different llvm version from clang recipes, see:
  https://github.com/kraj/meta-clang/pull/766
  https://lists.openembedded.org/g/bitbake-devel/message/14521
* as reported in:
  https://lists.openembedded.org/g/bitbake-devel/message/14523
  the other places in kirkstone and langdale which use LLVMVERSION
  (which doesn't have to be the same as LLVM_PREFERRED_VERSION) are:

  meta/classes-recipe/meson.bbclass:llvm-config = 'llvm-config${LLVMVERSION}'
  meta/recipes-graphics/mesa/mesa.inc:MESA_LLVM_RELEASE ?= "${LLVMVERSION}"

  these were removed in mickledore, but there are other places which
  use LLVMVERSION as the actual LLVM version as reported in:
  https://github.com/kraj/meta-clang/pull/766#pullrequestreview-1349170591:

  meta-intel/dynamic-layers/clang-layer/recipes-opencl/igc/intel-graphics-compiler_1.0.12812.24.bb:                  -DIGC_OPTION__LLVM_PREFERRED_VERSION=${LLVMVERSION} \
  meta-intel/dynamic-layers/clang-layer/recipes-opencl/opencl-clang/opencl-clang_14.0.0.bb:                  -DPREFERRED_LLVM_VERSION=${LLVMVERSION} \
  meta-intel/dynamic-layers/clang-layer/recipes-opencl/opencl-clang/opencl-clang_15.0.0.bb:                  -DPREFERRED_LLVM_VERSION=${LLVMVERSION} \
  meta-intel/conf/machine/include/meta-intel.inc:PREFERRED_VERSION_opencl-clang ?= "${@bb.utils.contains('LLVMVERSION', '14.0.3', '14.0.0', '15.0.0', d)}"
  meta-intel/conf/machine/include/meta-intel.inc:PREFERRED_VERSION_opencl-clang-native ?= "${@bb.utils.contains('LLVMVERSION', '14.0.3', '14.0.0', '15.0.0', d)}"
  meta-riscv/recipes-graphics/mesa/mesa-pvr.inc:MESA_LLVM_RELEASE ?= "${LLVMVERSION}"
  meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:# Check the LLVMVERSION defined in the meta-clang layer. Given Chromium is
  meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:# developed using new C++ features, the LLVMVERSION has to be >= 12. Otherwise,
  meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:  llvm_version = d.getVar('LLVMVERSION', False)
  meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:    bb.fatal("Your LLVMVERSION (%s) is lower than the minimum required "
  meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:             "LLVMVERSION (%s). If you are using dunfell, make sure you "
  meta-clang/conf/layer.conf:LLVMVERSION = "16.0.0"
  meta-clang/recipes-devtools/spirv-llvm-translator/spirv-llvm-translator_git.bb:        -DBASE_LLVM_VERSION=${LLVMVERSION} \
  meta-clang/dynamic-layers/openembedded-layer/recipes-devtools/bcc/bcc_0.26.0.bb:    -DLLVM_PACKAGE_VERSION=${LLVMVERSION} \
  meta-clang/dynamic-layers/openembedded-layer/recipes-devtools/bpftrace/bpftrace_0.17.0.bb:    pvsplit = d.getVar('LLVMVERSION').split('.')

Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
 meta/conf/distro/include/tcmode-default.inc | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/meta/conf/distro/include/tcmode-default.inc b/meta/conf/distro/include/tcmode-default.inc
index 30408e0729..2a3074b940 100644
--- a/meta/conf/distro/include/tcmode-default.inc
+++ b/meta/conf/distro/include/tcmode-default.inc
@@ -24,7 +24,11 @@ GLIBCVERSION ?= "2.37"
 LINUXLIBCVERSION ?= "6.1%"
 QEMUVERSION ?= "7.2%"
 GOVERSION ?= "1.20%"
-LLVMVERSION ?= "15.%"
+# Allow to independently set P_V for llvm* recipes and actual llvm version used in other places
+# This is needed for meta-clang which provides different llvm version from clang recipes, see:
+# https://github.com/kraj/meta-clang/pull/766
+LLVM_PREFERRED_VERSION ?= "15.%"
+LLVMVERSION ?= "${LLVM_PREFERRED_VERSION}"
 RUSTVERSION ?= "1.67%"
 
 PREFERRED_VERSION_gcc ?= "${GCCVERSION}"
@@ -77,9 +81,9 @@ PREFERRED_VERSION_go-runtime ?= "${GOVERSION}"
 PREFERRED_VERSION_nativesdk-go ?= "${GOVERSION}"
 PREFERRED_VERSION_nativesdk-go-runtime ?= "${GOVERSION}"
 
-PREFERRED_VERSION_llvm = "${LLVMVERSION}"
-PREFERRED_VERSION_llvm-native = "${LLVMVERSION}"
-PREFERRED_VERSION_nativesdk-llvm = "${LLVMVERSION}"
+PREFERRED_VERSION_llvm = "${LLVM_PREFERRED_VERSION}"
+PREFERRED_VERSION_llvm-native = "${LLVM_PREFERRED_VERSION}"
+PREFERRED_VERSION_nativesdk-llvm = "${LLVM_PREFERRED_VERSION}"
 
 # Rust toolchain preferred versions:
 
-- 
2.40.0

Re: [OE-core] [RFC][PATCH] tcmode-default.inc: Add LLVM_PREFERRED_VERSION

[Richard Purdie]

On Sat, 2023-04-01 at 18:32 +0200, Martin Jansa wrote:
> * Allow to independently set P_V for llvm* recipes and actual llvm version used in other places
> 
> * This is needed for meta-clang which provides different llvm version from clang recipes, see:
>   https://github.com/kraj/meta-clang/pull/766
>   https://lists.openembedded.org/g/bitbake-devel/message/14521
> * as reported in:
>   https://lists.openembedded.org/g/bitbake-devel/message/14523
>   the other places in kirkstone and langdale which use LLVMVERSION
>   (which doesn't have to be the same as LLVM_PREFERRED_VERSION) are:
> 
>   meta/classes-recipe/meson.bbclass:llvm-config = 'llvm-config${LLVMVERSION}'
>   meta/recipes-graphics/mesa/mesa.inc:MESA_LLVM_RELEASE ?= "${LLVMVERSION}"
> 
>   these were removed in mickledore, but there are other places which
>   use LLVMVERSION as the actual LLVM version as reported in:
>   https://github.com/kraj/meta-clang/pull/766#pullrequestreview-1349170591:
> 
>   meta-intel/dynamic-layers/clang-layer/recipes-opencl/igc/intel-graphics-compiler_1.0.12812.24.bb:                  -DIGC_OPTION__LLVM_PREFERRED_VERSION=${LLVMVERSION} \
>   meta-intel/dynamic-layers/clang-layer/recipes-opencl/opencl-clang/opencl-clang_14.0.0.bb:                  -DPREFERRED_LLVM_VERSION=${LLVMVERSION} \
>   meta-intel/dynamic-layers/clang-layer/recipes-opencl/opencl-clang/opencl-clang_15.0.0.bb:                  -DPREFERRED_LLVM_VERSION=${LLVMVERSION} \
>   meta-intel/conf/machine/include/meta-intel.inc:PREFERRED_VERSION_opencl-clang ?= "${@bb.utils.contains('LLVMVERSION', '14.0.3', '14.0.0', '15.0.0', d)}"
>   meta-intel/conf/machine/include/meta-intel.inc:PREFERRED_VERSION_opencl-clang-native ?= "${@bb.utils.contains('LLVMVERSION', '14.0.3', '14.0.0', '15.0.0', d)}"
>   meta-riscv/recipes-graphics/mesa/mesa-pvr.inc:MESA_LLVM_RELEASE ?= "${LLVMVERSION}"
>   meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:# Check the LLVMVERSION defined in the meta-clang layer. Given Chromium is
>   meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:# developed using new C++ features, the LLVMVERSION has to be >= 12. Otherwise,
>   meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:  llvm_version = d.getVar('LLVMVERSION', False)
>   meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:    bb.fatal("Your LLVMVERSION (%s) is lower than the minimum required "
>   meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:             "LLVMVERSION (%s). If you are using dunfell, make sure you "
>   meta-clang/conf/layer.conf:LLVMVERSION = "16.0.0"
>   meta-clang/recipes-devtools/spirv-llvm-translator/spirv-llvm-translator_git.bb:        -DBASE_LLVM_VERSION=${LLVMVERSION} \
>   meta-clang/dynamic-layers/openembedded-layer/recipes-devtools/bcc/bcc_0.26.0.bb:    -DLLVM_PACKAGE_VERSION=${LLVMVERSION} \
>   meta-clang/dynamic-layers/openembedded-layer/recipes-devtools/bpftrace/bpftrace_0.17.0.bb:    pvsplit = d.getVar('LLVMVERSION').split('.')
> 
> Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> ---
>  meta/conf/distro/include/tcmode-default.inc | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/meta/conf/distro/include/tcmode-default.inc b/meta/conf/distro/include/tcmode-default.inc
> index 30408e0729..2a3074b940 100644
> --- a/meta/conf/distro/include/tcmode-default.inc
> +++ b/meta/conf/distro/include/tcmode-default.inc
> @@ -24,7 +24,11 @@ GLIBCVERSION ?= "2.37"
>  LINUXLIBCVERSION ?= "6.1%"
>  QEMUVERSION ?= "7.2%"
>  GOVERSION ?= "1.20%"
> -LLVMVERSION ?= "15.%"
> +# Allow to independently set P_V for llvm* recipes and actual llvm version used in other places
> +# This is needed for meta-clang which provides different llvm version from clang recipes, see:
> +# https://github.com/kraj/meta-clang/pull/766
> +LLVM_PREFERRED_VERSION ?= "15.%"
> +LLVMVERSION ?= "${LLVM_PREFERRED_VERSION}"
>  RUSTVERSION ?= "1.67%"
> 

I don't think someone coming to this is going to understand whether
they should set LLVMVERSION or LLVM_PREFERRED_VERSION.

I also don't think someone is going to know which one to use if they
ever do reference an llvm version in a recipe.

Can we find a more descriptive name for one of them that would make
things clearer?

It doesn't help that the commit message says "read this, then read
this, then read this, then things might make sense". Commit messages
are meant to explain things in their own right. Linking to things for
more information is fine but this is relying on that far too much.

Cheers,

Richard

Re: [OE-core] [RFC][PATCH] tcmode-default.inc: Add LLVM_PREFERRED_VERSION

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 6126 bytes --]

On Sat, Apr 1, 2023 at 9:29 PM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Sat, 2023-04-01 at 18:32 +0200, Martin Jansa wrote:
> > * Allow to independently set P_V for llvm* recipes and actual llvm
> version used in other places
> >
> > * This is needed for meta-clang which provides different llvm version
> from clang recipes, see:
> >   https://github.com/kraj/meta-clang/pull/766
> >   https://lists.openembedded.org/g/bitbake-devel/message/14521
> > * as reported in:
> >   https://lists.openembedded.org/g/bitbake-devel/message/14523
> >   the other places in kirkstone and langdale which use LLVMVERSION
> >   (which doesn't have to be the same as LLVM_PREFERRED_VERSION) are:
> >
> >   meta/classes-recipe/meson.bbclass:llvm-config =
> 'llvm-config${LLVMVERSION}'
> >   meta/recipes-graphics/mesa/mesa.inc:MESA_LLVM_RELEASE ?=
> "${LLVMVERSION}"
> >
> >   these were removed in mickledore, but there are other places which
> >   use LLVMVERSION as the actual LLVM version as reported in:
> >
> https://github.com/kraj/meta-clang/pull/766#pullrequestreview-1349170591:
> >
> >   meta-intel/dynamic-layers/clang-layer/recipes-opencl/igc/
> intel-graphics-compiler_1.0.12812.24.bb:
> -DIGC_OPTION__LLVM_PREFERRED_VERSION=${LLVMVERSION} \
> >   meta-intel/dynamic-layers/clang-layer/recipes-opencl/opencl-clang/
> opencl-clang_14.0.0.bb:
> -DPREFERRED_LLVM_VERSION=${LLVMVERSION} \
> >   meta-intel/dynamic-layers/clang-layer/recipes-opencl/opencl-clang/
> opencl-clang_15.0.0.bb:
> -DPREFERRED_LLVM_VERSION=${LLVMVERSION} \
> >
>  meta-intel/conf/machine/include/meta-intel.inc:PREFERRED_VERSION_opencl-clang
> ?= "${@bb.utils.contains('LLVMVERSION', '14.0.3', '14.0.0', '15.0.0', d)}"
> >
>  meta-intel/conf/machine/include/meta-intel.inc:PREFERRED_VERSION_opencl-clang-native
> ?= "${@bb.utils.contains('LLVMVERSION', '14.0.3', '14.0.0', '15.0.0', d)}"
> >   meta-riscv/recipes-graphics/mesa/mesa-pvr.inc:MESA_LLVM_RELEASE ?=
> "${LLVMVERSION}"
> >   meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:#
> Check the LLVMVERSION defined in the meta-clang layer. Given Chromium is
> >   meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:#
> developed using new C++ features, the LLVMVERSION has to be >= 12.
> Otherwise,
> >   meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:
> llvm_version = d.getVar('LLVMVERSION', False)
> >   meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:
>   bb.fatal("Your LLVMVERSION (%s) is lower than the minimum required "
> >   meta-browser/meta-chromium/recipes-browser/chromium/chromium-gn.inc:
>            "LLVMVERSION (%s). If you are using dunfell, make sure you "
> >   meta-clang/conf/layer.conf:LLVMVERSION = "16.0.0"
> >   meta-clang/recipes-devtools/spirv-llvm-translator/
> spirv-llvm-translator_git.bb:        -DBASE_LLVM_VERSION=${LLVMVERSION} \
> >   meta-clang/dynamic-layers/openembedded-layer/recipes-devtools/bcc/
> bcc_0.26.0.bb:    -DLLVM_PACKAGE_VERSION=${LLVMVERSION} \
> >   meta-clang/dynamic-layers/openembedded-layer/recipes-devtools/bpftrace/
> bpftrace_0.17.0.bb:    pvsplit = d.getVar('LLVMVERSION').split('.')
> >
> > Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> > ---
> >  meta/conf/distro/include/tcmode-default.inc | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/meta/conf/distro/include/tcmode-default.inc
> b/meta/conf/distro/include/tcmode-default.inc
> > index 30408e0729..2a3074b940 100644
> > --- a/meta/conf/distro/include/tcmode-default.inc
> > +++ b/meta/conf/distro/include/tcmode-default.inc
> > @@ -24,7 +24,11 @@ GLIBCVERSION ?= "2.37"
> >  LINUXLIBCVERSION ?= "6.1%"
> >  QEMUVERSION ?= "7.2%"
> >  GOVERSION ?= "1.20%"
> > -LLVMVERSION ?= "15.%"
> > +# Allow to independently set P_V for llvm* recipes and actual llvm
> version used in other places
> > +# This is needed for meta-clang which provides different llvm version
> from clang recipes, see:
> > +# https://github.com/kraj/meta-clang/pull/766
> > +LLVM_PREFERRED_VERSION ?= "15.%"
> > +LLVMVERSION ?= "${LLVM_PREFERRED_VERSION}"
> >  RUSTVERSION ?= "1.67%"
> >
>
> I don't think someone coming to this is going to understand whether
> they should set LLVMVERSION or LLVM_PREFERRED_VERSION.
>
> I also don't think someone is going to know which one to use if they
> ever do reference an llvm version in a recipe.
>
> Can we find a more descriptive name for one of them that would make
> things clearer?
>

I'm open to suggestions (and one of the reasons this was sent as RFC),
LLVM_PREFERRED_VERSION was the best one I found.

It doesn't help that the commit message says "read this, then read
> this, then read this, then things might make sense". Commit messages
> are meant to explain things in their own right. Linking to things for
> more information is fine but this is relying on that far too much.


The first line explains why this is needed, the rest are "details" and from
those referenced links I've tried to copy the most relevant parts.

I was confused at first why warning about P_V didn't work in this case,
that's why I've shared much longer description on bitbake-devel and then
meta-clang PR. Now after reminding myself how P_V works and why it needs to
behave like that, it's quite clear to me what was happening - and I really
don't know if someone else would need the same P_V refresh.

Is this better?

LLVM_PREFERRED_VERSION is used only to set PREFERRED_VERSION of llvm
recipes and you might need to change it only if your layer provides another
set of llvm recipes with different version.

LLVMVERSION is the actual version of LLVM used during the build (and it can
be different from llvm recipes, e.g. when LLVM is provided by clang recipes
based on PREFERRED_PROVIDER setting and meta-clang already sets LLVMVERSION
in layer.conf).

PREFERRED_VERSION_llvm doesn't affect version of clang recipes even when
clang provides llvm, only PREFERRED_VERSION_clang would affect that.

[-- Attachment #2: Type: text/html, Size: 8434 bytes --]