-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't use static crt by default when build proc-macro #69519
Merged
Merged
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
7ca1b2f
Don't use static crt by default when build proc-macro.
12101111 7996df9
Run format.
12101111 89aebbd
Only run this test on musl
12101111 84349cc
Remove trailing whitespace.
12101111 dbed65a
Simplify checking of crt_static_feature()
12101111 7a89bf1
override flags from compiletest
12101111 75e6cfc
run crt-static test on all target
12101111 cfff1b4
When `crate_type` is `None`,check compiler options
12101111 afd374f
Ignore wasm32
12101111 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// Test proc-macro crate can be built without addtional RUSTFLAGS | ||
// on musl target | ||
// override -Ctarget-feature=-crt-static from compiletest | ||
// compile-flags: -Ctarget-feature= | ||
// build-pass | ||
// only-musl | ||
#![crate_type = "proc-macro"] | ||
|
||
extern crate proc_macro; | ||
|
||
use proc_macro::TokenStream; | ||
|
||
#[proc_macro_derive(Foo)] | ||
pub fn derive_foo(input: TokenStream) -> TokenStream { | ||
input | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test doesn't depend on any musl-specific functionality, so I would expect it to run everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depend on musl target but CI don't have it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which CI job was it? This is a problem with the CI configuration, not something that needs to be worked around in the test. Specifically, the musl targets built by CI are missing the dynamic version of
libstd
. To fix this, they need to be built withcrt_static=false
(theconfig.toml
option, not the rustc flag), like thedist-x86_64-musl
job does:rust/src/ci/docker/dist-x86_64-musl/Dockerfile
Line 37 in 1d5241c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@smaeul it failed on
x86_64-gnu-llvm-7
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
x86_64-gnu-llvm-7
job only builds forx86_64-unknown-linux-gnu
. It doesn't look like it does anything with musl at all. So where did"--target=x86_64-unknown-linux-musl"
in the rustc arguments for this test come from?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see, the old version of the test had
compile-flags: --target=x86_64-unknown-linux-musl
in it. That was the problem. After removing that line, the test should work on all targets.So since the behavior tested here is not musl-specific, I would suggest 1) removing
only-musl
and 2) renaming the test to mentioncrt-static
instead ofmusl
.