rust: Require `native: true` for `proc-macro` targets and allow linki… #11743

sdroege · 2023-04-27T07:15:47Z

…ng into targets for other platforms

proc-macros are basically compiler extensions and must be compiled for the build machine so that the compiler can load and execute it at runtime.

Without this, cross-compilation of targets that use proc-macro dependencies is going to fail.

Fixes #11702

sdroege · 2023-04-27T07:21:05Z

test cases/rust/18 proc-macro/meson.build

@@ -8,6 +8,7 @@ pm = shared_library(
  'proc_macro_examples',


A couple of lines above, why is this test disabled on non-Linux, how does it fail? Theoretically there shouldn't be anything special required.

@sdroege some of the tests are disabled on certain platforms because toolchains might not be setup on those platforms (work needed) or tests can take too long. In this case, you are probably running into the second option. Azure CI takes an ungodly amount of time and will timeout frequently.

It could very well be historical reasons that have been fixed since then. Feel free to open a separate PR that unskip them and see how it goes on the CI.

sdroege · 2023-04-27T07:23:16Z

test cases/rust/18 proc-macro/meson.build

@@ -8,6 +8,7 @@ pm = shared_library(
  'proc_macro_examples',
  'proc.rs',
  rust_crate_type : 'proc-macro',
+  native: true,


How would I enable this test on the CI for actually doing cross-compilation? I see that there are Linux/armhf and mingw64 cross tests in the CI. Should I add the corresponding rust toolchains there? How would I then enable this test to actually run?

See e.g. cross/linux-mingw-w64-64bit.txt

You just need to set up the rust compiler as a cross binary, and configure the corresponding .json file to change e.g. "tests": ["common", "cmake"], to specify which run_project_tests.py suites are now supported by that cross test configuration.

I don't think we should add native: true here, the whole point of this PR is to test it's the default.

Giving it a try at cross rust compilation in CI there: #11990.

mesonbuild/build.py

jpakkane · 2023-04-28T22:37:52Z

Is there ever a case where you'd want to compile proc-macro crates with a cross compiler? If no, could we not make it default to native: true instead of requiring people to write it out by hand every time?

sdroege · 2023-05-02T15:00:59Z

Is there ever a case where you'd want to compile proc-macro crates with a cross compiler? If no, could we not make it default to native: true instead of requiring people to write it out by hand every time?

That would also work, I guess.

The only case I could think of is when someone wants to compile the proc-macro on some build machine, and then compile something with that proc-macro on the target machine. Not sure if that's something worth worrying about?

eli-schwartz · 2023-05-02T15:16:39Z

You'd have to install one to do that. Is this expected to work?

sdroege · 2023-05-02T15:24:26Z

You'd have to install it or copy it around somehow manually, yes. I wouldn't expect that to work but with enough motivation and carefully set up environments you can probably make it work. It's something I wouldn't want to support though.

sdroege · 2023-05-03T07:18:32Z

So I guess the summary here would be to a) default to native: true, b) warn if native: false if not a crossbuild / error if crossbuild, c) warn if install: true that this is unsupported. I'll change it accordingly later.

For using a proc-macro built "elsewhere" (c) you need to ensure that exactly the same toolchain version is used and then it can theoretically work but I don't think this is explicitly supported by rustc either. If it's literally the same toolchain then there's no reason why it shouldn't work (you'd somehow need to tell the compiler where to find it though), if you build on a cross toolchain and then use it on the target with the same toolchain version it may or may not work.

…ng into targets for other platforms proc-macros are basically compiler extensions and must be compiled for the build machine so that the compiler can load and execute it at runtime. Without this, cross-compilation of targets that use proc-macro dependencies is going to fail. Fixes mesonbuild#11702

eli-schwartz · 2023-06-27T18:54:05Z

Ping. What's the status of wiring up tests?

dcbaker

From a code point of view this looks fine to me, I still wonder long-term if we want to deprecate the proc-macro crate type and just add a rust_module.proc_macro() helper to avoid this. I think this is a net improvement over what we have, so I'm happy

dcbaker · 2023-06-27T18:58:47Z

I'm also in favor of landing this after the rc if necessary. From my point of view this is a bug fix, since Meson allows something that cannot work in practice

eli-schwartz · 2023-06-27T19:00:40Z

Agreed -- once it is tested in CI I'm happy with merging it even past the rc freeze.

sdroege · 2023-06-27T19:07:44Z

What's the status of wiring up tests?

Doing tests looks like an adventure that will take me a couple of hours once I figured out how the CI works exactly and waited for a few dozen failed runs in the meantime, so I didn't get to that yet and it will probably take a while. I'd like to have this merged but I also think that having tests on the CI for this is important because otherwise it will just stop working again sooner or later :)

