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

[Richard Purdie <richard.purdie@linuxfoundation.org>]

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