rust-lang / cc-rs

Rust library for build scripts to compile C/C++ code into a Rust library
https://docs.rs/cc
Apache License 2.0
1.79k stars 434 forks source link

`is_flag_supported` does not respect `emit_rerun_if_env_changed` #1147

Closed oconnor663 closed 1 month ago

oconnor663 commented 1 month ago

In short, setting emit_rerun_if_env_changed(false) on a cc::Build does not prevent the is_flag_supported method from printing rerun-if-env-changed directives.

Here's a quick hello world example:

Cargo.toml

[package]
name = "scratch"
version = "0.1.0"
edition = "2021"

[build-dependencies]
cc = "1.1.0"

main.rs

fn main() {}

foo.c

void foo() {}

build.rs

fn main() {
    let mut build = cc::Build::new();
    build.file("foo.c");
    build.compile("foo");
}

If I run a verbose build and look at the rerun-if-env-changed directives, here's what I see:

$ touch build.rs && cargo build -vvv 2>&1 | grep rerun-if-env-changed
[scratch 0.1.0] cargo:rerun-if-env-changed=CC_x86_64-unknown-linux-gnu
[scratch 0.1.0] cargo:rerun-if-env-changed=CC_x86_64_unknown_linux_gnu
[scratch 0.1.0] cargo:rerun-if-env-changed=HOST_CC
[scratch 0.1.0] cargo:rerun-if-env-changed=CC
[scratch 0.1.0] cargo:rerun-if-env-changed=CC_ENABLE_DEBUG_OUTPUT
[scratch 0.1.0] cargo:rerun-if-env-changed=CRATE_CC_NO_DEFAULTS
[scratch 0.1.0] cargo:rerun-if-env-changed=CFLAGS_x86_64-unknown-linux-gnu
[scratch 0.1.0] cargo:rerun-if-env-changed=CFLAGS_x86_64_unknown_linux_gnu
[scratch 0.1.0] cargo:rerun-if-env-changed=HOST_CFLAGS
[scratch 0.1.0] cargo:rerun-if-env-changed=CFLAGS
[scratch 0.1.0] cargo:rerun-if-env-changed=AR_x86_64-unknown-linux-gnu
[scratch 0.1.0] cargo:rerun-if-env-changed=AR_x86_64_unknown_linux_gnu
[scratch 0.1.0] cargo:rerun-if-env-changed=HOST_AR
[scratch 0.1.0] cargo:rerun-if-env-changed=AR
[scratch 0.1.0] cargo:rerun-if-env-changed=ARFLAGS_x86_64-unknown-linux-gnu
[scratch 0.1.0] cargo:rerun-if-env-changed=ARFLAGS_x86_64_unknown_linux_gnu
[scratch 0.1.0] cargo:rerun-if-env-changed=HOST_ARFLAGS
[scratch 0.1.0] cargo:rerun-if-env-changed=ARFLAGS

Now, if I add build.emit_rerun_if_env_changed(false) to build.rs, here's what I see:

$ touch build.rs && cargo build -vvv 2>&1 | grep rerun-if-env-changed
[scratch 0.1.0] cargo:rerun-if-env-changed=CC_ENABLE_DEBUG_OUTPUT

That's maybe kinda a bug, since I expected to see nothing. It looks like CC_ENABLE_DEBUG_OUTPUT is hardcoded in a different place than the rest of the variables (which go through getenv). But the main bug I want to report is what happens if we add a call to is_flag_supported, like this:

build.rs

fn main() {
    let mut build = cc::Build::new();
    build.emit_rerun_if_env_changed(false);
    build.file("foo.c");
    build.is_flag_supported("-g").unwrap();
    build.compile("foo");
}
$ touch build.rs && cargo build -vvv 2>&1 | grep rerun-if-env-changed
[scratch 0.1.0] cargo:rerun-if-env-changed=CC_ENABLE_DEBUG_OUTPUT
[scratch 0.1.0] cargo:rerun-if-env-changed=CC_ENABLE_DEBUG_OUTPUT
[scratch 0.1.0] cargo:rerun-if-env-changed=CRATE_CC_NO_DEFAULTS
[scratch 0.1.0] cargo:rerun-if-env-changed=CFLAGS_x86_64-unknown-linux-gnu
[scratch 0.1.0] cargo:rerun-if-env-changed=CFLAGS_x86_64_unknown_linux_gnu
[scratch 0.1.0] cargo:rerun-if-env-changed=HOST_CFLAGS
[scratch 0.1.0] cargo:rerun-if-env-changed=CFLAGS

BUG: Now a lot of rerun-if-env-changed directives are making it out, even though we set build.emit_rerun_if_env_changed(false) early on.

I think the cause of this is that is_flag_supported calls Build::new internally, and the inner Build doesn't inherit this setting. I'll open a PR with a targeted fix for this issue, but we might want to consider something that would make it harder for similar mistakes to come up in the future.

This came up while I was investigating https://github.com/BLAKE3-team/BLAKE3/issues/324 and https://github.com/BLAKE3-team/BLAKE3/pull/413, and I might file follow-up issues related to those.

oconnor663 commented 1 month ago

https://github.com/rust-lang/cc-rs/pull/1148 was merged. Closing.