If someone wants to take this over please go ahead, otherwise I'll get to it once I have a bigger slot of time available.

xclaesse · 2023-07-14T18:22:13Z

mesonbuild/build.py

@@ -1408,7 +1408,7 @@ def link(self, target):
                msg = f"Can't link non-PIC static library {t.name!r} into shared library {self.name!r}. "
                msg += "Use the 'pic' option to static_library to build with PIC."
                raise InvalidArguments(msg)
-            if self.for_machine is not t.for_machine:
+            if self.for_machine is not t.for_machine and (not t.uses_rust() or t.rust_crate_type != 'proc-macro'):


I think t can be a CustomTarget which does not implement uses_rust(). What about:

Suggested change

if self.for_machine is not t.for_machine and (not t.uses_rust() or t.rust_crate_type != 'proc-macro'):

if self.for_machine is not t.for_machine and (not isinstance(t, SharedLibrary) or t.rust_crate_type != 'proc-macro'):

xclaesse · 2023-07-14T18:28:26Z

mesonbuild/build.py

@@ -2328,6 +2328,13 @@ def process_kwargs(self, kwargs):
            else:
                raise InvalidArguments(f'Invalid rust_crate_type "{rust_crate_type}": must be a string.')
            if rust_crate_type == 'proc-macro':
+                if 'native' not in kwargs:
+                    kwargs['native'] = True
+                if not kwargs.get('native', False):


You set it to true above if it's missing, so the False cannot be used.

Suggested change

if not kwargs.get('native', False):

if not kwargs['native']:

xclaesse · 2023-07-14T18:31:44Z

mesonbuild/build.py

+                    if self.environment.is_cross_build():
+                        raise InvalidArguments('Rust "proc-macro" crate type requires "native: true". This will fail in a cross-build.')
+                    else:
+                        mlog.warning('Rust "proc-macro" crate type requires "native: true". This will fail in a cross-build.')


It does not require "native: true", that's the default in this case. I think it should always be an hard error, no need to downgrade to a warning if it's not a cross build. To happen, user must have explicitely wrote:

shared_library(..., rust_crate_type: 'proc-macros', native: false)

which is always wrong.

What about:

Suggested change

if self.environment.is_cross_build():

raise InvalidArguments('Rust "proc-macro" crate type requires "native: true". This will fail in a cross-build.')

else:

mlog.warning('Rust "proc-macro" crate type requires "native: true". This will fail in a cross-build.')

raise InvalidArguments('Rust "proc-macro" crate type does not allow "native: false".')

rust: Require `native: true` for `proc-macro` targets and allow linking into targets for other platforms proc-macros are basically compiler extensions and must be compiled for the build machine so that the compiler can load and execute it at runtime. Without this, cross-compilation of targets that use proc-macro dependencies is going to fail. Fixes mesonbuild#11702

heftig · 2023-07-30T16:16:09Z

I'm encountering a bit more trouble cross-compiling Mesa for i686; the rusticl_proc-macros shared lib is used to build the rusticl static lib, which is then link_wholed into the RusticlOpenCL shared lib.

Meson adds the rusticl_proc-macros shared lib to the link args of the RusticlOpenCL shared lib, which doesn't do anything in a native build but fails to link in a cross build.

xclaesse · 2023-08-24T18:57:07Z

@heftig Can you please open a separate issue? A minimal test case would be nice too because I failed to reproduce.

