Skip to content
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

Build scripts are not informed of rustflags or rustc wrapper #9600

Closed
jonhoo opened this issue Jun 18, 2021 · 3 comments · Fixed by #9601
Closed

Build scripts are not informed of rustflags or rustc wrapper #9600

jonhoo opened this issue Jun 18, 2021 · 3 comments · Fixed by #9601
Labels
A-build-scripts Area: build.rs scripts C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`

Comments

@jonhoo
Copy link
Contributor

jonhoo commented Jun 18, 2021

Describe the problem you are trying to solve

I recently came across a bug in anyhow whose root cause is that it does "test compile" to see whether it can make use of some nightly features, but that test compile does not take into account Cargo configuration like [build] rustflags = . Looking through the environment variables Cargo sets for build scripts it doesn't appear that things like the rustc wrapper or the final rustflags is actually passed to the build script. Specifically, this can be seen by running

cargo new cargo-build-no-flags
cd cargo-build-no-flags
mkdir .cargo
cat >.cargo/config.toml <<EOF
[build]
rustflags = ["--cfg=wrapped"]
rustc-wrapper = "$PWD/.cargo/wrap.sh"
EOF
cat > .cargo/wrap.sh <<'EOF'
#!/bin/sh
rustc=$1
shift
exec "$rustc" "$@"
EOF
chmod +x .cargo/wrap.sh
cat > build.rs <<EOF
fn main() {
    for var in std::env::vars() {
        eprintln!("{:?}", var);
    }
}
EOF
cargo check -vv 2>&1 | grep '^\[cargo-build-no-flags' | tee build-output.log

Then, observe that "wrap", which appears in both build.rustflags and build.rustc-wrapper, does not appear in the build output:

$ grep wrap build-output.log
$

Describe the solution you'd like

The rust compiler is given to the build script in the form of the RUSTC environment variable, but that alone is not sufficient for a build script that wants to inspect (or use) the target Rust compilation context. The rustc configuration (the wrapper and RUSTFLAGS) should be passed to the build script as well, likely through two new environment variables:

  • CARGO_RUSTC_WRAPPER
  • CARGO_RUSTFLAGS

Note that these do not include _BUILD to avoid overlap with the environment variables Cargo reads.

@jonhoo jonhoo added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Jun 18, 2021
@cuviper
Copy link
Member

cuviper commented Jun 18, 2021

Describe the solution you'd like

The rust compiler is given to the build script in the form of the RUSTC environment variable, but that alone is not sufficient for a build script that wants to inspect (or use) the target Rust compilation context. The rustc configuration (the wrapper and RUSTFLAGS) should be passed to the build script as well, likely through two new environment variables:

  • CARGO_RUSTC_WRAPPER
  • CARGO_RUSTFLAGS

So those would be the target-specific flags, right? (i.e. matching the TARGET that cargo also sets for the build script.)

This would be a real boon to autocfg! (cc cuviper/autocfg#15 cuviper/autocfg#34)

@jonhoo
Copy link
Contributor Author

jonhoo commented Jun 18, 2021

Yes, my thinking was that they'd be set based on how Cargo decides to build the build script itself. See also the implementation of this in #9601, which is really quite simple.

@cuviper
Copy link
Member

cuviper commented Jun 18, 2021

Yes, my thinking was that they'd be set based on how Cargo decides to build the build script itself.

I don't think that's quite the same thing, since the build script runs on the host. But AFAICT the call you added does use the current bcx.target_data, the same that TARGET is set from, so it should be what I want.

jonhoo pushed a commit to jonhoo/anyhow that referenced this issue Jun 18, 2021
Without this, environments that configure `RUSTFLAGS` or the
`RUSTC_WRAPPER` in ways that break compilation with `backtrace` will
fail to compile anyhow (see dtolnay#156).

With this, the compiler probe takes into account that Cargo
configuration, and thus (more) accurately represents whether the
`backtrace` feature can be safely enabled.

Requires rust-lang/cargo#9600.

Fixes dtolnay#156.
@ehuss ehuss added the A-build-scripts Area: build.rs scripts label Jul 2, 2021
bors added a commit that referenced this issue Jul 20, 2021
Inform build scripts of rustc compiler context

Fixes #9600.
@bors bors closed this as completed in 6ee606b Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-scripts Area: build.rs scripts C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants