diff mbox series

rust: query the compiler for dylib path

Message ID 20241008224810.84024-1-tamird@gmail.com (mailing list archive)
State New
Headers show
Series rust: query the compiler for dylib path | expand

Commit Message

Tamir Duberstein Oct. 8, 2024, 10:48 p.m. UTC 
Rust proc-macro crates are loaded by the compiler at compile-time, so
are always dynamic libraries; on macOS, these artifacts get a .dylib
extension rather than .so.

Replace hardcoded paths ending in .so with paths obtained from the
compiler.

Signed-off-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 .gitignore                        |  1 +
 Makefile                          |  2 +-
 rust/Makefile                     | 21 ++++++++++++---------
 scripts/generate_rust_analyzer.py | 16 ++++++++++++----
 4 files changed, 26 insertions(+), 14 deletions(-)

Comments

Miguel Ojeda Oct. 9, 2024, 12:42 p.m. UTC | #1 
On Wed, Oct 9, 2024 at 12:48 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Rust proc-macro crates are loaded by the compiler at compile-time, so
> are always dynamic libraries; on macOS, these artifacts get a .dylib
> extension rather than .so.

What is the status of the macOS build support? A link would be nice here.

> Signed-off-by: Fiona Behrens <me@kloenk.dev>

Is this patch Fiona's/yours/both? Depending on that, different tags
are needed here (including `From:`). Please see:

    https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

> @@ -9,6 +9,8 @@ import logging
>  import os
>  import pathlib
>  import sys
> +import os
> +import subprocess

Nit: double import, unsorted.

Thanks!

Cheers,
Miguel
Tamir Duberstein Oct. 9, 2024, 12:56 p.m. UTC | #2 
On Wed, Oct 9, 2024 at 8:43 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Wed, Oct 9, 2024 at 12:48 AM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > Rust proc-macro crates are loaded by the compiler at compile-time, so
> > are always dynamic libraries; on macOS, these artifacts get a .dylib
> > extension rather than .so.
>
> What is the status of the macOS build support? A link would be nice here.

What would you have me link to? With this patch applied and using
https://github.com/bee-headers/homebrew-bee-headers I was able to build a kernel
on my apple silicon mac. Relevant config:

tamird@Tamirs-MBP linux % rg -N '_RUST' .config
CONFIG_RUSTC_VERSION=108100
CONFIG_RUST_IS_AVAILABLE=y
CONFIG_RUST=y
CONFIG_RUSTC_VERSION_TEXT="rustc 1.81.0 (eeb90cda1 2024-09-04)"
CONFIG_RUSTC_SUPPORTS_ARM64=y
CONFIG_HAVE_RUST=y
# CONFIG_RUST_FW_LOADER_ABSTRACTIONS is not set
# CONFIG_BLK_DEV_RUST_NULL is not set
# CONFIG_RUST_PHYLIB_ABSTRACTIONS is not set
CONFIG_SAMPLES_RUST=y
CONFIG_SAMPLE_RUST_MINIMAL=m
CONFIG_SAMPLE_RUST_PRINT=m
CONFIG_SAMPLE_RUST_HOSTPROGS=y
CONFIG_RUST_DEBUG_ASSERTIONS=y
CONFIG_RUST_OVERFLOW_CHECKS=y
# CONFIG_RUST_BUILD_ASSERT_ALLOW is not set

> > Signed-off-by: Fiona Behrens <me@kloenk.dev>
>
> Is this patch Fiona's/yours/both? Depending on that, different tags
> are needed here (including `From:`). Please see:
>
>     https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

Thanks. Fiona wrote the original patch ~2 years ago. I rebased it and
generalized it some. I'll use Co-developed-by.

> > @@ -9,6 +9,8 @@ import logging
> >  import os
> >  import pathlib
> >  import sys
> > +import os
> > +import subprocess
>
> Nit: double import, unsorted.

Ack, will fix.

