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

is_flag_supported does not respect emit_rerun_if_env_changed #1147

Closed
oconnor663 opened this issue Jul 12, 2024 · 1 comment
Closed

is_flag_supported does not respect emit_rerun_if_env_changed #1147

oconnor663 opened this issue Jul 12, 2024 · 1 comment

Comments

@oconnor663
Copy link
Contributor

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 BLAKE3-team/BLAKE3#324 and BLAKE3-team/BLAKE3#413, and I might file follow-up issues related to those.

@oconnor663 oconnor663 changed the title is_flag_supported does not respect emit_rerun_if_env_changed is_flag_supported does not respect emit_rerun_if_env_changed Jul 12, 2024
@oconnor663
Copy link
Contributor Author

#1148 was merged. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant