-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Make the libstd build script smaller #78924
Conversation
Error: Label libs-impl can only be set by Rust team members Please let |
r? @KodrAus (rust_highfive has picked a reviewer for you, use r? to override) |
@rustbot modify labels: +T-compiler |
r? @scottmcm |
Cc @joshtriplett because of the change to how libc linked. |
Sorry, this is code I've never seen before and I have no experience in how these things are built, so I don't feel qualified to review this. I'll head to zulip and see if triage knows a better reviewer... |
@bjorn3 can you explain why the |
I'm probably a goodish reviewer here, or at least will be able to forward if needed. |
Oh I missed that you used explicit |
This seems correct to me. That said, I would not anticipate us removing the build script support from rustbuild in the near future, it seems unlikely for that to be all that beneficial and the work to remove build scripts in comparison seems quite high. That said, the changes here seem reasonable and like they improve the quality of the code in question a little. @bors r+ rollup=never for easier bisection |
📌 Commit 89db5d03c6adcaad3e6564280c2db65d1397a294 has been approved by |
It is nice to have, but I don't think it is worth the effort either. At least for now. Maybe miniz_oxide will remove the build script in a future release and maybe cargo will get native support for the remaining usages of the build script of libstd, but then the compiler_builtins use case will still be hard to do without a build script. |
⌛ Testing commit 89db5d03c6adcaad3e6564280c2db65d1397a294 with merge cab1b85235027c97ae850618bba022dc518c922a... |
💔 Test failed - checks-actions |
r=me with commits squashed |
8fa9a08
to
07f5a41
Compare
Remove all rustc-link-lib from the std build script. Also remove use of feature = "restricted-std" where not necessary.
07f5a41
to
6f3872a
Compare
splat @bors r=Mark-Simulacrum |
📌 Commit 6f3872a has been approved by |
⌛ Testing commit 6f3872a with merge 45b36722bf32c27018e6c2c3e91d9a40cdc17732... |
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-actions |
Spurious network error:
@bors retry |
☀️ Test successful - checks-actions |
Of all sysroot crates currently only compiler_builtins, miniz_oxide and std require a build script. compiler_builtins uses to conditionally enable certain features and possibly compile a C version (source), miniz_oxide only uses it to detect if liballoc is supported as the MSRV is 1.34.0 instead of the 1.36.0 which stabilized liballoc (source). std now only uses it to enable
freebsd12
when theRUST_STD_FREEBSD_12_ABI
env var is set, to determine ifrestricted-std
should be set, to set theSTD_ENV_ARCH
env var identical toCARGO_CFG_TARGET_ARCH
, and to unconditionally enablebacktrace_in_libstd
.If all build scripts were to be removed, it would be possible for rustc to completely compile it's own sysroot. It currently requires a rustc version that already has an available libstd to compile the build scripts. If rustc can completely compile it's own sysroot, rustbuild could be simplified to not forcefully use the bootstrap compiler for build scripts.
@rustbot modify labels: +T-compiler +libs-impl