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

Cargo fails to detect environment variable is newly set and rebuild if the environment variable is set in config.toml #13280

Closed
usamoi opened this issue Jan 11, 2024 · 6 comments · Fixed by #14701
Labels
A-dep-info Area: dep-info, .d files A-environment-variables Area: environment variables A-rebuild-detection Area: rebuild detection and fingerprinting C-bug Category: bug S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@usamoi
Copy link

usamoi commented Jan 11, 2024

Problem

The problem can be only reproduced if the environment variable is "not set" in the first run, and "set" in the second run.

Steps

Incorrect behaivor

.cargo/config.toml:

[env]
Z = "0"

src/main.rs:

fn main() {
    println!("{:?}", option_env!("Z"));
}

run these commands:

cargo run     # first build, output Some("0")
Z=1 cargo run # cargo does not notice the environment variable is set and runs it, output Some("0")

Correct behaivor if we do not use a config.toml

src/main.rs:

fn main() {
    println!("{:?}", option_env!("Z"));
}

run these commands:

cargo run     # first build, output None
Z=1 cargo run # cargo does know the environment variable is set and rebuilds it, output Some("1")

Correct behaivor if we change the environment variable from a value to another value

.cargo/config.toml:

[env]
Z = "0"

src/main.rs:

fn main() {
    println!("{:?}", option_env!("Z"));
}

run these commands:

Z=2 cargo run # first build, output Some("2")
Z=1 cargo run # cargo does know the environment variable changes and rebuilds it, output Some("1")

Possible Solution(s)

No response

Notes

No response

Version

cargo 1.77.0-nightly (ac6bbb332 2023-12-26)
cargo 1.75.0 (1d8b05cdd 2023-11-20)
@usamoi usamoi added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Jan 11, 2024
@usamoi usamoi changed the title Cargo fails to detect environment variable is set and rebuild if the environment is set in config.toml Cargo fails to detect environment variable is newly set and rebuild if the environment variable is set in config.toml Jan 11, 2024
@ehuss ehuss added A-rebuild-detection Area: rebuild detection and fingerprinting S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review A-environment-variables Area: environment variables and removed S-triage Status: This issue is waiting on initial triage. labels Jan 11, 2024
@heisen-li
Copy link
Contributor

heisen-li commented Jan 11, 2024

By the way, it may be related to question #10358 & #12434.

@weihanglo
Copy link
Member

By the way, it may be related to question #10358 & #12434.

Hmm… I don't quite think so. It is related to cargo's own dep-info file failed to track env vars from [env] table. Cargo removes those here:

on_disk_info
.env
.retain(|(key, _)| !rustc_cmd.get_envs().contains_key(key) || key == CARGO_ENV);

env vars from [env] were applied to rustc_cmd in fill_env() earlier before this call.

@weihanglo
Copy link
Member

Note: #13596 was an attempt of addressing this issue. In that PR, maintainers expected that kind of environment variable detection to be in the fingerprint code, where Cargo's dep-info logic is located.

@weihanglo weihanglo added the A-dep-info Area: dep-info, .d files label Oct 9, 2024
@weihanglo
Copy link
Member

#14154 is another case that Cargo's depinfo strips internal set environment variables (such as CARGO_PKG_README) causing the rebuild detection failure.

@linyihai
Copy link
Contributor

#14154 is another case that Cargo's depinfo strips internal set environment variables (such as CARGO_PKG_README) causing the rebuild detection failure.

If the user just wants the environment variables in the [env] table to trigger recompilation, can we just fix that

This is a likely solution, env_keys is a HashSet contains the key defined in [env] table

    on_disk_info.env.retain(|(key, _)| {
        !rustc_cmd.get_envs().contains_key(key) || key == CARGO_ENV || env_keys.contains(key)
    });

@weihanglo
Copy link
Member

@linyihai

That seems worth a try!

For #14154 we could reopen if a fix doesn't cover it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dep-info Area: dep-info, .d files A-environment-variables Area: environment variables A-rebuild-detection Area: rebuild detection and fingerprinting C-bug Category: bug S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
5 participants