Skip to content

Commit d22c22c

Browse files
lberkicopybara-github
authored andcommitted
Update a comment to reflect the middleman-less reality.
And also allay my fears about Bazel being incorrect; I didn't realize it worked this way. RELNOTES: None. PiperOrigin-RevId: 695682365 Change-Id: I30305a86a6cc00f3118755d3aff491b1291e9cb3
1 parent 38ad734 commit d22c22c

File tree

1 file changed

+5
-14
lines changed

1 file changed

+5
-14
lines changed

src/main/java/com/google/devtools/build/lib/actions/RunfilesArtifactValue.java

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -125,20 +125,11 @@ public void forEachTree(RunfilesConsumer<TreeArtifactValue> consumer)
125125
@Override
126126
public boolean equals(Object o) {
127127
// This method, seemingly erroneously, does not check whether the runfilesTree of the two
128-
// objects is equivalent. This is because it's costly (it involves flattening nested sets and
129-
// even if one caches a fingerprint, it's still a fair amount of CPU) and because it's
130-
// currently not necessary: RunfilesArtifactValue is only ever created as the SkyValue of
131-
// runfiles tree and those are special-cased in ActionCacheChecker (see
132-
// ActionCacheChecker.checkMiddlemanAction()): the checksum of a runfiles tree artifact is the
133-
// function of the checksum of all the artifacts on the inputs of the middleman action, which
134-
// includes both the artifacts the runfiles tree links to and the runfiles input manifest,
135-
// which in turn encodes the structure of the runfiles tree. The checksum of the runfiles tree
136-
// artifact is here as the "metadata" field, which *is* compared here, so the
137-
// RunfilesArtifactValues of two runfiles middlemen will be equals iff they represent the same
138-
// runfiles tree.
139-
//
140-
// TODO(b/304440811): The above is either not true or true for a different reason.
141-
// Check and update the code accordingly.
128+
// objects are equivalent. This is because it's unnecessary because the layout of the runfiles
129+
// tree is already factored into the equality decision in two ways:
130+
// - Through "metadata", which takes the layout into account (see computeDigest())
131+
// - Through the runfiles input manifest file, which is part of the runfiles tree, which
132+
// contains the exact mapping and whose digest is in "fileValues"
142133
if (this == o) {
143134
return true;
144135
}

0 commit comments

Comments
 (0)