Make Artifacts be compared to each other based on their exec paths (and not their paths). This gives a predictable order in places where Artifacts are sorted by their natural order.
This works because exec paths of Artifacts are unique in any given build.
--
MOS_MIGRATED_REVID=90807141
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 95bb3ab..4eb11ca 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
@@ -33,6 +33,7 @@
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Comparator;
import java.util.List;
import javax.annotation.Nullable;
@@ -68,7 +69,25 @@
@SkylarkModule(name = "File",
doc = "This type represents a file used by the build system. It can be "
+ "either a source file or a derived file produced by a rule.")
-public class Artifact implements FileType.HasFilename, Comparable<Artifact>, ActionInput {
+public class Artifact implements FileType.HasFilename, ActionInput {
+
+ /**
+ * Compares artifact according to their exec paths. Sorts null values first.
+ */
+ public static final Comparator<Artifact> EXEC_PATH_COMPARATOR = new Comparator<Artifact>() {
+ @Override
+ public int compare(Artifact a, Artifact b) {
+ if (a == b) {
+ return 0;
+ } else if (a == null) {
+ return -1;
+ } else if (b == null) {
+ return -1;
+ } else {
+ return a.execPath.compareTo(b.execPath);
+ }
+ }
+ };
/** An object that can expand middleman artifacts. */
public interface MiddlemanExpander {
@@ -366,19 +385,12 @@
return false;
}
// We don't bother to check root in the equivalence relation, because we
- // assume that 'root' is an ancestor of 'path', and that all possible roots
- // are disjoint, so unless things are really screwed up, it's ok.
+ // assume that no root is an ancestor of another one.
Artifact that = (Artifact) other;
return this.path.equals(that.path);
}
@Override
- public final int compareTo(Artifact o) {
- // The artifact factory ensures that there is a unique artifact for a given path.
- return this.path.compareTo(o.path);
- }
-
- @Override
public final int hashCode() {
return path.hashCode();
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
index 921e4e7..cc076e0 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
@@ -167,7 +167,7 @@
}
// The order of the artifacts.entrySet iteration is unspecified - we use a TreeMap here to
// guarantee that the return value of this method is deterministic.
- Map<Artifact, String> orphanArtifacts = new TreeMap<>();
+ Map<Artifact, String> orphanArtifacts = new TreeMap<>(Artifact.EXEC_PATH_COMPARATOR);
for (Map.Entry<Artifact, String> entry : artifacts.entrySet()) {
Artifact a = entry.getKey();
if (!a.isSourceArtifact() && !artifactsWithActions.contains(a)) {
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java b/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java
index 0ba34cb..c4b3f53 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java
@@ -85,10 +85,10 @@
PathFragment bPath = new PathFragment("src/b");
Artifact aArtifact = new Artifact(aPath, rootDir);
Artifact bArtifact = new Artifact(bPath, rootDir);
- assertEquals(-1, aArtifact.compareTo(bArtifact));
- assertEquals(0, aArtifact.compareTo(aArtifact));
- assertEquals(0, bArtifact.compareTo(bArtifact));
- assertEquals(1, bArtifact.compareTo(aArtifact));
+ assertEquals(-1, Artifact.EXEC_PATH_COMPARATOR.compare(aArtifact, bArtifact));
+ assertEquals(0, Artifact.EXEC_PATH_COMPARATOR.compare(aArtifact, aArtifact));
+ assertEquals(0, Artifact.EXEC_PATH_COMPARATOR.compare(bArtifact, bArtifact));
+ assertEquals(1, Artifact.EXEC_PATH_COMPARATOR.compare(bArtifact, aArtifact));
}
@Test
