rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.7k stars 2.41k forks source link

Should `build.rs` be compiled like plugins? #5436

Open ehuss opened 6 years ago

ehuss commented 6 years ago

Currently, build.rs scripts (and build-dependencies) are built with the same restrictions as compiler plugins (panic and lto cannot be set, maybe other issues?). I don't think this is quite correct because build scripts are not used the same way proc-macro/plugins are.

This is caused by several parts of the code that check for_host() but don't differentiate for build scripts (but some places do!).

One consequence of the current implementation is that a dependency that is shared between regular dependencies and build-dependencies will be built without panic set. I'm not sure if there is much of a problem with this because dependencies built with unwind seem to be fine? (I'd like to better understand the difference, since the assembly output looks almost identical.) Another oddity is that with cargo test or cargo bench, these shared dependencies will be built twice, but with the exact same settings.

Repro:

cargo new --lib foo
cd foo
cat >> Cargo.toml <<EOL
shared = {path="shared"}
[build-dependencies]
shared = {path="shared"}
[profile.dev]
panic = "abort"
EOL

cargo new --lib shared
echo "fn main() {}" > build.rs

cargo build -v   # <- Notice that neither `build.rs` or `shared` is built with panic.

I think to fix it so that build.rs includes panic, it is a relatively simple change to avoid walking the RunCustomBuild unit in build_used_in_plugin_map.

Alternatively, to keep the current behavior and fix the shared dependencies, we should be clearing the panic flag in get_profile (instead of build_base_args), though I'm uncertain what other issues there might be.

In general, it seems to be dangerous to use for_host() if the code is really wanting to know if something is a plugin.

alexcrichton commented 6 years ago

In general the tweaks to things like panic/LTO are primarily intended to affect the final artifact rather than intermediate artifacts so I don't think we necessarily need to try too too hard to have 100% fidelity with things like build scripts. For exaple if we compiled all build scripts with LTO that'd actually be pretty bad in the sense that it'd be a lot of lost build time for not a lot of gain.

I'd be fine tweaking the panic functionality, though, it seems like it probably wouldn't cause too many probles!