Cargo does not rebuild if project got changed in the same second #5918
Comments
|
Related: #2426 "cargo build does not rebuild if a source file was modified during a build" Personally I think cargo should only rebuild a project if the contents change, not if the timestamp changes, so I feel compelled to say I disagree that it's a bug that it doesn't "even" recompile the project each time. 😀 But more to your point, perhaps cargo could trigger a rebuild if the current time and the last build time match. |
|
That's what we have incremental compilation for. Please don't make it even harder to test that.^^ ;) I proposed a fix for this at #5919 |
|
I think I see what you mean: cargo should be dumb and use file timestamps, and let the incremental compilation within rustc to shortcut when no recompilation is necessary? TIL! |
fix cargo not doing anything when the input and output mtimes are equal That's a problem as the input may have changed in that same second but after the output got generated! Fixes #5918
|
We've ended up deciding to revert the fix for this in #6484, so I'm gonna reopen this to track a fix again. From the discussion there here's some points:
It was decided to back the fix for this out because this use case seemed a bit more niche than editing a file during a build (which typically takes much longer). A long-term fix for this will probably involve cryptographically hashing files. My thinking for this is:
There's been bugs in Cargo as well historically requesting cryptographic hashes always be used but we have been wary to do that for performance. This would naturally open up a possibility of having a config option to always use cryptographic checks and never use fast mtime checks. |
|
I don't understand why spurious rebuilds are traded for spuriously failing to notice changes -- IOW, spuriously doing too much work is traded for spuriously doing the wrong thing. :( You talk about EDIT: Hm, okay, #2426 does list some realistic scenarios where this might occur. And it's a correctness issue there as well, not just missing an optimization. :/ Can you at least provide guidance about how to run benchmarks for rebuilds after a NOP edit, now that the obvious way got broken again? I mean, I can hope I never have to debug rustc incremental perf regressions again, but someobody will and then this landmine will be right there waiting for them.
Won't that hash the wrong thing if the file changed during the rustc invocation? |
|
Yeah you have to hash first. But with a long build or tests I've often changed some sources mid build and then had to touch files to get the edited files to actually recompile. No disrespect intended but benchmarking incremental builds seems like the niche case. One we'd want to find a way to support, but with some niche way to subvert the noop'ing (--rebuild or --assume-stale or something.) |
Fair enough. I have a reflex pretty deep in my muscle memory where I don't hit Ctrl-S while a build is ongoing, I guess that's why I never saw that.^^ |
ehuss
added
the
A-caching
…e, r=alexcrichton Fix rebuild_sub_package_then_while_package on HFS. This test was flaky on HFS ([azure failure](https://dev.azure.com/rust-lang/cargo/_build/results?buildId=20144&view=logs&j=a5e52b91-c83f-5429-4a68-c246fc63a4f7&t=d4864165-4be3-5e34-b483-a6b05303aa68&l=2018)), resulting in this error: ``` Compiling foo v0.0.1 (/Users/runner/runners/2.164.7/work/1/s/target/cit/t750/foo) error[E0460]: found possibly newer version of crate `b` which `a` depends on --> src/lib.rs:1:1 | 1 | extern crate a; extern crate b; pub fn toplevel() {} | ^^^^^^^^^^^^^^^ | = note: perhaps that crate needs to be recompiled? = note: the following crate versions were found: crate `b`: /Users/runner/runners/2.164.7/work/1/s/target/cit/t750/foo/target/debug/deps/libb-98160c67a5811c37.rlib crate `b`: /Users/runner/runners/2.164.7/work/1/s/target/cit/t750/foo/target/debug/deps/libb-98160c67a5811c37.rmeta crate `a`: /Users/runner/runners/2.164.7/work/1/s/target/cit/t750/foo/target/debug/deps/liba-7d2b9ccd932a36e9.rmeta ``` There are two race-condition bugs here. Race 1: The second cargo build command (`cargo build -pb`) would sometimes not build, because it thought `b` is fresh. This can happen when the first build finishes and changing `b/src/lib.rs` happen within the same second. (#5918) The test silently ignored this failure, this is not the cause of the CI failure, though. Race 2: The first and second build commands work as expected. The third build command fails because it thinks both `a` and `b` are "fresh". However, `b` was just rebuilt, and `a` depends on it, so `a` should have been rebuilt. It thinks `a` is fresh because `a`'s mtime is equal to `b` when `b` finishes compiling within the same second that the first build finished. The solution here is to make sure the second step happens well after the first. The underlying problem is #5918.
|
#6529 would fix this. |
To reproduce, take a small crate (I am using https://github.com/rust-lang-nursery/rustc-perf/tree/master/collector/benchmarks/coercions), and run
These should give fairly consistent timings. However, they do not:
As you can see, it does not even recompile the project each time! Seems like it is (a) using a one-second granularity, and (b) if the last change and the last build happened in the same second, it assumes the build was first! The latter is a wrong assumption, leading to this bug.
The text was updated successfully, but these errors were encountered: