diff options
author | 2018-08-06 09:46:12 -0700 | |
---|---|---|
committer | 2018-08-06 09:47:54 -0700 | |
commit | 32a97a653735cf7c9eff5abdf0f7632f9a6525e2 (patch) | |
tree | 19fbf98de12c4125ef39312cfde5622ec41d2541 /src | |
parent | e6cd3f836171d5db7bcc955b8bab8556cde3cc96 (diff) |
Order tree artifacts.
Fixes #5686.
RELNOTES: None
PiperOrigin-RevId: 207559658
Diffstat (limited to 'src')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java | 26 | ||||
-rw-r--r-- | src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java | 23 |
2 files changed, 36 insertions, 13 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java index 13ff08a65f..aca26775a1 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java @@ -13,13 +13,12 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; -import com.google.common.base.Function; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; -import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.cache.DigestUtils; @@ -29,6 +28,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyValue; import java.io.IOException; import java.util.Arrays; +import java.util.Comparator; import java.util.Map; import java.util.Set; import javax.annotation.Nullable; @@ -39,21 +39,14 @@ import javax.annotation.Nullable; */ @AutoCodec class TreeArtifactValue implements SkyValue { - private static final Function<Artifact, PathFragment> PARENT_RELATIVE_PATHS = - new Function<Artifact, PathFragment>() { - @Override - public PathFragment apply(Artifact artifact) { - return artifact.getParentRelativePath(); - } - }; private final byte[] digest; - private final Map<TreeFileArtifact, FileArtifactValue> childData; + private final ImmutableSortedMap<TreeFileArtifact, FileArtifactValue> childData; @AutoCodec.VisibleForSerialization TreeArtifactValue(byte[] digest, Map<TreeFileArtifact, FileArtifactValue> childData) { this.digest = digest; - this.childData = ImmutableMap.copyOf(childData); + this.childData = sortedCopy(childData); } /** @@ -69,7 +62,13 @@ class TreeArtifactValue implements SkyValue { return new TreeArtifactValue( DigestUtils.fromMetadata(digestBuilder).getDigestBytesUnsafe(), - ImmutableMap.copyOf(childFileValues)); + sortedCopy(childFileValues)); + } + + private static ImmutableSortedMap<TreeFileArtifact, FileArtifactValue> sortedCopy( + Map<TreeFileArtifact, FileArtifactValue> data) { + return ImmutableSortedMap.copyOf(data, + Comparator.comparing(TreeFileArtifact::getParentRelativePath)); } FileArtifactValue getSelfData() { @@ -81,7 +80,8 @@ class TreeArtifactValue implements SkyValue { } Set<PathFragment> getChildPaths() { - return ImmutableSet.copyOf(Iterables.transform(childData.keySet(), PARENT_RELATIVE_PATHS)); + return ImmutableSet.copyOf(Iterables.transform(childData.keySet(), + TreeFileArtifact::getParentRelativePath)); } @Nullable diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java index 0f5d21bca7..15e7b82168 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java @@ -55,9 +55,14 @@ import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import java.io.IOException; import java.util.Arrays; +import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Random; +import java.util.stream.Collectors; +import java.util.stream.IntStream; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -124,6 +129,24 @@ public class TreeArtifactMetadataTest extends ArtifactFunctionTestCase { } @Test + public void testTreeArtifactOrdering() throws Exception { + int rangeSize = 100; + int attempts = 10; + List<PathFragment> children = + IntStream.range(0, rangeSize) + .mapToObj(i -> PathFragment.create("file" + i)) + .collect(Collectors.toList()); + + for (int i = 0; i < attempts; i++) { + Collections.shuffle(children, new Random()); + Artifact treeArtifact = createTreeArtifact("out"); + TreeArtifactValue value = evaluateTreeArtifact(treeArtifact, children); + assertThat(value.getChildPaths()).containsExactlyElementsIn(children); + assertThat(value.getChildPaths()).isOrdered(Comparator.naturalOrder()); + } + } + + @Test public void testEqualTreeArtifacts() throws Exception { Artifact treeArtifact = createTreeArtifact("out"); ImmutableList<PathFragment> children = |