>
> Thanks!
>
> Cheers,
> Miguel
Miguel Ojeda Oct. 9, 2024, 1:12 p.m. UTC | #3 
On Wed, Oct 9, 2024 at 2:57 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> What would you have me link to? With this patch applied and using

I was thinking perhaps the series from Daniel, if that is the latest
discussion (?), i.e. I would like to understand what is the policy
around changes like this, what happens if it breaks, etc.

> https://github.com/bee-headers/homebrew-bee-headers I was able to build a kernel
> on my apple silicon mac. Relevant config:

That is great.

Thanks!

Cheers,
Miguel
Tamir Duberstein Oct. 9, 2024, 1:18 p.m. UTC | #4 
On Wed, Oct 9, 2024 at 9:13 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Wed, Oct 9, 2024 at 2:57 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > What would you have me link to? With this patch applied and using
>
> I was thinking perhaps the series from Daniel, if that is the latest
> discussion (?), i.e. I would like to understand what is the policy
> around changes like this, what happens if it breaks, etc.

Ah, I see. The relevant discussion took place on zulip[0]. As for policy: I'm
guessing there isn't one, and the whole endeavor is best-effort.

Daniel's other patches weren't necessary for the kernel config I'm using.

> > https://github.com/bee-headers/homebrew-bee-headers I was able to build a kernel
> > on my apple silicon mac. Relevant config:
>
> That is great.
>
> Thanks!
>
> Cheers,
> Miguel

Thanks Miguel! As this is my first patch, please let me know if further action
is required.

Cheers.
Tamir

[0] https://rust-for-linux.zulipchat.com/#narrow/stream/288089-General/topic/macOS.20build
Miguel Ojeda Oct. 9, 2024, 2:20 p.m. UTC | #5 
On Wed, Oct 9, 2024 at 3:18 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Ah, I see. The relevant discussion took place on zulip[0]. As for policy: I'm
> guessing there isn't one, and the whole endeavor is best-effort.

I am aware of that discussion (it is in our Zulip), but I was
referring to the kernel-wide macOS support, because I had read
Daniel's summary and v2 of that series, and it is not clear to me what
is the latest status on supporting macOS.

In particular, there were some patches NAK'd with arguments that may
apply here (e.g. extra process spawns).

Moreover, how will it get tested going forward? (e.g. currently I
can't, but I could look into setting something up if the kernel wants
to support this). If it breaks, is it considered a bug? etc.

> Thanks Miguel! As this is my first patch, please let me know if further action
> is required.

You're welcome! Yes, a new version would be needed with the proper
tags/authorship, but first we should probably wait to hear what Kbuild
(or the kernel) thinks.

Cheers,
Miguel
Tamir Duberstein Oct. 9, 2024, 2:47 p.m. UTC | #6 
On Wed, Oct 9, 2024 at 10:20 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> In particular, there were some patches NAK'd with arguments that may
> apply here (e.g. extra process spawns).

Understood. My guess is nobody will care about the process spawn in
scripts/generate_rust_analyzer.py. Someone might care about the one in
rust/Makefile, but there are already 4 others. By the way, I notice those are
using $(shell ...) - should I be using that form as well?

> Moreover, how will it get tested going forward? (e.g. currently I
> can't, but I could look into setting something up if the kernel wants
> to support this). If it breaks, is it considered a bug? etc.

I guess that's not for me to say. It would be great to have basic automation.

> > Thanks Miguel! As this is my first patch, please let me know if further action
> > is required.
>
> You're welcome! Yes, a new version would be needed with the proper
> tags/authorship, but first we should probably wait to hear what Kbuild
> (or the kernel) thinks.
>
> Cheers,
> Miguel

> Please read the section of the documentation I linked, it contains an
> example on how this should be done, i.e. the Co-developed-by tag
> cannot be on its own:

