Unshare FileArtifactValues associated with constant-metadata Artifacts: they can't really be used across servers. PiperOrigin-RevId: 236567498
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java index b2f3384..f82e696 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
@@ -89,7 +89,10 @@ Runfiles runfiles = new Runfiles.Builder("workspace").addArtifact(artifact).build(); RunfilesSupplier supplier = new RunfilesSupplierImpl(PathFragment.create("runfiles"), runfiles); FakeActionInputFileCache mockCache = new FakeActionInputFileCache(); - mockCache.put(artifact, FileArtifactValue.createNormalFile(FAKE_DIGEST, 0)); + mockCache.put( + artifact, + FileArtifactValue.createNormalFile( + FAKE_DIGEST, /*proxy=*/ null, 0L, /*isShareable=*/ true)); expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, ArtifactPathResolver.IDENTITY, true); @@ -151,8 +154,14 @@ new Runfiles.Builder("workspace").addArtifact(artifact1).addArtifact(artifact2).build(); RunfilesSupplier supplier = new RunfilesSupplierImpl(PathFragment.create("runfiles"), runfiles); FakeActionInputFileCache mockCache = new FakeActionInputFileCache(); - mockCache.put(artifact1, FileArtifactValue.createNormalFile(FAKE_DIGEST, 1)); - mockCache.put(artifact2, FileArtifactValue.createNormalFile(FAKE_DIGEST, 2)); + mockCache.put( + artifact1, + FileArtifactValue.createNormalFile( + FAKE_DIGEST, /*proxy=*/ null, 1L, /*isShareable=*/ true)); + mockCache.put( + artifact2, + FileArtifactValue.createNormalFile( + FAKE_DIGEST, /*proxy=*/ null, 12L, /*isShareable=*/ true)); expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, ArtifactPathResolver.IDENTITY, true); @@ -175,7 +184,10 @@ .build(); RunfilesSupplier supplier = new RunfilesSupplierImpl(PathFragment.create("runfiles"), runfiles); FakeActionInputFileCache mockCache = new FakeActionInputFileCache(); - mockCache.put(artifact, FileArtifactValue.createNormalFile(FAKE_DIGEST, 1)); + mockCache.put( + artifact, + FileArtifactValue.createNormalFile( + FAKE_DIGEST, /*proxy=*/ null, 1L, /*isShareable=*/ true)); expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, ArtifactPathResolver.IDENTITY, true); @@ -196,7 +208,10 @@ .build(); RunfilesSupplier supplier = new RunfilesSupplierImpl(PathFragment.create("runfiles"), runfiles); FakeActionInputFileCache mockCache = new FakeActionInputFileCache(); - mockCache.put(artifact, FileArtifactValue.createNormalFile(FAKE_DIGEST, 1)); + mockCache.put( + artifact, + FileArtifactValue.createNormalFile( + FAKE_DIGEST, /*proxy=*/ null, 1L, /*isShareable=*/ true)); expander.addRunfilesToInputs(inputMappings, supplier, mockCache, NO_ARTIFACT_EXPANDER, ArtifactPathResolver.IDENTITY, true);
diff --git a/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java b/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java index 3d21a51..a746131 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java +++ b/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java
@@ -47,7 +47,10 @@ Path path = execRoot.getRelative(input.getExecPath()); FileStatus stat = path.stat(Symlinks.FOLLOW); return FileArtifactValue.createNormalFile( - HashCode.fromString(hexDigest).asBytes(), FileContentsProxy.create(stat), stat.getSize()); + HashCode.fromString(hexDigest).asBytes(), + FileContentsProxy.create(stat), + stat.getSize(), + /*isShareable=*/ true); } @Override
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java index eab3acf..d373868 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
@@ -54,7 +54,9 @@ @Test public void withNonArtifactInput() throws Exception { ActionInput input = ActionInputHelper.fromPath("foo/bar"); - FileArtifactValue metadata = FileArtifactValue.createNormalFile(new byte[] {1, 2, 3}, 10); + FileArtifactValue metadata = + FileArtifactValue.createNormalFile( + new byte[] {1, 2, 3}, /*proxy=*/ null, 10L, /*isShareable=*/ true); ActionInputMap map = new ActionInputMap(1); map.putWithNoDepOwner(input, metadata); assertThat(map.getMetadata(input)).isEqualTo(metadata); @@ -72,7 +74,9 @@ public void withArtifactInput() throws Exception { PathFragment path = PathFragment.create("src/a"); Artifact artifact = new Artifact(path, sourceRoot); - FileArtifactValue metadata = FileArtifactValue.createNormalFile(new byte[] {1, 2, 3}, 10); + FileArtifactValue metadata = + FileArtifactValue.createNormalFile( + new byte[] {1, 2, 3}, /*proxy=*/ null, 10L, /*isShareable=*/ true); ActionInputMap map = new ActionInputMap(1); map.putWithNoDepOwner(artifact, metadata); ActionMetadataHandler handler = new ActionMetadataHandler(
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileArtifactValueTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileArtifactValueTest.java index fbad99e..d689928 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FileArtifactValueTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileArtifactValueTest.java
@@ -14,7 +14,7 @@ package com.google.devtools.build.lib.skyframe; import static com.google.common.truth.Truth.assertThat; -import static com.google.devtools.build.lib.actions.FileArtifactValue.create; +import static com.google.devtools.build.lib.actions.FileArtifactValue.createShareable; import static org.junit.Assert.fail; import com.google.common.io.BaseEncoding; @@ -60,16 +60,37 @@ // and inequality with members of other equality groups. new EqualsTester() .addEqualityGroup( - FileArtifactValue.createNormalFile(toBytes("00112233445566778899AABBCCDDEEFF"), 1), - FileArtifactValue.createNormalFile(toBytes("00112233445566778899AABBCCDDEEFF"), 1)) + FileArtifactValue.createNormalFile( + toBytes("00112233445566778899AABBCCDDEEFF"), + /*proxy=*/ null, + 1L, + /*isShareable=*/ true), + FileArtifactValue.createNormalFile( + toBytes("00112233445566778899AABBCCDDEEFF"), + /*proxy=*/ null, + 1L, + /*isShareable=*/ true)) .addEqualityGroup( - FileArtifactValue.createNormalFile(toBytes("00112233445566778899AABBCCDDEEFF"), 2)) + FileArtifactValue.createNormalFile( + toBytes("00112233445566778899AABBCCDDEEFF"), + /*proxy=*/ null, + 2L, + /*isShareable=*/ true)) .addEqualityGroup(FileArtifactValue.createDirectory(1)) .addEqualityGroup( - FileArtifactValue.createNormalFile(toBytes("FFFFFF00000000000000000000000000"), 1)) + FileArtifactValue.createNormalFile( + toBytes("FFFFFF00000000000000000000000000"), + /*proxy=*/ null, + 1L, + /*isShareable=*/ true)) .addEqualityGroup( - FileArtifactValue.createDirectory(2), - FileArtifactValue.createDirectory(2)) + FileArtifactValue.createNormalFile( + toBytes("FFFFFF00000000000000000000000000"), + /*proxy=*/ null, + 1L, + /*isShareable=*/ false)) + .addEqualityGroup( + FileArtifactValue.createDirectory(2), FileArtifactValue.createDirectory(2)) .addEqualityGroup(FileArtifactValue.OMITTED_FILE_MARKER) .addEqualityGroup(FileArtifactValue.MISSING_FILE_MARKER) .addEqualityGroup(FileArtifactValue.DEFAULT_MIDDLEMAN) @@ -94,33 +115,33 @@ new EqualsTester() // We check for ctime and inode equality for paths. - .addEqualityGroup(create(path1)) - .addEqualityGroup(create(path2)) - .addEqualityGroup(create(mtimePath)) - .addEqualityGroup(create(digestPath)) - .addEqualityGroup(create(empty1)) - .addEqualityGroup(create(empty2)) - .addEqualityGroup(create(empty3)) + .addEqualityGroup(createShareable(path1)) + .addEqualityGroup(createShareable(path2)) + .addEqualityGroup(createShareable(mtimePath)) + .addEqualityGroup(createShareable(digestPath)) + .addEqualityGroup(createShareable(empty1)) + .addEqualityGroup(createShareable(empty2)) + .addEqualityGroup(createShareable(empty3)) // We check for mtime equality for directories. - .addEqualityGroup(create(dir1)) - .addEqualityGroup(create(dir2), create(dir3)) + .addEqualityGroup(createShareable(dir1)) + .addEqualityGroup(createShareable(dir2), createShareable(dir3)) .testEquals(); } @Test public void testCtimeInEquality() throws Exception { Path path = scratchFile("/dir/artifact1", 0L, "content"); - FileArtifactValue before = create(path); + FileArtifactValue before = createShareable(path); clock.advanceMillis(1); path.chmod(0777); - FileArtifactValue after = create(path); + FileArtifactValue after = createShareable(path); assertThat(before).isNotEqualTo(after); } @Test public void testNoMtimeIfNonemptyFile() throws Exception { Path path = scratchFile("/root/non-empty", 1L, "abc"); - FileArtifactValue value = create(path); + FileArtifactValue value = createShareable(path); assertThat(value.getDigest()).isEqualTo(path.getDigest()); assertThat(value.getSize()).isEqualTo(3L); try { @@ -134,7 +155,7 @@ @Test public void testDirectory() throws Exception { Path path = scratchDir("/dir", /*mtime=*/ 1L); - FileArtifactValue value = create(path); + FileArtifactValue value = createShareable(path); assertThat(value.getDigest()).isNull(); assertThat(value.getModifiedTime()).isEqualTo(1L); } @@ -144,7 +165,7 @@ public void testEmptyFile() throws Exception { Path path = scratchFile("/root/empty", 1L, ""); path.setLastModifiedTime(1L); - FileArtifactValue value = create(path); + FileArtifactValue value = createShareable(path); assertThat(value.getDigest()).isEqualTo(path.getDigest()); assertThat(value.getSize()).isEqualTo(0L); try { @@ -174,7 +195,7 @@ path.getParentDirectory().createDirectoryAndParents(); FileSystemUtils.writeContentAsLatin1(path, "content"); try { - create(path); + createShareable(path); fail(); } catch (IOException e) { assertThat(e).isSameAs(exception); @@ -184,7 +205,7 @@ @Test public void testUptodateCheck() throws Exception { Path path = scratchFile("/dir/artifact1", 0L, "content"); - FileArtifactValue value = create(path); + FileArtifactValue value = createShareable(path); clock.advanceMillis(1); assertThat(value.wasModifiedSinceDigest(path)).isFalse(); clock.advanceMillis(1); @@ -199,7 +220,7 @@ @Test public void testUptodateCheckDeleteFile() throws Exception { Path path = scratchFile("/dir/artifact1", 0L, "content"); - FileArtifactValue value = create(path); + FileArtifactValue value = createShareable(path); assertThat(value.wasModifiedSinceDigest(path)).isFalse(); path.delete(); assertThat(value.wasModifiedSinceDigest(path)).isTrue(); @@ -209,7 +230,7 @@ public void testUptodateCheckDirectory() throws Exception { // For now, we don't attempt to detect changes to directories. Path path = scratchDir("/dir", 0L); - FileArtifactValue value = create(path); + FileArtifactValue value = createShareable(path); assertThat(value.wasModifiedSinceDigest(path)).isFalse(); path.delete(); clock.advanceMillis(1); @@ -220,7 +241,7 @@ public void testUptodateChangeFileToDirectory() throws Exception { // For now, we don't attempt to detect changes to directories. Path path = scratchFile("/dir/file", 0L, ""); - FileArtifactValue value = create(path); + FileArtifactValue value = createShareable(path); assertThat(value.wasModifiedSinceDigest(path)).isFalse(); // If we only check ctime, then we need to change the clock here, or we get a ctime match on the // stat. @@ -239,7 +260,7 @@ @Test public void testIsMarkerValue_notMarker() throws Exception { - FileArtifactValue value = create(scratchFile("/dir/artifact1", 0L, "content")); + FileArtifactValue value = createShareable(scratchFile("/dir/artifact1", 0L, "content")); assertThat(value.isMarkerValue()).isFalse(); } }
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java index 4b55ffc..4ff3220 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
@@ -1003,7 +1003,7 @@ && ((Artifact) skyKey.argument()).isTreeArtifact()) { return TreeArtifactValue.create(allTreeFiles); } - return FileArtifactValue.create( + return FileArtifactValue.createShareable( ArtifactSkyKey.artifact((SkyKey) skyKey.argument()).getPath()); } catch (IOException e) { throw new SkyFunctionException(e, Transience.PERSISTENT){}; @@ -1017,8 +1017,7 @@ } public void addNewTreeFileArtifact(TreeFileArtifact input) throws IOException { - allTreeFiles.put(input, - FileArtifactValue.create(input.getPath())); + allTreeFiles.put(input, FileArtifactValue.createShareable(input.getPath())); } }
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 d7dcdcd..04b3211 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
@@ -106,7 +106,8 @@ // breaking changes. Map<String, FileArtifactValue> digestBuilder = new HashMap<>(); for (PathFragment child : children) { - FileArtifactValue subdigest = FileArtifactValue.create(tree.getPath().getRelative(child)); + FileArtifactValue subdigest = + FileArtifactValue.createShareable(tree.getPath().getRelative(child)); digestBuilder.put(child.getPathString(), subdigest); } assertThat(DigestUtils.fromMetadata(digestBuilder).getDigestBytesUnsafe())