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 26c0be6
Show file tree
Hide file tree
Showing 2 changed files with 62 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;

/** Last change time of the file. */
private final long changeTime;

/** 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.changeTime = 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
&& changeTime == key.changeTime
&& 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(changeTime);
result = 31 * result + Longs.hashCode(modifiedTime);
result = 31 * result + Longs.hashCode(size);
return result;
Expand Down
56 changes: 56 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,60 @@ 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
touch -t 197001010000 template1
tar -cf pkg/template_archive_alpha.tar template1

echo "test : delta" > template1
touch -t 197001010000 template1
tar -cf pkg/template_archive_delta.tar 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 26c0be6

Please sign in to comment.