Apple: Fix direct linking with +verbatim #132394

madsmtm · 2024-10-31T08:35:51Z

Linking with +verbatim somewhat worked before, but only when the library was included as part of an rlib.

CC @petrochenkov, since you've worked on +verbatim before.

@rustbot label O-apple A-linkage

try-job: aarch64-apple
try-job: x86_64-apple-2

rustbot · 2024-10-31T08:36:00Z

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-10-31T08:36:02Z

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

madsmtm · 2024-10-31T08:37:04Z

tests/run-make/native-link-modifier-verbatim-linker/rmake.rs

-//@ ignore-apple
-// Reason: linking fails due to the unusual ".ext" staticlib name.


This ignore was added in #101806, and only later rationalized when converting to rmake.

jieyouxu · 2024-10-31T08:40:51Z

The changes itself looks reasonable to me, but I don't know the implications of +verbatim so:

r? @petrochenkov

…try> Apple: Fix direct linking with +verbatim Linking with `+verbatim` somewhat worked before, but only when the library was included as part of an rlib. Fixes rust-lang#132264. CC `@petrochenkov,` since you've worked on `+verbatim` before. `@rustbot` label O-apple A-linkage try-job: aarch64-apple try-job: x86_64-apple-2

Linking with +verbatim somewhat worked before, but only when the library was included as part of an rlib.

madsmtm · 2024-10-31T08:52:01Z

Fixed the tidy error now

jieyouxu · 2024-10-31T08:54:18Z

@bors try

…try> Apple: Fix direct linking with +verbatim Linking with `+verbatim` somewhat worked before, but only when the library was included as part of an rlib. Fixes rust-lang#132264. CC `@petrochenkov,` since you've worked on `+verbatim` before. `@rustbot` label O-apple A-linkage try-job: aarch64-apple try-job: x86_64-apple-2

bors · 2024-10-31T08:55:29Z

⌛ Trying commit 71a8af9 with merge b856a70...

bors · 2024-10-31T10:25:43Z

☀️ Try build successful - checks-actions
Build commit: b856a70 (b856a702839228028a62e27c73436bf9ff590a4e)

jieyouxu · 2024-10-31T10:28:24Z

Try job passed, feel free to r=me if petrochenkov has no further concerns.

petrochenkov · 2024-10-31T21:15:53Z

Please, don't merge this yet, I'll look.

petrochenkov · 2024-11-09T20:09:38Z

I'm tentatively against doing this, otherwise I'd merge this quickly.
I still need to write an explanation though, sorry for the delays.

madsmtm · 2024-11-09T23:44:20Z

sorry for the delays.

There's no haste from my side, I don't need this myself.

petrochenkov · 2025-03-09T12:42:07Z

I think we should either

delegate this to linker (if the linker doesn't support it, then it's not supported) - that's the documented status quo
or just make rustc own native library search consistently on all targets (at least for static libraries), and use linker only as a fallback (like in linker: Allow MSVC to use import libraries following the Meson/MinGW convention #123436).

I'll close this PR to maintain the status quo for now, but I'm also trying to make a step towards the second alternative in #138273 by making the search directory list for native libs consistent between rustc and linker.

metadata: Ignore sysroot when doing the manual native lib search in rustc This is the opposite alternative to rust-lang#138170 and another way to make native library search consistent between rustc and linker. This way the directory list searched by rustc is still a prefix of the directory list considered by linker, but it's a shorter prefix than in rust-lang#138170. We can include the sysroot directories into rustc's search again later if the issues with rust-lang#138170 are resolved, it will be a backward compatible change. This may break some code doing weird things on unstable rustc, or tier 2-3 targets, like bundling `libunwind.a` or sanitizers into something. Note that this doesn't affect shipped `libc.a`, because it lives in `self-contained` directories in sysroot, and `self-contained` sysroot is already not included into the rustc's search. All libunwind and sanitizer libs should be moved to `self-contained` sysroot too eventually. With the consistent search directory list between rustc and linker we can make rustc own the native library search (at least for static libs) and use linker search only as a fallback (like in rust-lang#123436). This will allow addressing issues like rust-lang#132394 once and for all on all targets. r? `@bjorn3`

metadata: Ignore sysroot when doing the manual native lib search in rustc This is the opposite alternative to rust-lang#138170 and another way to make native library search consistent between rustc and linker. This way the directory list searched by rustc is still a prefix of the directory list considered by linker, but it's a shorter prefix than in rust-lang#138170. We can include the sysroot directories into rustc's search again later if the issues with rust-lang#138170 are resolved, it will be a backward compatible change. This may break some code doing weird things on unstable rustc, or tier 2-3 targets, like bundling `libunwind.a` or sanitizers into something. Note that this doesn't affect shipped `libc.a`, because it lives in `self-contained` directories in sysroot, and `self-contained` sysroot is already not included into the rustc's search. All libunwind and sanitizer libs should be moved to `self-contained` sysroot too eventually. With the consistent search directory list between rustc and linker we can make rustc own the native library search (at least for static libs) and use linker search only as a fallback (like in rust-lang#123436). This will allow addressing issues like rust-lang#132394 once and for all on all targets. r? ``@bjorn3``

Rollup merge of rust-lang#138273 - petrochenkov:nonatroot, r=bjorn3 metadata: Ignore sysroot when doing the manual native lib search in rustc This is the opposite alternative to rust-lang#138170 and another way to make native library search consistent between rustc and linker. This way the directory list searched by rustc is still a prefix of the directory list considered by linker, but it's a shorter prefix than in rust-lang#138170. We can include the sysroot directories into rustc's search again later if the issues with rust-lang#138170 are resolved, it will be a backward compatible change. This may break some code doing weird things on unstable rustc, or tier 2-3 targets, like bundling `libunwind.a` or sanitizers into something. Note that this doesn't affect shipped `libc.a`, because it lives in `self-contained` directories in sysroot, and `self-contained` sysroot is already not included into the rustc's search. All libunwind and sanitizer libs should be moved to `self-contained` sysroot too eventually. With the consistent search directory list between rustc and linker we can make rustc own the native library search (at least for static libs) and use linker search only as a fallback (like in rust-lang#123436). This will allow addressing issues like rust-lang#132394 once and for all on all targets. r? ``@bjorn3``

petrochenkov · 2025-03-20T16:53:55Z

or just make rustc own native library search consistently on all targets (at least for static libraries), and use linker only as a fallback

Done in #138753.

rustbot assigned jieyouxu Oct 31, 2024

rustbot added A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 31, 2024

rustbot added A-linkage Area: linking into static, shared libraries and binaries O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) labels Oct 31, 2024

