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

warn if rerun-if-changed=PATH is used but PATH points to none-existing file #6003

Open
matthiaskrgr opened this issue Sep 10, 2018 · 3 comments
Labels
A-build-scripts Area: build.rs scripts A-diagnostics Area: Error and warning messages generated by Cargo itself. A-rebuild-detection Area: rebuild detection and fingerprinting C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing.

Comments

@matthiaskrgr
Copy link
Member

I recently ran into a weird bug where my project would always rebuild main.rs even though I didn't change any of its files.

Turned out in build.rs I had rerun-if-changed=PATH set to a wrong PATH, a PATH that never was any file, which caused to trigger rebuilds every time.

Can we print a warning if the PATH is empty, or are there cases where this could be intended?

@matthiaskrgr matthiaskrgr changed the title warn if rerun-if-changed=PATH is used but PATH is empty warn if rerun-if-changed=PATH is used but PATH points to none-existing file Sep 10, 2018
@alexcrichton alexcrichton added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Sep 10, 2018
matthiaskrgr added a commit to matthiaskrgr/cargo that referenced this issue Sep 13, 2018
@remysucre
Copy link

Another reasonable behavior (perhaps on top of the warning) is to rebuild only when the file is removed between builds or is created just before a new build (i.e. the file is "modified" in the sense that it went from existent to non-existent or vice versa).

@ehuss ehuss added A-build-scripts Area: build.rs scripts A-rebuild-detection Area: rebuild detection and fingerprinting labels Apr 6, 2020
@dtolnay
Copy link
Member

dtolnay commented Mar 13, 2021

I agree with @remysucre that a warning is not necessarily appropriate.

I came across the spurious rebuilding behavior described in this issue in the case of https://github.com/rust-lang/crates.io — they have a cargo:rerun-if-changed=.env but a .env file is not checked into the repo, in fact it is ignored via gitignore.

The behavior I would have expected is that Cargo remembers which rerun-if-changed files did not exist in the most recent build, and does not rebuild if they still do not exist. Could we consider the continual rebuilding behavior a bug?

crates.io$  cargo check
...
...
    Finished dev [unoptimized + debuginfo] target(s) in 21.91s

crates.io$  cargo check
   Compiling cargo-registry v0.2.2
    Finished dev [unoptimized + debuginfo] target(s) in 3.21s

crates.io$  cargo check
   Compiling cargo-registry v0.2.2
    Finished dev [unoptimized + debuginfo] target(s) in 3.24s

crates.io$  cargo check                       # each of these is a rebuild
   Compiling cargo-registry v0.2.2
    Finished dev [unoptimized + debuginfo] target(s) in 3.19s

crates.io$  touch .env

crates.io$  cargo check
   Compiling cargo-registry v0.2.2
    Finished dev [unoptimized + debuginfo] target(s) in 3.20s

crates.io$  cargo check
    Finished dev [unoptimized + debuginfo] target(s) in 0.12s

crates.io$  cargo check
    Finished dev [unoptimized + debuginfo] target(s) in 0.12s

crates.io$  cargo check                       # no longer rebuilding now that .env exists
    Finished dev [unoptimized + debuginfo] target(s) in 0.12s

@ehuss
Copy link
Contributor

ehuss commented Mar 17, 2021

There is a small backwards compatibility hazard for changing the behavior, if any project is relying on the way it works now. For example, rerun-if-changed=phony to force a rebuild every time. (similar to a Makefile .PHONY target)

I tried searching through crates.io, and I only found one that seemed to be doing this here.

I found a few others that do cargo:rerun-if-changed= without a value, which was a bit of a surprise. However, I don't think that would affect this issue (it seems like cargo treats that as "scan the whole directory").

I'm not sure if that's a big enough concern to block changing this behavior. I agree it is probably not the best behavior. A warning is an option, though I'm uncertain how likely that could trigger a false positive.

It won't be trivial to implement because cargo does not track the status of every file (it just tracks the newest mtime).

@epage epage added the S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. label Oct 24, 2023
@epage epage added the A-diagnostics Area: Error and warning messages generated by Cargo itself. label Nov 3, 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-diagnostics Area: Error and warning messages generated by Cargo itself. A-rebuild-detection Area: rebuild detection and fingerprinting C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing.
Projects
None yet
Development

No branches or pull requests

6 participants