NACK: [SRU][M][PATCH 1/1] UBUNTU: [Packaging] define Rust flags only with Rust-enabled kernels

[Juerg Haefliger]

On Tue, 26 Sep 2023 11:27:43 +0100
Dimitri John Ledkov <dimitri.ledkov at canonical.com> wrote:

> On Tue, 26 Sept 2023 at 09:32, Andrea Righi <andrea.righi at 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 at 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 at lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team  
> 
> 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230926/2dbad04f/attachment.sig>