madsmtm commented Oct 31, 2024

View reviewed changes

rustbot assigned petrochenkov and unassigned jieyouxu Oct 31, 2024

This comment was marked as outdated.

Sign in to view

This comment has been minimized.

Sign in to view

This comment was marked as resolved.

Sign in to view

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 31, 2024

Apple: Fix direct linking with +verbatim

71a8af9

Linking with +verbatim somewhat worked before, but only when the library was included as part of an rlib.

madsmtm force-pushed the fix-apple-verbatim-link branch from 3cb4340 to 71a8af9 Compare October 31, 2024 08:51

petrochenkov removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 1, 2024

petrochenkov added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 1, 2024

petrochenkov mentioned this pull request Mar 9, 2025

metadata: Ignore sysroot when doing the manual native lib search in rustc #138273

Merged

petrochenkov closed this Mar 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apple: Fix direct linking with +verbatim #132394

Apple: Fix direct linking with +verbatim #132394

madsmtm commented Oct 31, 2024 •

edited by jieyouxu

Loading

rustbot commented Oct 31, 2024

rustbot commented Oct 31, 2024

madsmtm Oct 31, 2024

jieyouxu commented Oct 31, 2024

This comment was marked as outdated.

This comment was marked as outdated.

This comment has been minimized.

This comment has been minimized.

This comment was marked as resolved.

madsmtm commented Oct 31, 2024

jieyouxu commented Oct 31, 2024

bors commented Oct 31, 2024

bors commented Oct 31, 2024

jieyouxu commented Oct 31, 2024

petrochenkov commented Oct 31, 2024

petrochenkov commented Nov 9, 2024

madsmtm commented Nov 9, 2024

petrochenkov commented Mar 9, 2025

petrochenkov commented Mar 20, 2025

		//@ ignore-apple
		// Reason: linking fails due to the unusual ".ext" staticlib name.

Apple: Fix direct linking with +verbatim #132394

Apple: Fix direct linking with +verbatim #132394

Conversation

madsmtm commented Oct 31, 2024 • edited by jieyouxu Loading

rustbot commented Oct 31, 2024

rustbot commented Oct 31, 2024

madsmtm Oct 31, 2024

Choose a reason for hiding this comment

jieyouxu commented Oct 31, 2024

This comment was marked as outdated.

This comment was marked as outdated.

This comment has been minimized.

This comment has been minimized.

This comment was marked as resolved.

madsmtm commented Oct 31, 2024

jieyouxu commented Oct 31, 2024

bors commented Oct 31, 2024

bors commented Oct 31, 2024

jieyouxu commented Oct 31, 2024

petrochenkov commented Oct 31, 2024

petrochenkov commented Nov 9, 2024

madsmtm commented Nov 9, 2024

petrochenkov commented Mar 9, 2025

petrochenkov commented Mar 20, 2025

madsmtm commented Oct 31, 2024 •

edited by jieyouxu

Loading