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

Tue Sep 26 12:16:17 UTC 2023

On Tue, 26 Sep 2023 12:52:25 +0200
Andrea Righi <andrea.righi at 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 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.  
> 
> 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 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  
> > 
> > 
> > 
> > -- 
> > okurrr,
> > 
> > Dimitri  
> 

-------------- 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/e9cb1246/attachment.sig>