[5.4.1] Use ctime in file digest cache key #18102
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File digests are now additionally keyed by ctime for supported file system implementations. Since Bazel has a non-zero default for `--cache_computed_file_digests`, this may be required for correctness in cases where different files have identical mtime and inode number. For example, this can happen on Linux when files are extracted from a tar file with fixed mtime and are then replaced with `mv`, which preserves inodes. Since Java (N)IO doesn't have support for reading file ctimes on Windows, a new method backed by a native implementation is added to `WindowsFileOperation`. Adding a call to this function to `stat` uncovered previously silent bugs where Unix-style `PathFragment`s were created on Windows: 1. Bzlmod's `createLocalRepoSpec` did not correctly extract the path from a registry's `file://` URI on Windows. 2. `--package_path` isn't usable with absolute paths on Windows as it splits on `:`. Since the flag is deprecated, this commit fixes the tests rather than the implementation. Fixes #14723 Closes #18003. PiperOrigin-RevId: 524297459 Change-Id: I96bfc0210e2f71bf8603c7b7cc5eb06a04048c85
keertk
added
team-ExternalDeps
|
@Wyverald @meteorcloudy there are some problems with Please lmk how to proceed. Thanks! |
|
/cc @fmeum Can you help backport this change properly to 5.4.1? |
|
@meteorcloudy @keertk I created #18115. |
|
Thanks @fmeum! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
File digests are now additionally keyed by ctime for supported file system implementations. Since Bazel has a non-zero default for
--cache_computed_file_digests, this may be required for correctness in cases where different files have identical mtime and inode number. For example, this can happen on Linux when files are extracted from a tar file with fixed mtime and are then replaced withmv, which preserves inodes.Since Java (N)IO doesn't have support for reading file ctimes on Windows, a new method backed by a native implementation is added to
WindowsFileOperation. Adding a call to this function tostatuncovered previously silent bugs where Unix-stylePathFragments were created on Windows:createLocalRepoSpecdid not correctly extract the path from a registry'sfile://URI on Windows.--package_pathisn't usable with absolute paths on Windows as it splits on:. Since the flag is deprecated, this commit fixes the tests rather than the implementation.Fixes #14723
Closes #18003.
Commit 763f1d9
PiperOrigin-RevId: 524297459
Change-Id: I96bfc0210e2f71bf8603c7b7cc5eb06a04048c85