My apologies for the oversight.
Daniel Gomez Oct. 10, 2024, 8:08 a.m. UTC | #7 
On Wed Oct 9, 2024 at 12:48 AM CEST, Tamir Duberstein wrote:
> Rust proc-macro crates are loaded by the compiler at compile-time, so
> are always dynamic libraries; on macOS, these artifacts get a .dylib
> extension rather than .so.
>
> Replace hardcoded paths ending in .so with paths obtained from the
> compiler.
>
> Signed-off-by: Fiona Behrens <me@kloenk.dev>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
>  .gitignore                        |  1 +
>  Makefile                          |  2 +-
>  rust/Makefile                     | 21 ++++++++++++---------
>  scripts/generate_rust_analyzer.py | 16 ++++++++++++----
>  4 files changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 56972adb5031..7cfe4f70b39a 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 c5493c0c0ca1..3808869fb95b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1506,7 +1506,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 b5e0a73b78f3..a185a4d05b08 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) += alloc.o bindings.o kernel.o
>  always-$(CONFIG_RUST) += exports_alloc_generated.h exports_helpers_generated.h \
> @@ -36,9 +33,15 @@ 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 := $($(RUSTC) --print file-names --crate-name macros --crate-type proc-macro - < /dev/null)
> +libmacros_extension := $(patsubst libmacros.%,%,$(libmacros_name))
> +
> +always-$(CONFIG_RUST) += $(libmacros_name)
> +no-clean-files += $(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)
> @@ -115,10 +118,10 @@ rustdoc-alloc: $(RUST_LIB_SRC)/alloc/src/lib.rs rustdoc-core rustdoc-compiler_bu
>  	+$(call if_changed,rustdoc)
>  
>  rustdoc-kernel: private rustc_target_flags = --extern alloc \
> -    --extern build_error --extern macros=$(objtree)/$(obj)/libmacros.so \
> +    --extern build_error --extern macros=$(objtree)/$(obj)/$(libmacros_name) \
>      --extern bindings --extern uapi
>  rustdoc-kernel: $(src)/kernel/lib.rs rustdoc-core rustdoc-macros \
> -    rustdoc-compiler_builtins rustdoc-alloc $(obj)/libmacros.so \
> +    rustdoc-compiler_builtins rustdoc-alloc $(obj)/$(libmacros_name) \
>      $(obj)/bindings.o FORCE
>  	+$(call if_changed,rustdoc)
>  
> @@ -339,10 +342,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 $@
> @@ -421,7 +424,7 @@ $(obj)/uapi.o: $(src)/uapi/lib.rs \
>  $(obj)/kernel.o: private rustc_target_flags = --extern alloc \
>      --extern build_error --extern macros --extern bindings --extern uapi
>  $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(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 d2bc63cde8c6..3834ab0eea9d 100755
> --- a/scripts/generate_rust_analyzer.py
> +++ b/scripts/generate_rust_analyzer.py
> @@ -9,6 +9,8 @@ import logging
>  import os
>  import pathlib
>  import sys
> +import os
> +import subprocess

import os is duplicated.

>  
>  def args_crates_cfgs(cfgs):
>      crates_cfgs = {}
> @@ -35,8 +37,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 +48,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')
> +            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(
> @@ -77,7 +86,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",
Daniel Gomez Oct. 10, 2024, 8:31 a.m. UTC | #8 
On Wed Oct 9, 2024 at 3:12 PM CEST, Miguel Ojeda wrote:
> On Wed, Oct 9, 2024 at 2:57 PM Tamir Duberstein <tamird@gmail.com> wrote:
>>
>> What would you have me link to? With this patch applied and using
>
> I was thinking perhaps the series from Daniel, if that is the latest
> discussion (?), i.e. I would like to understand what is the policy
> around changes like this, what happens if it breaks, etc.

Building Linux in macOS is now supported for arm64 (targets tested: allyesconfig
and defconfig). The upstream policy is to use the build system variables to
configure the necessary tweaks to support building in macOS. However, this
will not always be possible then, patches are welcome from the build system
maintainer to support "portability" across OSes. But not full integration.
Please, let me know if this is not clear.

This policy forces macOS users to provide their own environment, documentation
and dependencies. I created bee-headers with the intention of providing all this
(including the missing byteswap, elf and endian headers) as reference or for
developers to use.

So, after installing bee-headers and all other linux dependencies (llvm, sed,
awk, etc), one would simply do:

source bee-init # To initiate the build enviroment
make LLVM=1 defconfig
make LLVM=1 -j$(nproc)

Here a more detailed documentation:
https://github.com/bee-headers/homebrew-bee-headers/blob/main/README.md

The last version of the series is v3 [1], which fixes a build error that cannot
be fixed in macOS via build system environment. This is enabled when using Intel
Xe Graphics (DRM_XE), included in allyesconfig target.

[1] https://lore.kernel.org/all/20240925-macos-build-support-v3-1-233dda880e60@samsung.com/

>
>> https://github.com/bee-headers/homebrew-bee-headers I was able to build a kernel
>> on my apple silicon mac. Relevant config:
>
> That is great.
>
> Thanks!
>
> Cheers,
> Miguel
Miguel Ojeda Oct. 12, 2024, 1:17 p.m. UTC | #9 
On Wed, Oct 9, 2024 at 4:48 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Understood. My guess is nobody will care about the process spawn in
> scripts/generate_rust_analyzer.py. Someone might care about the one in
> rust/Makefile, but there are already 4 others. By the way, I notice those are

Yeah, I was referring to the `Makefile` one (the other one, indeed,
does not matter, as you say).

> using $(shell ...) - should I be using that form as well?

Hmm... I assume you tested the patch, but how would the patch work
without it? Or am I confused?

> I guess that's not for me to say. It would be great to have basic automation.

Generally, when submitting a new feature for upstream, especially one
that requires new testing, it is common that the submitter is asked to
take care of it or help doing so. I guess, in this case, Daniel is the
one handling the macOS support out-of-tree.

Anyway, we may need to use variables for this, so I think it is fine
-- upstream can keep the variable working easily, and out-of-tree can
test the overall macOS support.

> My apologies for the oversight.

No worries, thanks!

Cheers,
Miguel
Miguel Ojeda Oct. 12, 2024, 1:41 p.m. UTC | #10 
On Thu, Oct 10, 2024 at 10:31 AM Daniel Gomez <da.gomez@samsung.com> wrote:
>
> Building Linux in macOS is now supported for arm64 (targets tested: allyesconfig
> and defconfig). The upstream policy is to use the build system variables to
> configure the necessary tweaks to support building in macOS. However, this
> will not always be possible then, patches are welcome from the build system
> maintainer to support "portability" across OSes. But not full integration.
> Please, let me know if this is not clear.

Thanks for writing this -- it seems similar to the summary you wrote
elsewhere, but it does confirm we should probably be using build
variables instead (i.e. I don't think the overall macOS support
questions are answered, but we don't need to answer them here).

In other words, it sounds to me like the solution here is to simply
provide a variable with the current name as the default, and let
out-of-tree override that if they need, rather than query `rustc`.

Thus upstream can keep the variable working very easily, and
out-of-tree can maintain/test the overall macOS support.

Does that sound reasonable for everyone?

Cheers,
Miguel
Tamir Duberstein Oct. 12, 2024, 2:25 p.m. UTC | #11 
On Sat, Oct 12, 2024, 09:41 Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> In other words, it sounds to me like the solution here is to simply
> provide a variable with the current name as the default, and let
> out-of-tree override that if they need, rather than query `rustc`.

In order for this to be reasonably maintainable we'd want the variable
to be something like DYLIB_SUFFIX so that we don't have to revisit this
if macros are ever provided by more than one crate (or worse, have to
provide N variables).

If this is the preferred path, I can rework this patch in that direction.

Tamir
Miguel Ojeda Oct. 12, 2024, 11:37 p.m. UTC | #12 
On Sat, Oct 12, 2024 at 4:25 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> In order for this to be reasonably maintainable we'd want the variable
> to be something like DYLIB_SUFFIX so that we don't have to revisit this
> if macros are ever provided by more than one crate (or worse, have to
> provide N variables).

That is reasonable, and in this it is probably the right approach
since the complexity is similar, but I wanted to clarify that, in the
kernel, revisiting is fine and expected (features are generally not
added if they are not used or expected to be used very soon, so
revisiting to add a more complex feature later is the normal
approach).

But before we do that, is there a way to force `rustc` to load current
name (or trick it to do so, say, with a symlink)? i.e. can it be
reasonably done out-of-tree without changes to the filename?

Thanks!

Cheers,
Miguel
Tamir Duberstein Oct. 12, 2024, 11:52 p.m. UTC | #13 
On Sat, Oct 12, 2024 at 7:37 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
> But before we do that, is there a way to force `rustc` to load current
> name (or trick it to do so, say, with a symlink)? i.e. can it be
> reasonably done out-of-tree without changes to the filename?

I think the problem is that rustc produces .dylib on macOS rather than
it _looking_ for .dylib; the path to the .so is fully specified in the
rustc invocation of the targets that depend on it. I don't know of a way
to control the file suffix externally.

A symlink would possibly work (unless rustc refuses to load anything
other than .dylib on macOS for whatever reason), but wouldn't be very
ergonomic; you'd have to create the symlink blind or else run the build
system until it fails, then create the symlink, and then run the build
again.
Masahiro Yamada Oct. 14, 2024, 6:45 p.m. UTC | #14 
On Sun, Oct 13, 2024 at 8:53 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Sat, Oct 12, 2024 at 7:37 PM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> > But before we do that, is there a way to force `rustc` to load current
> > name (or trick it to do so, say, with a symlink)? i.e. can it be
> > reasonably done out-of-tree without changes to the filename?
>
> I think the problem is that rustc produces .dylib on macOS rather than
> it _looking_ for .dylib; the path to the .so is fully specified in the
> rustc invocation of the targets that depend on it. I don't know of a way
> to control the file suffix externally.

rustc ignores --emit=link=rust/libmacro.so
and produces rust/libmacro.dylib.

Is this a bug in rustc?




> A symlink would possibly work (unless rustc refuses to load anything
> other than .dylib on macOS for whatever reason), but wouldn't be very
> ergonomic; you'd have to create the symlink blind or else run the build
> system until it fails, then create the symlink, and then run the build
> again.
Miguel Ojeda Oct. 14, 2024, 7:08 p.m. UTC | #15 
On Mon, Oct 14, 2024 at 8:46 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> rustc ignores --emit=link=rust/libmacro.so
> and produces rust/libmacro.dylib.
>
> Is this a bug in rustc?

Hmm... From a quick test in Linux and macOS (in a GitHub runner):

    uname
    echo | rustc --crate-type=proc-macro --emit=link=a.so -
    echo | rustc --crate-type=proc-macro --emit=link=b.dylib -
    file a.so
    file b.dylib

gives:

    Darwin
    a.so: Mach-O 64-bit dynamically linked shared library arm64
    b.dylib: Mach-O 64-bit dynamically linked shared library arm64

    Linux
    a.so: ELF 64-bit LSB shared object, x86-64, version 1...
    b.dylib: ELF 64-bit LSB shared object, x86-64, version 1...

Cheers,
Miguel
Tamir Duberstein Oct. 15, 2024, 1:10 a.m. UTC | #16 
On Mon, Oct 14, 2024 at 3:09 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Mon, Oct 14, 2024 at 8:46 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > rustc ignores --emit=link=rust/libmacro.so
> > and produces rust/libmacro.dylib.
> >
> > Is this a bug in rustc?

This was my bad - I set this thread down the wrong path. In fact the
current build produces libmacros.so, but I haven't been able to convince
rustc to consume it. Changing `--extern macros` to `--extern
macros=$(objtree)/$(obj)/libmacros.so` produces 'error[E0463]: can't
find crate for `macros`'.

With a toy example, trying to link a proc-macro crate .so produces a
more informative message than the kernel build does:
```
 rustc main.rs --extern macros=./libmacros.so
error: extern location for macros is of an unknown type: ./libmacros.so
 --> main.rs:4:1
  |
4 | extern crate macros;
  | ^^^^^^^^^^^^^^^^^^^^

error: file name should be lib*.rlib or lib*.dylib
 --> main.rs:4:1
  |
4 | extern crate macros;
  | ^^^^^^^^^^^^^^^^^^^^
```

I've opened https://github.com/rust-lang/rust/issues/131720, let's see
what the experts think.
Miguel Ojeda Oct. 15, 2024, 9:10 a.m. UTC | #17 
On Sun, Oct 13, 2024 at 1:53 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> A symlink would possibly work (unless rustc refuses to load anything
> other than .dylib on macOS for whatever reason), but wouldn't be very
> ergonomic; you'd have to create the symlink blind or else run the build
> system until it fails, then create the symlink, and then run the build
> again.

Could you please test if the symlink work?

It is definitely not elegant, and if we start having several macro
crates, it will not really work for you.

However, it would be good to know the potential options available here
and, if it works, it would allow you to get it working right away
while upstream Rust replies.

Thanks!

Cheers,
Miguel
Miguel Ojeda Oct. 15, 2024, 10:33 a.m. UTC | #18 
On Tue, Oct 15, 2024 at 3:11 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> I've opened https://github.com/rust-lang/rust/issues/131720, let's see
> what the experts think.

Thanks -- I added some context there and linked to it (and Lore) in our list:

    https://github.com/Rust-for-Linux/linux/issues/355

Cheers,
Miguel
Tamir Duberstein Oct. 15, 2024, 3:05 p.m. UTC | #19 
I did more digging and I don't think this is going to be readily
fixable upstream:
see https://github.com/rust-lang/rust/issues/131720#issuecomment-2414179941.

A symlink fixes the problem if we *never* specify a path to
libmacros.so, is that how we want to proceed? Note that currently we do
specify it in one place in rust/Makefile and again in
generate_rust_analyzer.py, so those would need updates.

Tamir
Miguel Ojeda Oct. 15, 2024, 3:30 p.m. UTC | #20 
On Tue, Oct 15, 2024 at 5:06 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> I did more digging and I don't think this is going to be readily
> fixable upstream:
> see https://github.com/rust-lang/rust/issues/131720#issuecomment-2414179941.
>
> A symlink fixes the problem if we *never* specify a path to
> libmacros.so, is that how we want to proceed? Note that currently we do
> specify it in one place in rust/Makefile and again in
> generate_rust_analyzer.py, so those would need updates.

If a trick still requires a similar amount of changes to mainline,
then I think we should go for something better/more proper, i.e. the
idea is to minimize changes/complexity upstream, after all.

Thanks!

Cheers,
Miguel
Tamir Duberstein Oct. 15, 2024, 3:53 p.m. UTC | #21 
On Tue, Oct 15, 2024 at 11:30 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> If a trick still requires a similar amount of changes to mainline,
> then I think we should go for something better/more proper, i.e. the
> idea is to minimize changes/complexity upstream, after all.

In that case v5[0] is probably the way to go?

On Mon, Oct 14, 2024 at 2:45 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> This no-clean-files is meaningless and unnecessary.
> This line exists inside the "ifdef CONFIG_RUST" ... "endif" block.
>
> no-clean-files is only used by scripts/Makefile.clean,
> which does not include include/config/auto.conf.

I see. Was it necessary before this patch? Looks like it came with the
initial rust support patch.

[0] https://lore.kernel.org/all/20241010142833.98528-2-tamird@gmail.com/
Masahiro Yamada Oct. 16, 2024, 1:35 a.m. UTC | #22 
On Wed, Oct 16, 2024 at 12:53 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Tue, Oct 15, 2024 at 11:30 AM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> >
> > If a trick still requires a similar amount of changes to mainline,
> > then I think we should go for something better/more proper, i.e. the
> > idea is to minimize changes/complexity upstream, after all.
>
> In that case v5[0] is probably the way to go?
>
> On Mon, Oct 14, 2024 at 2:45 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > This no-clean-files is meaningless and unnecessary.
> > This line exists inside the "ifdef CONFIG_RUST" ... "endif" block.
> >
> > no-clean-files is only used by scripts/Makefile.clean,
> > which does not include include/config/auto.conf.
>
> I see. Was it necessary before this patch? Looks like it came with the
> initial rust support patch.


You can just delete no-clean-files from your patch.


Files specified to $(always-y) are removed by "make clean".
That's why [0] added no-clean-files to negate it.


You are moving "always-$(CONFIG_RUST) += libmacros.so"
into the "ifdef CONFIG_RUST" ... "endif" block, which is not
parsed by "make clean".




>
> [0] https://lore.kernel.org/all/20241010142833.98528-2-tamird@gmail.com/
diff mbox series

Patch

diff --git a/.gitignore b/.gitignore
index 56972adb5031..7cfe4f70b39a 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 c5493c0c0ca1..3808869fb95b 100644
--- a/Makefile
+++ b/Makefile
@@ -1506,7 +1506,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 b5e0a73b78f3..a185a4d05b08 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) += alloc.o bindings.o kernel.o
 always-$(CONFIG_RUST) += exports_alloc_generated.h exports_helpers_generated.h \
@@ -36,9 +33,15 @@  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 := $($(RUSTC) --print file-names --crate-name macros --crate-type proc-macro - < /dev/null)
+libmacros_extension := $(patsubst libmacros.%,%,$(libmacros_name))
+
+always-$(CONFIG_RUST) += $(libmacros_name)
+no-clean-files += $(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)
@@ -115,10 +118,10 @@  rustdoc-alloc: $(RUST_LIB_SRC)/alloc/src/lib.rs rustdoc-core rustdoc-compiler_bu
 	+$(call if_changed,rustdoc)
 
 rustdoc-kernel: private rustc_target_flags = --extern alloc \
-    --extern build_error --extern macros=$(objtree)/$(obj)/libmacros.so \
+    --extern build_error --extern macros=$(objtree)/$(obj)/$(libmacros_name) \
     --extern bindings --extern uapi
 rustdoc-kernel: $(src)/kernel/lib.rs rustdoc-core rustdoc-macros \
-    rustdoc-compiler_builtins rustdoc-alloc $(obj)/libmacros.so \
+    rustdoc-compiler_builtins rustdoc-alloc $(obj)/$(libmacros_name) \
     $(obj)/bindings.o FORCE
 	+$(call if_changed,rustdoc)
 
@@ -339,10 +342,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 $@
@@ -421,7 +424,7 @@  $(obj)/uapi.o: $(src)/uapi/lib.rs \
 $(obj)/kernel.o: private rustc_target_flags = --extern alloc \
     --extern build_error --extern macros --extern bindings --extern uapi
 $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(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 d2bc63cde8c6..3834ab0eea9d 100755
--- a/scripts/generate_rust_analyzer.py
+++ b/scripts/generate_rust_analyzer.py
@@ -9,6 +9,8 @@  import logging
 import os
 import pathlib
 import sys
+import os
+import subprocess
 
 def args_crates_cfgs(cfgs):
     crates_cfgs = {}
@@ -35,8 +37,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 +48,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')
+            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(
@@ -77,7 +86,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",