diff options
3 files changed, 26 insertions, 14 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 95bb3abbfa..4eb11cad0c 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 com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.Collection; +import java.util.Comparator; import java.util.List; import javax.annotation.Nullable; @@ -68,7 +69,25 @@ import javax.annotation.Nullable; @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 @@ public class Artifact implements FileType.HasFilename, Comparable<Artifact>, Act 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 921e4e7164..cc076e00b8 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 @@ public class CachingAnalysisEnvironment implements AnalysisEnvironment { } // 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 0ba34cb057..c4b3f53d28 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 @@ public class ArtifactTest { 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 |