diff options
author | felly <felly@google.com> | 2018-02-23 12:23:56 -0800 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-02-23 12:25:25 -0800 |
commit | 9f7eb09e53ed7720a5e0b038df67263e44c28101 (patch) | |
tree | bbecf5e070d191a7349d339fbdd6f75226fc1bef /src/main/java/com/google/devtools | |
parent | 054cf789556c36181196be391d47344186283bb5 (diff) |
Fix Fix crash on incremental builds across configuration changes when using Skyframe native Filesets which reference output files.
We were failing to override equality of Artifact to use the artifact owner. See the javadocs on ArtifactSkyKey for more discussion of this.
Before this change, the weak interning of keys done in LegacySkyKey and FilesetEntryKey spuriously matched keys across incremental builds in cases where Artifacts differed only in their owner.
PiperOrigin-RevId: 186805663
Diffstat (limited to 'src/main/java/com/google/devtools')
3 files changed, 30 insertions, 5 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java index 91b5313502..85873b27a0 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java @@ -645,6 +645,18 @@ public class Artifact return Objects.equals(this.execPath, that.execPath) && Objects.equals(this.root, that.root); } + /** + * Compare equality including Artifact owner equality, a notable difference compared to the + * {@link #equals(Object)} method of {@link Artifact}. + */ + public static boolean equalWithOwner(@Nullable Artifact first, @Nullable Artifact second) { + if (first != null) { + return first.equals(second) && first.getArtifactOwner().equals(second.getArtifactOwner()); + } else { + return second == null; + } + } + @Override public final int hashCode() { // This is just path.hashCode(). We cache a copy in the Artifact object to reduce LLC misses diff --git a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java index 7569544b9d..623888a1cc 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java +++ b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java @@ -98,6 +98,23 @@ public interface FilesetTraversalParams { return RootedPath.toRootedPath(getRootPart(), getRelativePart()); } + @Override + public boolean equals(Object o) { + if (o == this) { + return true; + } + if (o instanceof FilesetTraversalParams.DirectTraversalRoot) { + FilesetTraversalParams.DirectTraversalRoot that = + (FilesetTraversalParams.DirectTraversalRoot) o; + // Careful! We must compare the artifact owners, which the default {@link Artifact#equals()} + // method does not do. See the comments on {@link ArtifactSkyKey} and http://b/73738481. + return Artifact.equalWithOwner(this.getOutputArtifact(), that.getOutputArtifact()) + && (this.getRootPart().equals(that.getRootPart())) + && (this.getRelativePart().equals(that.getRelativePart())); + } + return false; + } + @Memoized @Override public abstract int hashCode(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactSkyKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactSkyKey.java index dd3aa91fea..96b8d1666f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactSkyKey.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactSkyKey.java @@ -111,10 +111,6 @@ public class ArtifactSkyKey implements SkyKey { return TO_ARTIFACT.apply((ArtifactSkyKey) key.argument()); } - public static boolean equalWithOwner(Artifact first, Artifact second) { - return first.equals(second) && first.getArtifactOwner().equals(second.getArtifactOwner()); - } - @Override public SkyFunctionName functionName() { return SkyFunctions.ARTIFACT; @@ -159,7 +155,7 @@ public class ArtifactSkyKey implements SkyKey { return false; } ArtifactSkyKey thatArtifactSkyKey = ((ArtifactSkyKey) that); - return equalWithOwner(artifact, thatArtifactSkyKey.artifact) + return Artifact.equalWithOwner(artifact, thatArtifactSkyKey.artifact) && isMandatory == thatArtifactSkyKey.isMandatory; } |