xclaesse · 2023-08-24T18:59:28Z

Closing this PR because it is included into #11714 which does a bigger refactoring of proc-macro.

sdroege requested a review from jpakkane as a code owner April 27, 2023 07:15

sdroege mentioned this pull request Apr 27, 2023

rust: Uses wrong target architecture for proc-macro targets when cross-compiling #11702

Closed

sdroege force-pushed the rust-proc-macro-native branch from 13f3a7f to 105b7fd Compare April 27, 2023 07:18

sdroege commented Apr 27, 2023

View reviewed changes

eli-schwartz reviewed Apr 27, 2023

View reviewed changes

mesonbuild/build.py Outdated Show resolved Hide resolved

sdroege force-pushed the rust-proc-macro-native branch from 105b7fd to 9d61c0f Compare April 27, 2023 12:05

dcbaker added the language:rust label Apr 27, 2023

sdroege force-pushed the rust-proc-macro-native branch from 9d61c0f to ef0c71d Compare May 3, 2023 12:11

sdroege force-pushed the rust-proc-macro-native branch from ef0c71d to 7e7749d Compare May 3, 2023 12:22

dcbaker approved these changes Jun 27, 2023

View reviewed changes

dcbaker added this to the 1.2.0 milestone Jun 27, 2023

tristan957 modified the milestones: 1.2.0, 1.3.0, 1.1.2 Jul 13, 2023

xclaesse reviewed Jul 14, 2023

View reviewed changes

jpakkane modified the milestones: 1.2.0, 1.3.0 Jul 16, 2023

xclaesse mentioned this pull request Jul 20, 2023

CI: Cross compile Rust tests #11990

Open

xclaesse mentioned this pull request Aug 9, 2023

Rust: Replace rust_crate_type with rust_abi #11714

Merged

xclaesse closed this Aug 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rust: Require `native: true` for `proc-macro` targets and allow linki… #11743

rust: Require `native: true` for `proc-macro` targets and allow linki… #11743

sdroege commented Apr 27, 2023

sdroege Apr 27, 2023

tristan957 Apr 30, 2023

xclaesse Aug 9, 2023

sdroege Apr 27, 2023

eli-schwartz Apr 30, 2023

xclaesse Jul 14, 2023

jpakkane commented Apr 28, 2023

sdroege commented May 2, 2023

eli-schwartz commented May 2, 2023

sdroege commented May 2, 2023

sdroege commented May 3, 2023

eli-schwartz commented Jun 27, 2023

dcbaker left a comment

dcbaker commented Jun 27, 2023 •

edited

Loading

eli-schwartz commented Jun 27, 2023

sdroege commented Jun 27, 2023

xclaesse Jul 14, 2023

xclaesse Jul 14, 2023

xclaesse Jul 14, 2023

heftig commented Jul 30, 2023

xclaesse commented Aug 24, 2023

xclaesse commented Aug 24, 2023

	if self.for_machine is not t.for_machine and (not t.uses_rust() or t.rust_crate_type != 'proc-macro'):
	if self.for_machine is not t.for_machine and (not isinstance(t, SharedLibrary) or t.rust_crate_type != 'proc-macro'):

rust: Require native: true for proc-macro targets and allow linki… #11743

rust: Require native: true for proc-macro targets and allow linki… #11743

Conversation

sdroege commented Apr 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpakkane commented Apr 28, 2023

sdroege commented May 2, 2023

eli-schwartz commented May 2, 2023

sdroege commented May 2, 2023

sdroege commented May 3, 2023

eli-schwartz commented Jun 27, 2023

dcbaker left a comment

Choose a reason for hiding this comment

dcbaker commented Jun 27, 2023 • edited Loading

eli-schwartz commented Jun 27, 2023

sdroege commented Jun 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heftig commented Jul 30, 2023

xclaesse commented Aug 24, 2023

xclaesse commented Aug 24, 2023

rust: Require `native: true` for `proc-macro` targets and allow linki… #11743

rust: Require `native: true` for `proc-macro` targets and allow linki… #11743

dcbaker commented Jun 27, 2023 •

edited

Loading