-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Failed build script is not tracked in fingerprint. #6770
Comments
Jeez we just can't seem to get this right :(. I really would start seriously considering larger scale revamps because it's be clear that ever since inception we haven't been able to ever produce a bug-free system where incremental works correctly in all cases... Is the problem here that there's still a file on the filesystem which indicates a successful run, and so during the failing step above Cargo correctly realizes it doesn't need to rebuild the build script itself but incorrectly reuses the old successful output assuming the new binary succeeded? I sort of forget how this is solved for crates, but I vaguely remember us deleting files that is recognized specially (this could be totally wrong though). Do you recall off the top of your head how that works for normal crates? |
Correct. The build script fingerprint only tracks the For context, this was discovered by @eddyb working on the gll crate which has a number of modules in the build script and he was hitting the OOM killer (I think).
A normal crate would catch this from the dep-info file. The running of a build script is the odd one out. It requires build script authors to be extremely diligent in tracking all inputs, and I think we can ease that a little. I suspect I'm missing something, though. |
Hm ok, so it sounds like the build script execution is buggily not tracking the build script itself. In that one of the inputs to the build script execution should be the binary of the build script itself, which we should recognize has changed from the previous successful run which triggers a run until it succeeds. I do remember we fixed a number of issues like this by adding dependency edges from packages to their dependencies (in the fingerprints), so maybe we're just missing that for build script execution? |
…chton Fix fingerprint for canceled build script. Fixes #6770. See that issue for a description. The fix is to include the compilation of the build script itself in the fingerprint of the `RunCustomBuild` Unit. This also includes some documentation for how fingerprints work. Also includes a test for docker-style caching of dependencies where timestamps get truncated to the nearest second.
There is a scenario where cargo fails to rebuild when the execution of a build script is interrupted (such as hitting ctrl-c). There are a variety of ways this can happen, the test below illustrates one way.
The fundamental problem is that the fingerprint does not track enough information about the execution of a build script as a dependency. #6720 added tracking the
invoked.timestamp
file, but that is not sufficient.I've been pondering various ways to fix this, but not loving the options. I would be interested if anyone has any more ideas. Bonus points if both this and #6733 can be fixed. Some options:
build_script_local_fingerprints
, but there are some complications with updating that after the build is done (it would need to do everything thatprepare_build_cmd
does, too).invoked.timestamp.start
(and deleteinvoked.timestamp
) and then when the build script succeeds, rename it toinvoked.timestamp
. I think this should work, but I'm not sure.Example
(This currently fails on the last line where the build succeeds when it should fail.)
The text was updated successfully, but these errors were encountered: