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.rs: rerun-if-env-changed should mean "changed compared to last execution of the same kind" (check vs build/run) #7432

Open
Boscop opened this issue Sep 25, 2019 · 1 comment
Labels
A-build-scripts Area: build.rs scripts A-rebuild-detection Area: rebuild detection and fingerprinting C-bug Category: bug Command-check S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.

Comments

@Boscop
Copy link

Boscop commented Sep 25, 2019

Currently, it seems that build.rs scripts share the same environment when run by cargo build/cargo run and cargo check.
This is a problem which leads to unnecessary rebuilds of libsqlite3-sys whenever I have cargo watch -x run running in cmd.exe, when I then do cargo check in my editor (so that I can jump to errors/warnings easily).
This causes a rebuild of libsqlite3-sys and all its dependent crates in my workspace for every switch between cargo check and the cargo run started by cargo watch.
Because: libsqlite3-sys uses rerun-if-env-changed=PATH AND apparently cargo watch gives a different PATH to cargo run than cmd.exe has [1]. Because if I run cargo run normally (not through cargo watch) this doesn't happen.
The same happens when I run cargo run normally (not through cargo watch) but in Cmder instead of in cmd.exe, and here I know that it has a different PATH than my editor. In Sublime Text and in Cmder I ran the python command import os; os.environ['PATH'] to check, and indeed, Cmder adds some of his own folders to PATH, which is why I switched to using the bare cmd.exe (to avoid the recompilations of libsqlite3-sys and its dependent crates in my workspace, which takes several minutes every time). But in bare cmd.exe, the same unnecessary recompilations happen when using cargo watch -x run and doing cargo check in Sublime Text.

So currently, it seems that rerun-if-env-changed means, "changed, regardless of whether we also switched between check and build/run".
I suggest that rerun-if-env-changed should mean "changed compared to last execution of the same kind": If the build.rs script is currently being executed because of cargo check, compare the env var to the last time it was also executed by cargo check, NOT by cargo build.
And similarly for cargo build/run. build and run are treated the same, because run implies build. But check doesn't imply build, they also produce separate artifacts in the target dir. So in a similar vein, their env's should be kept separate with respect to rerun-if-env-changed!

[1]: Btw, it's not cargo-watch's fault that the PATH for its spawned process is different, it's cargo's: #7431

@Boscop Boscop added the C-bug Category: bug label Sep 25, 2019
@ehuss ehuss added A-build-scripts Area: build.rs scripts Command-check labels Oct 10, 2019
guusw added a commit to fragcolor-xyz/shards that referenced this issue Jul 1, 2022
Prevents having to work around unnecessary rebuils caused by rust-analyzer
rust-lang/cargo#7432
guusw added a commit to fragcolor-xyz/shards that referenced this issue Jul 1, 2022
Prevents having to work around unnecessary rebuils caused by rust-analyzer
rust-lang/cargo#7432
Kryptos-FR pushed a commit to fragcolor-xyz/shards that referenced this issue Jul 1, 2022
Prevents having to work around unnecessary rebuils caused by rust-analyzer
rust-lang/cargo#7432
guusw added a commit to fragcolor-xyz/shards that referenced this issue Jul 4, 2022
Prevents having to work around unnecessary rebuils caused by rust-analyzer
rust-lang/cargo#7432
guusw added a commit to fragcolor-xyz/shards that referenced this issue Jul 4, 2022
Prevents having to work around unnecessary rebuils caused by rust-analyzer
rust-lang/cargo#7432
@epage epage added the A-rebuild-detection Area: rebuild detection and fingerprinting label Nov 2, 2023
@epage
Copy link
Contributor

epage commented Nov 2, 2023

Whats not to clear to me is why this matters as we don't tell build scripts which is being done (see #4001)

@epage epage added the S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. label Nov 2, 2023
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 A-rebuild-detection Area: rebuild detection and fingerprinting C-bug Category: bug Command-check S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.
Projects
None yet
Development

No branches or pull requests

3 participants