| Message ID | 20230926083057.47067-1-andrea.righi@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,M,1/1] UBUNTU: [Packaging] define Rust flags only with Rust-enabled kernels | expand |
On Tue, 26 Sept 2023 at 09:32, Andrea Righi <andrea.righi@canonical.com> wrote: > > To enable Rust support in the kernel we need to pass special build > flags, that are used to select the proper Rust toolchain version and > dependent components. > > These flags are passed also to the scripts called during the > updateconfigs step and the build, such as 'kernelconfig', using the > $(kmake) variable. > > However, $(kmake) can also contain cross-compiler variables that may > potentially affect the config or the build in certain architectures. Not potentially, but intentionally. Because sometimes we use different gcc for different arch (i.e. gcc-11 for armhf, and gcc-12 for arm64 build), and we must use full kmake for generating the configs that should be the same as the one that will be used for build. I don't see how this will produce correct results. Excluding rust variables is good, but the gcc and crosscompile should remain in place imho. > > For this reason, avoid using kmake during 'updateconfigs' and explicitly > pass the Rust flags using a separate environment variable that will be > defined only for Rust-enabled kernels (do_lib_rust=true). > > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > --- > debian/rules.d/0-common-vars.mk | 2 -- > debian/rules.d/1-maintainer.mk | 2 +- > debian/rules.d/2-binary-arch.mk | 16 ++++++++++++++++ > debian/scripts/misc/kernelconfig | 2 +- > 4 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/debian/rules.d/0-common-vars.mk b/debian/rules.d/0-common-vars.mk > index 410de315afba..d62a9b063241 100644 > --- a/debian/rules.d/0-common-vars.mk > +++ b/debian/rules.d/0-common-vars.mk > @@ -238,8 +238,6 @@ kmake = make ARCH=$(build_arch) \ > CROSS_COMPILE=$(CROSS_COMPILE) \ > HOSTCC=$(HOSTCC) \ > CC=$(CROSS_COMPILE)$(gcc) \ > - RUSTC=rustc-1.68 HOSTRUSTC=rustc-1.68 BINDGEN=bindgen-0.56 RUSTFMT=/usr/lib/rust-1.68/bin/rustfmt RUST_LIB_SRC=/usr/src/rustc-1.68.2/library \ > - CLANG_PATH=clang-15 LIBCLANG_PATH=/usr/lib/llvm-15/lib \ > KERNELVERSION=$(abi_release)-$(target_flavour) \ > CONFIG_DEBUG_SECTION_MISMATCH=y \ > KBUILD_BUILD_VERSION="$(uploadnum)" \ > diff --git a/debian/rules.d/1-maintainer.mk b/debian/rules.d/1-maintainer.mk > index 839e93a15371..a72e5fe93784 100644 > --- a/debian/rules.d/1-maintainer.mk > +++ b/debian/rules.d/1-maintainer.mk > @@ -29,7 +29,7 @@ configs-targets := updateconfigs defaultconfigs genconfigs editconfigs > .PHONY: $(configs-targets) > $(configs-targets): > dh_testdir > - kmake='$(kmake)' skip_checks=$(do_skip_checks) conc_level=$(conc_level) \ > + RUST_FLAGS='$(RUST_FLAGS)' skip_checks=$(do_skip_checks) conc_level=$(conc_level) \ > $(SHELL) $(DROOT)/scripts/misc/kernelconfig $@ > > .PHONY: printenv > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > index 53fbb55c198a..6e9c78aadb45 100644 > --- a/debian/rules.d/2-binary-arch.mk > +++ b/debian/rules.d/2-binary-arch.mk > @@ -10,6 +10,22 @@ build_cd = > build_O = O=$(builddir)/build-$* > endif > > +# Define Rust build flags only for Rust-enabled kernels > +ifeq ($(do_lib_rust),true) > +RUST_FLAGS = \ > + RUSTC=rustc-1.68 \ > + HOSTRUSTC=rustc-1.68 \ > + BINDGEN=bindgen-0.56 \ > + RUSTFMT=/usr/lib/rust-1.68/bin/rustfmt \ > + RUST_LIB_SRC=/usr/src/rustc-1.68.2/library \ > + CLANG_PATH=clang-15 \ > + LIBCLANG_PATH=/usr/lib/llvm-15/lib > +else > +RUST_FLAGS= > +endif > + > +kmake += $(RUST_FLAGS) > + > # TODO this is probably wrong, and should be using $(DEB_HOST_MULTIARCH) > shlibdeps_opts = $(if $(CROSS_COMPILE),-- -l$(CROSS_COMPILE:%-=/usr/%)/lib) > > diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig > index 035426762f8a..741650340b65 100755 > --- a/debian/scripts/misc/kernelconfig > +++ b/debian/scripts/misc/kernelconfig > @@ -118,7 +118,7 @@ EOF > # Call config target > echo > echo "* Run ${target} on ${arch}/${flavour} ..." > - ${kmake} O=build "${env[@]}" "${target}" > + make ${RUST_FLAGS} O=build "${env[@]}" "${target}" > > # Move config for further processing > mv build/.config "${tmp_conf_file}" > -- > 2.40.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On Tue, Sep 26, 2023 at 11:27:43AM +0100, Dimitri John Ledkov wrote: > On Tue, 26 Sept 2023 at 09:32, Andrea Righi <andrea.righi@canonical.com> wrote: > > > > To enable Rust support in the kernel we need to pass special build > > flags, that are used to select the proper Rust toolchain version and > > dependent components. > > > > These flags are passed also to the scripts called during the > > updateconfigs step and the build, such as 'kernelconfig', using the > > $(kmake) variable. > > > > However, $(kmake) can also contain cross-compiler variables that may > > potentially affect the config or the build in certain architectures. > > Not potentially, but intentionally. > > Because sometimes we use different gcc for different arch (i.e. gcc-11 > for armhf, and gcc-12 for arm64 build), and we must use full kmake for > generating the configs that should be the same as the one that will be > used for build. This is a valid point, I think there's still value to isolate the Rust options in a separate variable and use them only with kernels that need to produce linux-lib-rust, in this way we can also finally drop the ugly SAUCE patch: 29e85bc64ffc ("UBUNTU: SAUCE: enforce rust availability only on x86_64") I'll send a v2 and for now we will keep using kmake during updateconfigs. Thanks, -Andrea > I don't see how this will produce correct results. Excluding rust > variables is good, but the gcc and crosscompile should remain in place > imho. > > > > > > For this reason, avoid using kmake during 'updateconfigs' and explicitly > > pass the Rust flags using a separate environment variable that will be > > defined only for Rust-enabled kernels (do_lib_rust=true). > > > > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > > --- > > debian/rules.d/0-common-vars.mk | 2 -- > > debian/rules.d/1-maintainer.mk | 2 +- > > debian/rules.d/2-binary-arch.mk | 16 ++++++++++++++++ > > debian/scripts/misc/kernelconfig | 2 +- > > 4 files changed, 18 insertions(+), 4 deletions(-) > > > > diff --git a/debian/rules.d/0-common-vars.mk b/debian/rules.d/0-common-vars.mk > > index 410de315afba..d62a9b063241 100644 > > --- a/debian/rules.d/0-common-vars.mk > > +++ b/debian/rules.d/0-common-vars.mk > > @@ -238,8 +238,6 @@ kmake = make ARCH=$(build_arch) \ > > CROSS_COMPILE=$(CROSS_COMPILE) \ > > HOSTCC=$(HOSTCC) \ > > CC=$(CROSS_COMPILE)$(gcc) \ > > - RUSTC=rustc-1.68 HOSTRUSTC=rustc-1.68 BINDGEN=bindgen-0.56 RUSTFMT=/usr/lib/rust-1.68/bin/rustfmt RUST_LIB_SRC=/usr/src/rustc-1.68.2/library \ > > - CLANG_PATH=clang-15 LIBCLANG_PATH=/usr/lib/llvm-15/lib \ > > KERNELVERSION=$(abi_release)-$(target_flavour) \ > > CONFIG_DEBUG_SECTION_MISMATCH=y \ > > KBUILD_BUILD_VERSION="$(uploadnum)" \ > > diff --git a/debian/rules.d/1-maintainer.mk b/debian/rules.d/1-maintainer.mk > > index 839e93a15371..a72e5fe93784 100644 > > --- a/debian/rules.d/1-maintainer.mk > > +++ b/debian/rules.d/1-maintainer.mk > > @@ -29,7 +29,7 @@ configs-targets := updateconfigs defaultconfigs genconfigs editconfigs > > .PHONY: $(configs-targets) > > $(configs-targets): > > dh_testdir > > - kmake='$(kmake)' skip_checks=$(do_skip_checks) conc_level=$(conc_level) \ > > + RUST_FLAGS='$(RUST_FLAGS)' skip_checks=$(do_skip_checks) conc_level=$(conc_level) \ > > $(SHELL) $(DROOT)/scripts/misc/kernelconfig $@ > > > > .PHONY: printenv > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > > index 53fbb55c198a..6e9c78aadb45 100644 > > --- a/debian/rules.d/2-binary-arch.mk > > +++ b/debian/rules.d/2-binary-arch.mk > > @@ -10,6 +10,22 @@ build_cd = > > build_O = O=$(builddir)/build-$* > > endif > > > > +# Define Rust build flags only for Rust-enabled kernels > > +ifeq ($(do_lib_rust),true) > > +RUST_FLAGS = \ > > + RUSTC=rustc-1.68 \ > > + HOSTRUSTC=rustc-1.68 \ > > + BINDGEN=bindgen-0.56 \ > > + RUSTFMT=/usr/lib/rust-1.68/bin/rustfmt \ > > + RUST_LIB_SRC=/usr/src/rustc-1.68.2/library \ > > + CLANG_PATH=clang-15 \ > > + LIBCLANG_PATH=/usr/lib/llvm-15/lib > > +else > > +RUST_FLAGS= > > +endif > > + > > +kmake += $(RUST_FLAGS) > > + > > # TODO this is probably wrong, and should be using $(DEB_HOST_MULTIARCH) > > shlibdeps_opts = $(if $(CROSS_COMPILE),-- -l$(CROSS_COMPILE:%-=/usr/%)/lib) > > > > diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig > > index 035426762f8a..741650340b65 100755 > > --- a/debian/scripts/misc/kernelconfig > > +++ b/debian/scripts/misc/kernelconfig > > @@ -118,7 +118,7 @@ EOF > > # Call config target > > echo > > echo "* Run ${target} on ${arch}/${flavour} ..." > > - ${kmake} O=build "${env[@]}" "${target}" > > + make ${RUST_FLAGS} O=build "${env[@]}" "${target}" > > > > # Move config for further processing > > mv build/.config "${tmp_conf_file}" > > -- > > 2.40.1 > > > > > > -- > > kernel-team mailing list > > kernel-team@lists.ubuntu.com > > https://lists.ubuntu.com/mailman/listinfo/kernel-team > > > > -- > okurrr, > > Dimitri
On Tue, 26 Sep 2023 11:27:43 +0100 Dimitri John Ledkov <dimitri.ledkov@canonical.com> wrote: > On Tue, 26 Sept 2023 at 09:32, Andrea Righi <andrea.righi@canonical.com> wrote: > > > > To enable Rust support in the kernel we need to pass special build > > flags, that are used to select the proper Rust toolchain version and > > dependent components. > > > > These flags are passed also to the scripts called during the > > updateconfigs step and the build, such as 'kernelconfig', using the > > $(kmake) variable. > > > > However, $(kmake) can also contain cross-compiler variables that may > > potentially affect the config or the build in certain architectures. > > Not potentially, but intentionally. > > Because sometimes we use different gcc for different arch (i.e. gcc-11 > for armhf, and gcc-12 for arm64 build), and we must use full kmake for > generating the configs that should be the same as the one that will be > used for build. > > I don't see how this will produce correct results. Excluding rust > variables is good, but the gcc and crosscompile should remain in place > imho. gcc is set separately in the kernelconfig script (also ugly) and so are ARCH and CROSS_COMPILE. We used to use 'plain' make in kernelconfig until Andrea switched to kmake for Rust. We do not need kmake in kernelconfig. But yes, best to use kmake everywhere but that's a bigger change. ...Juerg > > > > > For this reason, avoid using kmake during 'updateconfigs' and explicitly > > pass the Rust flags using a separate environment variable that will be > > defined only for Rust-enabled kernels (do_lib_rust=true). > > > > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > > --- > > debian/rules.d/0-common-vars.mk | 2 -- > > debian/rules.d/1-maintainer.mk | 2 +- > > debian/rules.d/2-binary-arch.mk | 16 ++++++++++++++++ > > debian/scripts/misc/kernelconfig | 2 +- > > 4 files changed, 18 insertions(+), 4 deletions(-) > > > > diff --git a/debian/rules.d/0-common-vars.mk b/debian/rules.d/0-common-vars.mk > > index 410de315afba..d62a9b063241 100644 > > --- a/debian/rules.d/0-common-vars.mk > > +++ b/debian/rules.d/0-common-vars.mk > > @@ -238,8 +238,6 @@ kmake = make ARCH=$(build_arch) \ > > CROSS_COMPILE=$(CROSS_COMPILE) \ > > HOSTCC=$(HOSTCC) \ > > CC=$(CROSS_COMPILE)$(gcc) \ > > - RUSTC=rustc-1.68 HOSTRUSTC=rustc-1.68 BINDGEN=bindgen-0.56 RUSTFMT=/usr/lib/rust-1.68/bin/rustfmt RUST_LIB_SRC=/usr/src/rustc-1.68.2/library \ > > - CLANG_PATH=clang-15 LIBCLANG_PATH=/usr/lib/llvm-15/lib \ > > KERNELVERSION=$(abi_release)-$(target_flavour) \ > > CONFIG_DEBUG_SECTION_MISMATCH=y \ > > KBUILD_BUILD_VERSION="$(uploadnum)" \ > > diff --git a/debian/rules.d/1-maintainer.mk b/debian/rules.d/1-maintainer.mk > > index 839e93a15371..a72e5fe93784 100644 > > --- a/debian/rules.d/1-maintainer.mk > > +++ b/debian/rules.d/1-maintainer.mk > > @@ -29,7 +29,7 @@ configs-targets := updateconfigs defaultconfigs genconfigs editconfigs > > .PHONY: $(configs-targets) > > $(configs-targets): > > dh_testdir > > - kmake='$(kmake)' skip_checks=$(do_skip_checks) conc_level=$(conc_level) \ > > + RUST_FLAGS='$(RUST_FLAGS)' skip_checks=$(do_skip_checks) conc_level=$(conc_level) \ > > $(SHELL) $(DROOT)/scripts/misc/kernelconfig $@ > > > > .PHONY: printenv > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > > index 53fbb55c198a..6e9c78aadb45 100644 > > --- a/debian/rules.d/2-binary-arch.mk > > +++ b/debian/rules.d/2-binary-arch.mk > > @@ -10,6 +10,22 @@ build_cd = > > build_O = O=$(builddir)/build-$* > > endif > > > > +# Define Rust build flags only for Rust-enabled kernels > > +ifeq ($(do_lib_rust),true) > > +RUST_FLAGS = \ > > + RUSTC=rustc-1.68 \ > > + HOSTRUSTC=rustc-1.68 \ > > + BINDGEN=bindgen-0.56 \ > > + RUSTFMT=/usr/lib/rust-1.68/bin/rustfmt \ > > + RUST_LIB_SRC=/usr/src/rustc-1.68.2/library \ > > + CLANG_PATH=clang-15 \ > > + LIBCLANG_PATH=/usr/lib/llvm-15/lib > > +else > > +RUST_FLAGS= > > +endif > > + > > +kmake += $(RUST_FLAGS) > > + > > # TODO this is probably wrong, and should be using $(DEB_HOST_MULTIARCH) > > shlibdeps_opts = $(if $(CROSS_COMPILE),-- -l$(CROSS_COMPILE:%-=/usr/%)/lib) > > > > diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig > > index 035426762f8a..741650340b65 100755 > > --- a/debian/scripts/misc/kernelconfig > > +++ b/debian/scripts/misc/kernelconfig > > @@ -118,7 +118,7 @@ EOF > > # Call config target > > echo > > echo "* Run ${target} on ${arch}/${flavour} ..." > > - ${kmake} O=build "${env[@]}" "${target}" > > + make ${RUST_FLAGS} O=build "${env[@]}" "${target}" > > > > # Move config for further processing > > mv build/.config "${tmp_conf_file}" > > -- > > 2.40.1 > > > > > > -- > > kernel-team mailing list > > kernel-team@lists.ubuntu.com > > https://lists.ubuntu.com/mailman/listinfo/kernel-team > > >
On Tue, 26 Sep 2023 12:52:25 +0200 Andrea Righi <andrea.righi@canonical.com> wrote: > On Tue, Sep 26, 2023 at 11:27:43AM +0100, Dimitri John Ledkov wrote: > > On Tue, 26 Sept 2023 at 09:32, Andrea Righi <andrea.righi@canonical.com> wrote: > > > > > > To enable Rust support in the kernel we need to pass special build > > > flags, that are used to select the proper Rust toolchain version and > > > dependent components. > > > > > > These flags are passed also to the scripts called during the > > > updateconfigs step and the build, such as 'kernelconfig', using the > > > $(kmake) variable. > > > > > > However, $(kmake) can also contain cross-compiler variables that may > > > potentially affect the config or the build in certain architectures. > > > > Not potentially, but intentionally. > > > > Because sometimes we use different gcc for different arch (i.e. gcc-11 > > for armhf, and gcc-12 for arm64 build), and we must use full kmake for > > generating the configs that should be the same as the one that will be > > used for build. > > This is a valid point, I think there's still value to isolate the Rust > options in a separate variable Why a separate variable? Isn't it enough to simply add them to kmake for rust-enabled kernels? ...Juerg > and use them only with kernels that need > to produce linux-lib-rust, in this way we can also finally drop the ugly > SAUCE patch: > > 29e85bc64ffc ("UBUNTU: SAUCE: enforce rust availability only on x86_64") > > I'll send a v2 and for now we will keep using kmake during > updateconfigs. > > Thanks, > -Andrea > > > I don't see how this will produce correct results. Excluding rust > > variables is good, but the gcc and crosscompile should remain in place > > imho. > > > > > > > > > > For this reason, avoid using kmake during 'updateconfigs' and explicitly > > > pass the Rust flags using a separate environment variable that will be > > > defined only for Rust-enabled kernels (do_lib_rust=true). > > > > > > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > > > --- > > > debian/rules.d/0-common-vars.mk | 2 -- > > > debian/rules.d/1-maintainer.mk | 2 +- > > > debian/rules.d/2-binary-arch.mk | 16 ++++++++++++++++ > > > debian/scripts/misc/kernelconfig | 2 +- > > > 4 files changed, 18 insertions(+), 4 deletions(-) > > > > > > diff --git a/debian/rules.d/0-common-vars.mk b/debian/rules.d/0-common-vars.mk > > > index 410de315afba..d62a9b063241 100644 > > > --- a/debian/rules.d/0-common-vars.mk > > > +++ b/debian/rules.d/0-common-vars.mk > > > @@ -238,8 +238,6 @@ kmake = make ARCH=$(build_arch) \ > > > CROSS_COMPILE=$(CROSS_COMPILE) \ > > > HOSTCC=$(HOSTCC) \ > > > CC=$(CROSS_COMPILE)$(gcc) \ > > > - RUSTC=rustc-1.68 HOSTRUSTC=rustc-1.68 BINDGEN=bindgen-0.56 RUSTFMT=/usr/lib/rust-1.68/bin/rustfmt RUST_LIB_SRC=/usr/src/rustc-1.68.2/library \ > > > - CLANG_PATH=clang-15 LIBCLANG_PATH=/usr/lib/llvm-15/lib \ > > > KERNELVERSION=$(abi_release)-$(target_flavour) \ > > > CONFIG_DEBUG_SECTION_MISMATCH=y \ > > > KBUILD_BUILD_VERSION="$(uploadnum)" \ > > > diff --git a/debian/rules.d/1-maintainer.mk b/debian/rules.d/1-maintainer.mk > > > index 839e93a15371..a72e5fe93784 100644 > > > --- a/debian/rules.d/1-maintainer.mk > > > +++ b/debian/rules.d/1-maintainer.mk > > > @@ -29,7 +29,7 @@ configs-targets := updateconfigs defaultconfigs genconfigs editconfigs > > > .PHONY: $(configs-targets) > > > $(configs-targets): > > > dh_testdir > > > - kmake='$(kmake)' skip_checks=$(do_skip_checks) conc_level=$(conc_level) \ > > > + RUST_FLAGS='$(RUST_FLAGS)' skip_checks=$(do_skip_checks) conc_level=$(conc_level) \ > > > $(SHELL) $(DROOT)/scripts/misc/kernelconfig $@ > > > > > > .PHONY: printenv > > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > > > index 53fbb55c198a..6e9c78aadb45 100644 > > > --- a/debian/rules.d/2-binary-arch.mk > > > +++ b/debian/rules.d/2-binary-arch.mk > > > @@ -10,6 +10,22 @@ build_cd = > > > build_O = O=$(builddir)/build-$* > > > endif > > > > > > +# Define Rust build flags only for Rust-enabled kernels > > > +ifeq ($(do_lib_rust),true) > > > +RUST_FLAGS = \ > > > + RUSTC=rustc-1.68 \ > > > + HOSTRUSTC=rustc-1.68 \ > > > + BINDGEN=bindgen-0.56 \ > > > + RUSTFMT=/usr/lib/rust-1.68/bin/rustfmt \ > > > + RUST_LIB_SRC=/usr/src/rustc-1.68.2/library \ > > > + CLANG_PATH=clang-15 \ > > > + LIBCLANG_PATH=/usr/lib/llvm-15/lib > > > +else > > > +RUST_FLAGS= > > > +endif > > > + > > > +kmake += $(RUST_FLAGS) > > > + > > > # TODO this is probably wrong, and should be using $(DEB_HOST_MULTIARCH) > > > shlibdeps_opts = $(if $(CROSS_COMPILE),-- -l$(CROSS_COMPILE:%-=/usr/%)/lib) > > > > > > diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig > > > index 035426762f8a..741650340b65 100755 > > > --- a/debian/scripts/misc/kernelconfig > > > +++ b/debian/scripts/misc/kernelconfig > > > @@ -118,7 +118,7 @@ EOF > > > # Call config target > > > echo > > > echo "* Run ${target} on ${arch}/${flavour} ..." > > > - ${kmake} O=build "${env[@]}" "${target}" > > > + make ${RUST_FLAGS} O=build "${env[@]}" "${target}" > > > > > > # Move config for further processing > > > mv build/.config "${tmp_conf_file}" > > > -- > > > 2.40.1 > > > > > > > > > -- > > > kernel-team mailing list > > > kernel-team@lists.ubuntu.com > > > https://lists.ubuntu.com/mailman/listinfo/kernel-team > > > > > > > > -- > > okurrr, > > > > Dimitri >
On Tue, Sep 26, 2023 at 02:16:17PM +0200, Juerg Haefliger wrote: > On Tue, 26 Sep 2023 12:52:25 +0200 > Andrea Righi <andrea.righi@canonical.com> wrote: > > > On Tue, Sep 26, 2023 at 11:27:43AM +0100, Dimitri John Ledkov wrote: > > > On Tue, 26 Sept 2023 at 09:32, Andrea Righi <andrea.righi@canonical.com> wrote: > > > > > > > > To enable Rust support in the kernel we need to pass special build > > > > flags, that are used to select the proper Rust toolchain version and > > > > dependent components. > > > > > > > > These flags are passed also to the scripts called during the > > > > updateconfigs step and the build, such as 'kernelconfig', using the > > > > $(kmake) variable. > > > > > > > > However, $(kmake) can also contain cross-compiler variables that may > > > > potentially affect the config or the build in certain architectures. > > > > > > Not potentially, but intentionally. > > > > > > Because sometimes we use different gcc for different arch (i.e. gcc-11 > > > for armhf, and gcc-12 for arm64 build), and we must use full kmake for > > > generating the configs that should be the same as the one that will be > > > used for build. > > > > This is a valid point, I think there's still value to isolate the Rust > > options in a separate variable > > Why a separate variable? Isn't it enough to simply add them to kmake for > rust-enabled kernels? The amount of env variables can change between a kernel version and another (we may need to foce a specific clang/libclang, or host rustc, or potentially other dependencies required by Rust), that's why I'd prefer to isolate these extra options rather than mixing them into kmake directly. More in general, I'd really love if we could evaluate all these options at build time, instead of config time... There is this trend now in the kernel to add scripts used by Kconfig to probe system tools and libs at config time and set the config options accordingly. That is quite bad for us, because our config phase is decoupled from the build phase and often the config environment is different than the build environment (especially with devel kernels). I think it would be much better for us to have the possibility (special option?) to say "enable whatever I need, without checking what's installed in my system" and then if something is missing at build time, simply let the build fail. -Andrea
On Tue, 26 Sept 2023 at 13:14, Juerg Haefliger <juerg.haefliger@canonical.com> wrote: > > On Tue, 26 Sep 2023 11:27:43 +0100 > Dimitri John Ledkov <dimitri.ledkov@canonical.com> wrote: > > > On Tue, 26 Sept 2023 at 09:32, Andrea Righi <andrea.righi@canonical.com> wrote: > > > > > > To enable Rust support in the kernel we need to pass special build > > > flags, that are used to select the proper Rust toolchain version and > > > dependent components. > > > > > > These flags are passed also to the scripts called during the > > > updateconfigs step and the build, such as 'kernelconfig', using the > > > $(kmake) variable. > > > > > > However, $(kmake) can also contain cross-compiler variables that may > > > potentially affect the config or the build in certain architectures. > > > > Not potentially, but intentionally. > > > > Because sometimes we use different gcc for different arch (i.e. gcc-11 > > for armhf, and gcc-12 for arm64 build), and we must use full kmake for > > generating the configs that should be the same as the one that will be > > used for build. > > > > I don't see how this will produce correct results. Excluding rust > > variables is good, but the gcc and crosscompile should remain in place > > imho. > > gcc is set separately in the kernelconfig script (also ugly) and so are ARCH > and CROSS_COMPILE. We used to use 'plain' make in kernelconfig until Andrea > switched to kmake for Rust. We do not need kmake in kernelconfig. But yes, > best to use kmake everywhere but that's a bigger change. > I do agree here that ideally we don't need all of those tools installed, and we should be able to produce correct config always. Thus I guess the next cycle goal is to fake upstream fake tools to actually produce valid configs by default as needed by us. Meaning adding fake tools for rust, llvm, and anything else not already covered there. Because a chroot without GCC should be able to still produce valid config for us for all arches. > ...Juerg > > > > > > > > > > For this reason, avoid using kmake during 'updateconfigs' and explicitly > > > pass the Rust flags using a separate environment variable that will be > > > defined only for Rust-enabled kernels (do_lib_rust=true). > > > > > > Signed-off-by: Andrea Righi <andrea.righi@canonical.com> > > > --- > > > debian/rules.d/0-common-vars.mk | 2 -- > > > debian/rules.d/1-maintainer.mk | 2 +- > > > debian/rules.d/2-binary-arch.mk | 16 ++++++++++++++++ > > > debian/scripts/misc/kernelconfig | 2 +- > > > 4 files changed, 18 insertions(+), 4 deletions(-) > > > > > > diff --git a/debian/rules.d/0-common-vars.mk b/debian/rules.d/0-common-vars.mk > > > index 410de315afba..d62a9b063241 100644 > > > --- a/debian/rules.d/0-common-vars.mk > > > +++ b/debian/rules.d/0-common-vars.mk > > > @@ -238,8 +238,6 @@ kmake = make ARCH=$(build_arch) \ > > > CROSS_COMPILE=$(CROSS_COMPILE) \ > > > HOSTCC=$(HOSTCC) \ > > > CC=$(CROSS_COMPILE)$(gcc) \ > > > - RUSTC=rustc-1.68 HOSTRUSTC=rustc-1.68 BINDGEN=bindgen-0.56 RUSTFMT=/usr/lib/rust-1.68/bin/rustfmt RUST_LIB_SRC=/usr/src/rustc-1.68.2/library \ > > > - CLANG_PATH=clang-15 LIBCLANG_PATH=/usr/lib/llvm-15/lib \ > > > KERNELVERSION=$(abi_release)-$(target_flavour) \ > > > CONFIG_DEBUG_SECTION_MISMATCH=y \ > > > KBUILD_BUILD_VERSION="$(uploadnum)" \ > > > diff --git a/debian/rules.d/1-maintainer.mk b/debian/rules.d/1-maintainer.mk > > > index 839e93a15371..a72e5fe93784 100644 > > > --- a/debian/rules.d/1-maintainer.mk > > > +++ b/debian/rules.d/1-maintainer.mk > > > @@ -29,7 +29,7 @@ configs-targets := updateconfigs defaultconfigs genconfigs editconfigs > > > .PHONY: $(configs-targets) > > > $(configs-targets): > > > dh_testdir > > > - kmake='$(kmake)' skip_checks=$(do_skip_checks) conc_level=$(conc_level) \ > > > + RUST_FLAGS='$(RUST_FLAGS)' skip_checks=$(do_skip_checks) conc_level=$(conc_level) \ > > > $(SHELL) $(DROOT)/scripts/misc/kernelconfig $@ > > > > > > .PHONY: printenv > > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > > > index 53fbb55c198a..6e9c78aadb45 100644 > > > --- a/debian/rules.d/2-binary-arch.mk > > > +++ b/debian/rules.d/2-binary-arch.mk > > > @@ -10,6 +10,22 @@ build_cd = > > > build_O = O=$(builddir)/build-$* > > > endif > > > > > > +# Define Rust build flags only for Rust-enabled kernels > > > +ifeq ($(do_lib_rust),true) > > > +RUST_FLAGS = \ > > > + RUSTC=rustc-1.68 \ > > > + HOSTRUSTC=rustc-1.68 \ > > > + BINDGEN=bindgen-0.56 \ > > > + RUSTFMT=/usr/lib/rust-1.68/bin/rustfmt \ > > > + RUST_LIB_SRC=/usr/src/rustc-1.68.2/library \ > > > + CLANG_PATH=clang-15 \ > > > + LIBCLANG_PATH=/usr/lib/llvm-15/lib > > > +else > > > +RUST_FLAGS= > > > +endif > > > + > > > +kmake += $(RUST_FLAGS) > > > + > > > # TODO this is probably wrong, and should be using $(DEB_HOST_MULTIARCH) > > > shlibdeps_opts = $(if $(CROSS_COMPILE),-- -l$(CROSS_COMPILE:%-=/usr/%)/lib) > > > > > > diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig > > > index 035426762f8a..741650340b65 100755 > > > --- a/debian/scripts/misc/kernelconfig > > > +++ b/debian/scripts/misc/kernelconfig > > > @@ -118,7 +118,7 @@ EOF > > > # Call config target > > > echo > > > echo "* Run ${target} on ${arch}/${flavour} ..." > > > - ${kmake} O=build "${env[@]}" "${target}" > > > + make ${RUST_FLAGS} O=build "${env[@]}" "${target}" > > > > > > # Move config for further processing > > > mv build/.config "${tmp_conf_file}" > > > -- > > > 2.40.1 > > > > > > > > > -- > > > kernel-team mailing list > > > kernel-team@lists.ubuntu.com > > > https://lists.ubuntu.com/mailman/listinfo/kernel-team > > > > > > >
diff --git a/debian/rules.d/0-common-vars.mk b/debian/rules.d/0-common-vars.mk index 410de315afba..d62a9b063241 100644 --- a/debian/rules.d/0-common-vars.mk +++ b/debian/rules.d/0-common-vars.mk @@ -238,8 +238,6 @@ kmake = make ARCH=$(build_arch) \ CROSS_COMPILE=$(CROSS_COMPILE) \ HOSTCC=$(HOSTCC) \ CC=$(CROSS_COMPILE)$(gcc) \ - RUSTC=rustc-1.68 HOSTRUSTC=rustc-1.68 BINDGEN=bindgen-0.56 RUSTFMT=/usr/lib/rust-1.68/bin/rustfmt RUST_LIB_SRC=/usr/src/rustc-1.68.2/library \ - CLANG_PATH=clang-15 LIBCLANG_PATH=/usr/lib/llvm-15/lib \ KERNELVERSION=$(abi_release)-$(target_flavour) \ CONFIG_DEBUG_SECTION_MISMATCH=y \ KBUILD_BUILD_VERSION="$(uploadnum)" \ diff --git a/debian/rules.d/1-maintainer.mk b/debian/rules.d/1-maintainer.mk index 839e93a15371..a72e5fe93784 100644 --- a/debian/rules.d/1-maintainer.mk +++ b/debian/rules.d/1-maintainer.mk @@ -29,7 +29,7 @@ configs-targets := updateconfigs defaultconfigs genconfigs editconfigs .PHONY: $(configs-targets) $(configs-targets): dh_testdir - kmake='$(kmake)' skip_checks=$(do_skip_checks) conc_level=$(conc_level) \ + RUST_FLAGS='$(RUST_FLAGS)' skip_checks=$(do_skip_checks) conc_level=$(conc_level) \ $(SHELL) $(DROOT)/scripts/misc/kernelconfig $@ .PHONY: printenv diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk index 53fbb55c198a..6e9c78aadb45 100644 --- a/debian/rules.d/2-binary-arch.mk +++ b/debian/rules.d/2-binary-arch.mk @@ -10,6 +10,22 @@ build_cd = build_O = O=$(builddir)/build-$* endif +# Define Rust build flags only for Rust-enabled kernels +ifeq ($(do_lib_rust),true) +RUST_FLAGS = \ + RUSTC=rustc-1.68 \ + HOSTRUSTC=rustc-1.68 \ + BINDGEN=bindgen-0.56 \ + RUSTFMT=/usr/lib/rust-1.68/bin/rustfmt \ + RUST_LIB_SRC=/usr/src/rustc-1.68.2/library \ + CLANG_PATH=clang-15 \ + LIBCLANG_PATH=/usr/lib/llvm-15/lib +else +RUST_FLAGS= +endif + +kmake += $(RUST_FLAGS) + # TODO this is probably wrong, and should be using $(DEB_HOST_MULTIARCH) shlibdeps_opts = $(if $(CROSS_COMPILE),-- -l$(CROSS_COMPILE:%-=/usr/%)/lib) diff --git a/debian/scripts/misc/kernelconfig b/debian/scripts/misc/kernelconfig index 035426762f8a..741650340b65 100755 --- a/debian/scripts/misc/kernelconfig +++ b/debian/scripts/misc/kernelconfig @@ -118,7 +118,7 @@ EOF # Call config target echo echo "* Run ${target} on ${arch}/${flavour} ..." - ${kmake} O=build "${env[@]}" "${target}" + make ${RUST_FLAGS} O=build "${env[@]}" "${target}" # Move config for further processing mv build/.config "${tmp_conf_file}"
To enable Rust support in the kernel we need to pass special build flags, that are used to select the proper Rust toolchain version and dependent components. These flags are passed also to the scripts called during the updateconfigs step and the build, such as 'kernelconfig', using the $(kmake) variable. However, $(kmake) can also contain cross-compiler variables that may potentially affect the config or the build in certain architectures. For this reason, avoid using kmake during 'updateconfigs' and explicitly pass the Rust flags using a separate environment variable that will be defined only for Rust-enabled kernels (do_lib_rust=true). Signed-off-by: Andrea Righi <andrea.righi@canonical.com> --- debian/rules.d/0-common-vars.mk | 2 -- debian/rules.d/1-maintainer.mk | 2 +- debian/rules.d/2-binary-arch.mk | 16 ++++++++++++++++ debian/scripts/misc/kernelconfig | 2 +- 4 files changed, 18 insertions(+), 4 deletions(-)