[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20241216094917.m265abadlg7boqip@AALNPWDAGOMEZ1.aal.scsc.local>
Date: Mon, 16 Dec 2024 10:49:17 +0100
From: Daniel Gomez <da.gomez@...sung.com>
To: Tamir Duberstein <tamird@...il.com>, Masahiro Yamada
<masahiroy@...nel.org>, Nathan Chancellor <nathan@...nel.org>, Nicolas
Schier <nicolas@...sle.eu>, Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor
<alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>, Gary Guo
<gary@...yguo.net>, Björn Roy Baron
<bjorn3_gh@...tonmail.com>, Benno Lossin <benno.lossin@...ton.me>, Andreas
Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, Trevor
Gross <tmgross@...ch.edu>
CC: <linux-kernel@...r.kernel.org>, <linux-kbuild@...r.kernel.org>,
<rust-for-linux@...r.kernel.org>, Fiona Behrens <me@...enk.dev>
Subject: Re: [PATCH RESEND v6] rust: use host dylib naming convention
On Tue, Nov 05, 2024 at 08:05:43AM -0400, Tamir Duberstein wrote:
> Because the `macros` crate exposes procedural macros, it must be
> compiled as a dynamic library (so it can be loaded by the compiler at
> compile-time).
>
> Before this change the resulting artifact was always named
> `libmacros.so`, which works on hosts where this matches the naming
> convention for dynamic libraries. However the proper name on macOS would
> be `libmacros.dylib`.
>
> This turns out to matter even when the dependency is passed with a path
> (`--extern macros=path/to/libmacros.so` rather than `--extern macros`)
> because rustc uses the file name to infer the type of the library (see
> link). This is because there's no way to specify both the path to and
> the type of the external library via CLI flags. The compiler could
> speculatively parse the file to determine its type, but it does not do
> so today.
>
> This means that libraries that match neither rustc's naming convention
> for static libraries nor the platform's naming convention for dynamic
> libraries are *rejected*.
>
> The only solution I've found is to follow the host platform's naming
> convention. This patch does that by querying the compiler to determine
> the appropriate name for the artifact. This allows the kernel to build
> with CONFIG_RUST=y on macOS.
>
> Link: https://github.com/rust-lang/rust/blob/d829780/compiler/rustc_metadata/src/locator.rs#L728-L752
> Co-developed-by: Fiona Behrens <me@...enk.dev>
> Signed-off-by: Fiona Behrens <me@...enk.dev>
> Signed-off-by: Tamir Duberstein <tamird@...il.com>
This works for me. In macOS, I've built the rnull driver as built-in and
as module.
Tested-by: Daniel Gomez <da.gomez@...sung.com>
Thanks,
Daniel
> ---
> V5 -> V6: Removed setting `no-clean-files`. Rewrote description.
> e4 -> V5: Added missing `shell` in rust/Makefile.
> V3 -> V4: Added motivation. Added missing Signed-off-by.
> V2 -> V3: Added .strip() to rustc output to remove errant newline.
> V1 -> V2: De-duplicated and sorted imports. Changed Signed-off-by to
> Co-developed-by.
> ---
> .gitignore | 1 +
> Makefile | 2 +-
> rust/Makefile | 20 +++++++++++---------
> scripts/generate_rust_analyzer.py | 15 +++++++++++----
> 4 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index a61e4778d011cf706e6784818a1357f392f3a669..088696a6a46a12fdb77eb9ccab5b6b7b11ef4707 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -22,6 +22,7 @@
> *.dtb.S
> *.dtbo.S
> *.dwo
> +*.dylib
> *.elf
> *.gcno
> *.gcda
> diff --git a/Makefile b/Makefile
> index a9e723cb05961877d5e6b50920dcabc78cf4988f..470e6f20c513bf0f90a42c586aa92dd7a8b16fb0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1513,7 +1513,7 @@ MRPROPER_FILES += include/config include/generated \
> certs/x509.genkey \
> vmlinux-gdb.py \
> rpmbuild \
> - rust/libmacros.so
> + rust/libmacros.so rust/libmacros.dylib
>
> # clean - Delete most, but leave enough to build external modules
> #
> diff --git a/rust/Makefile b/rust/Makefile
> index 3678e79317f12d7116ad0c2ac1ca416ee5b969aa..0d882103375aa45be78e22f59c64739c8722fd98 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -11,9 +11,6 @@ always-$(CONFIG_RUST) += exports_core_generated.h
> obj-$(CONFIG_RUST) += helpers/helpers.o
> CFLAGS_REMOVE_helpers/helpers.o = -Wmissing-prototypes -Wmissing-declarations
>
> -always-$(CONFIG_RUST) += libmacros.so
> -no-clean-files += libmacros.so
> -
> always-$(CONFIG_RUST) += bindings/bindings_generated.rs bindings/bindings_helpers_generated.rs
> obj-$(CONFIG_RUST) += bindings.o kernel.o
> always-$(CONFIG_RUST) += exports_helpers_generated.h \
> @@ -36,9 +33,14 @@ always-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated_kunit.c
> obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated.o
> obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated_kunit.o
>
> -# Avoids running `$(RUSTC)` for the sysroot when it may not be available.
> +# Avoids running `$(RUSTC)` when it may not be available.
> ifdef CONFIG_RUST
>
> +libmacros_name := $(shell $(RUSTC) --print file-names --crate-name macros --crate-type proc-macro - < /dev/null)
> +libmacros_extension := $(patsubst libmacros.%,%,$(libmacros_name))
> +
> +always-$(CONFIG_RUST) += $(libmacros_name)
> +
> # `$(rust_flags)` is passed in case the user added `--sysroot`.
> rustc_sysroot := $(shell MAKEFLAGS= $(RUSTC) $(rust_flags) --print sysroot)
> rustc_host_target := $(shell $(RUSTC) --version --verbose | grep -F 'host: ' | cut -d' ' -f2)
> @@ -104,10 +106,10 @@ rustdoc-compiler_builtins: $(src)/compiler_builtins.rs rustdoc-core FORCE
> +$(call if_changed,rustdoc)
>
> rustdoc-kernel: private rustc_target_flags = \
> - --extern build_error --extern macros=$(objtree)/$(obj)/libmacros.so \
> + --extern build_error --extern macros \
> --extern bindings --extern uapi
> rustdoc-kernel: $(src)/kernel/lib.rs rustdoc-core rustdoc-macros \
> - rustdoc-compiler_builtins $(obj)/libmacros.so \
> + rustdoc-compiler_builtins $(obj)/$(libmacros_name) \
> $(obj)/bindings.o FORCE
> +$(call if_changed,rustdoc)
>
> @@ -325,10 +327,10 @@ quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
> -Clink-args='$(call escsq,$(KBUILD_HOSTLDFLAGS))' \
> --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
> --crate-type proc-macro \
> - --crate-name $(patsubst lib%.so,%,$(notdir $@)) $<
> + --crate-name $(patsubst lib%.$(libmacros_extension),%,$(notdir $@)) $<
>
> # Procedural macros can only be used with the `rustc` that compiled it.
> -$(obj)/libmacros.so: $(src)/macros/lib.rs FORCE
> +$(obj)/$(libmacros_name): $(src)/macros/lib.rs FORCE
> +$(call if_changed_dep,rustc_procmacro)
>
> quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L $@
> @@ -401,7 +403,7 @@ $(obj)/uapi.o: $(src)/uapi/lib.rs \
> $(obj)/kernel.o: private rustc_target_flags = \
> --extern build_error --extern macros --extern bindings --extern uapi
> $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/build_error.o \
> - $(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE
> + $(obj)/$(libmacros_name) $(obj)/bindings.o $(obj)/uapi.o FORCE
> +$(call if_changed_rule,rustc_library)
>
> endif # CONFIG_RUST
> diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py
> index 09e1d166d8d236fcf8c2b2375624b243ebf6e7f7..aa8ea1a4dbe5f9037c8c231d87ddc8d95c297f12 100755
> --- a/scripts/generate_rust_analyzer.py
> +++ b/scripts/generate_rust_analyzer.py
> @@ -8,6 +8,7 @@ import json
> import logging
> import os
> import pathlib
> +import subprocess
> import sys
>
> def args_crates_cfgs(cfgs):
> @@ -35,8 +36,7 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
> crates_cfgs = args_crates_cfgs(cfgs)
>
> def append_crate(display_name, root_module, deps, cfg=[], is_workspace_member=True, is_proc_macro=False):
> - crates_indexes[display_name] = len(crates)
> - crates.append({
> + crate = {
> "display_name": display_name,
> "root_module": str(root_module),
> "is_workspace_member": is_workspace_member,
> @@ -47,7 +47,15 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
> "env": {
> "RUST_MODFILE": "This is only for rust-analyzer"
> }
> - })
> + }
> + if is_proc_macro:
> + proc_macro_dylib_name = subprocess.check_output(
> + [os.environ["RUSTC"], "--print", "file-names", "--crate-name", display_name, "--crate-type", "proc-macro", "-"],
> + stdin=subprocess.DEVNULL,
> + ).decode('utf-8').strip()
> + crate["proc_macro_dylib_path"] = f"{objtree}/rust/{proc_macro_dylib_name}"
> + crates_indexes[display_name] = len(crates)
> + crates.append(crate)
>
> # First, the ones in `rust/` since they are a bit special.
> append_crate(
> @@ -70,7 +78,6 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs):
> [],
> is_proc_macro=True,
> )
> - crates[-1]["proc_macro_dylib_path"] = f"{objtree}/rust/libmacros.so"
>
> append_crate(
> "build_error",
>
> ---
> base-commit: 6ce162a002657910104c7a07fb50017681bc476c
> change-id: 20241016-b4-dylib-host-macos-16757c350b49
>
> Best regards,
> -----BEGIN SSH SIGNATURE-----
> U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgtYz36g7iDMSkY5K7Ab51ksGX7h
> JgsMRt+XVZTrIzMVIAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5
> AAAAQFg99wbgHLYJEpFzt5B+h6GLMb65HtsNoryxz5lkEldZ7z5BEXUUDiFLlDnDu8jlkv
> SiANzyz5noKnp8NCgIxQk=
> -----END SSH SIGNATURE-----
> --
> Tamir Duberstein <tamird@...il.com>
>
Powered by blists - more mailing lists