Skip to content

Commit

Permalink
Use ctime in file digest cache key
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fmeum committed Apr 6, 2023
1 parent f03d58d commit e6e6e50
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ private static class CacheKey {
/** File system identifier of the file (typically the inode number). */
private final long nodeId;

/** Creation time of the file. */
private final long createdTime;

/** Last modification time of the file. */
private final long modifiedTime;

Expand All @@ -62,6 +65,7 @@ private static class CacheKey {
public CacheKey(Path path, FileStatus status) throws IOException {
this.path = path.asFragment();
this.nodeId = status.getNodeId();
this.createdTime = status.getLastChangeTime();
this.modifiedTime = status.getLastModifiedTime();
this.size = status.getSize();
}
Expand All @@ -76,6 +80,7 @@ public boolean equals(Object object) {
CacheKey key = (CacheKey) object;
return path.equals(key.path)
&& nodeId == key.nodeId
&& createdTime == key.createdTime
&& modifiedTime == key.modifiedTime
&& size == key.size;
}
Expand All @@ -86,6 +91,7 @@ public int hashCode() {
int result = 17;
result = 31 * result + path.hashCode();
result = 31 * result + Longs.hashCode(nodeId);
result = 31 * result + Longs.hashCode(createdTime);
result = 31 * result + Longs.hashCode(modifiedTime);
result = 31 * result + Longs.hashCode(size);
return result;
Expand Down
7 changes: 7 additions & 0 deletions src/test/shell/bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1366,3 +1366,10 @@ sh_test(
],
tags = ["no_windows"],
)

sh_test(
name = "digest_cache_invalidation_test",
srcs = ["digest_cache_invalidation_test.sh"],
data = [":test-deps"],
deps = ["@bazel_tools//tools/bash/runfiles"],
)
53 changes: 53 additions & 0 deletions src/test/shell/integration/execution_phase_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -369,4 +369,57 @@ function test_track_directory_crossing_package() {
>& "$TEST_log" || fail "Expected success"
expect_log "WARNING: Directory artifact foo/dir crosses package boundary into"
}

# Regression test for https://github.com/bazelbuild/bazel/issues/14723
function test_fixed_mtime_move_detected_as_change() {
mkdir -p pkg
cat > pkg/BUILD <<'EOF'
load("rules.bzl", "my_expand")
genrule(
name = "my_templates",
srcs = ["template_archive.tar"],
outs = ["template1"],
cmd = "tar -C $(RULEDIR) -xf $<",
)
my_expand(
name = "expand1",
input = "template1",
output = "expanded1",
to_sub = {"test":"foo"}
)
EOF
cat > pkg/rules.bzl <<'EOF'
def _my_expand_impl(ctx):
ctx.actions.expand_template(
template = ctx.file.input,
output = ctx.outputs.output,
substitutions = ctx.attr.to_sub
)
my_expand = rule(
implementation = _my_expand_impl,
attrs = {
"input": attr.label(allow_single_file=True),
"output": attr.output(),
"to_sub" : attr.string_dict(),
}
)
EOF

echo "test : alpha" > template1
tar -cf pkg/template_archive_alpha.tar --mtime='1970-01-01' template1
echo "test : delta" > template1
tar -cf pkg/template_archive_delta.tar --mtime='1970-01-01' template1

mv pkg/template_archive_alpha.tar pkg/template_archive.tar
bazel build //pkg:expand1 || fail "Expected success"
assert_equals "foo : alpha" "$(cat bazel-bin/pkg/expanded1)"

mv pkg/template_archive_delta.tar pkg/template_archive.tar
bazel build //pkg:expand1 || fail "Expected success"
assert_equals "foo : delta" "$(cat bazel-bin/pkg/expanded1)"
}

run_suite "Integration tests of ${PRODUCT_NAME} using the execution phase."

0 comments on commit e6e6e50

Please sign in to comment.