Add an archived representation for tree artifacts.
Tree artifacts are currently passed as individual files which are in the
directory. This change is a prototype which allows to pass those directories
using a single archived file instead.
Add a new option which, if enabled, assigns an archived representation to each
of the tree artifacts. The representation is a single file, which contains all
of the files in the artifact (zip archive). Those files are added in a
directory using a reserved name (containing a `:`), therefore cannot clash with
user-created files.
Expand the TreeArtifactValue to enable storing the archived representation and
update the inputs mapping code to include the archived file.
Add handling for changes to the archived files in `FilesystemValueChecker` to
ensure correctness in case of incremental builds, when the files get
edited/deleted.
Add explicit errors for use cases for which archived tree artifacts are not
supported yet like inputs discovery, shared actions, action templates or
discarding orphaned artifacts.
Please note that this feature relies on spawn runners to create the artifact
itself -- using it with one which does not support it will result with an error
(missing tree artifact output).
Add a new, parameterized category of tests for `FilesystemValueChecker` to
cover handling of archived files. Improve reliability of
`FilesystemValueChecker` by replacing the clock used to generate file `ctimes`
for files with a `ManualClock` (previously `JavaClock`, using
`System.currentTimeMillis()`).
PiperOrigin-RevId: 327472179
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
index 5bd1ed4..cae0cb1 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
@@ -24,7 +24,6 @@
import com.google.common.util.concurrent.Runnables;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionLookupData;
-import com.google.devtools.build.lib.actions.ActionLookupKey;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
@@ -38,7 +37,6 @@
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ServerDirectories;
import com.google.devtools.build.lib.clock.BlazeClock;
-import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.events.NullEventHandler;
@@ -66,7 +64,6 @@
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.devtools.build.lib.vfs.UnixGlob;
-import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
import com.google.devtools.build.skyframe.Differencer.Diff;
import com.google.devtools.build.skyframe.EvaluationContext;
import com.google.devtools.build.skyframe.EvaluationResult;
@@ -97,7 +94,7 @@
/** Tests for {@link FilesystemValueChecker}. */
@RunWith(JUnit4.class)
-public final class FilesystemValueCheckerTest {
+public final class FilesystemValueCheckerTest extends FilesystemValueCheckerTestBase {
private static final EvaluationContext EVALUATION_OPTIONS =
EvaluationContext.newBuilder()
.setKeepGoing(false)
@@ -108,16 +105,12 @@
private RecordingDifferencer differencer;
private MemoizingEvaluator evaluator;
private SequentialBuildDriver driver;
- private MockFileSystem fs;
private Path pkgRoot;
- private static final int FSVC_THREADS_FOR_TEST = 200;
-
@Before
public final void setUp() throws Exception {
ImmutableMap.Builder<SkyFunctionName, SkyFunction> skyFunctions = ImmutableMap.builder();
- fs = new MockFileSystem();
pkgRoot = fs.getPath("/testroot");
pkgRoot.createDirectoryAndParents();
FileSystemUtils.createEmptyFile(pkgRoot.getRelative("WORKSPACE"));
@@ -318,9 +311,10 @@
// their ctime.
TimestampGranularityUtils.waitForTimestampGranularity(
System.currentTimeMillis(), OutErr.SYSTEM_OUT_ERR);
- // Update path1's contents and mtime. This will update the file's ctime.
+ // Update path1's contents. This will update the file's ctime with current time indicated by the
+ // clock.
+ fs.advanceClockMillis(1);
FileSystemUtils.writeContentAsLatin1(path1, "hello1");
- path1.setLastModifiedTime(27);
// Update path2's mtime but not its contents. We expect that an mtime change suffices to update
// the ctime.
path2.setLastModifiedTime(42);
@@ -387,9 +381,8 @@
FileSystemUtils.writeContentAsLatin1(out2.getPath(), "fizzlepop");
TimestampGranularityMonitor tsgm = new TimestampGranularityMonitor(BlazeClock.instance());
- ActionLookupKey actionLookupKey = new SimpleActionLookupKey();
- SkyKey actionKey1 = ActionLookupData.create(actionLookupKey, 0);
- SkyKey actionKey2 = ActionLookupData.create(actionLookupKey, 1);
+ SkyKey actionKey1 = ActionLookupData.create(ACTION_LOOKUP_KEY, 0);
+ SkyKey actionKey2 = ActionLookupData.create(ACTION_LOOKUP_KEY, 1);
pretendBuildTwoArtifacts(out1, actionKey1, out2, actionKey2, batchStatter, tsgm);
@@ -516,12 +509,11 @@
SpecialArtifact last = createTreeArtifact("zzzzzzzzzz");
last.getPath().createDirectoryAndParents();
- ActionLookupKey actionLookupKey = new SimpleActionLookupKey();
- SkyKey actionKey1 = ActionLookupData.create(actionLookupKey, 0);
- SkyKey actionKey2 = ActionLookupData.create(actionLookupKey, 1);
- SkyKey actionKeyEmpty = ActionLookupData.create(actionLookupKey, 2);
- SkyKey actionKeyUnchanging = ActionLookupData.create(actionLookupKey, 3);
- SkyKey actionKeyLast = ActionLookupData.create(actionLookupKey, 4);
+ SkyKey actionKey1 = ActionLookupData.create(ACTION_LOOKUP_KEY, 0);
+ SkyKey actionKey2 = ActionLookupData.create(ACTION_LOOKUP_KEY, 1);
+ SkyKey actionKeyEmpty = ActionLookupData.create(ACTION_LOOKUP_KEY, 2);
+ SkyKey actionKeyUnchanging = ActionLookupData.create(ACTION_LOOKUP_KEY, 3);
+ SkyKey actionKeyLast = ActionLookupData.create(ACTION_LOOKUP_KEY, 4);
differencer.inject(
ImmutableMap.of(
actionKey1,
@@ -732,17 +724,6 @@
ArtifactRoot.asDerivedRoot(fs.getPath("/"), outSegment), outputPath.getRelative(relPath));
}
- private SpecialArtifact createTreeArtifact(String relPath) throws IOException {
- String outSegment = "bin";
- Path outputDir = fs.getPath("/" + outSegment);
- Path outputPath = outputDir.getRelative(relPath);
- outputDir.createDirectory();
- ArtifactRoot derivedRoot = ArtifactRoot.asDerivedRoot(fs.getPath("/"), outSegment);
- return ActionsTestUtil.createTreeArtifactWithGeneratingAction(
- derivedRoot,
- derivedRoot.getExecPath().getRelative(derivedRoot.getRoot().relativize(outputPath)));
- }
-
@Test
// TODO(b/154337187): Remove the following annotation to re-enable once this test is de-flaked.
@Ignore
@@ -859,38 +840,6 @@
/*actionDependsOnBuildId=*/ false);
}
- private static ActionExecutionValue actionValueWithTreeArtifacts(
- List<TreeFileArtifact> contents) {
- TreeArtifactValue.MultiBuilder treeArtifacts = TreeArtifactValue.newMultiBuilder();
-
- for (TreeFileArtifact output : contents) {
- Path path = output.getPath();
- try {
- FileArtifactValue noDigest =
- ActionMetadataHandler.fileArtifactValueFromArtifact(
- output,
- FileStatusWithDigestAdapter.adapt(path.statIfFound(Symlinks.NOFOLLOW)),
- null);
- FileArtifactValue withDigest =
- FileArtifactValue.createFromInjectedDigest(
- noDigest, path.getDigest(), !output.isConstantMetadata());
- treeArtifacts.putChild(output, withDigest);
- } catch (IOException e) {
- throw new IllegalStateException(e);
- }
- }
-
- Map<Artifact, TreeArtifactValue> treeArtifactData = new HashMap<>();
- treeArtifacts.injectTo(treeArtifactData::put);
-
- return ActionExecutionValue.create(
- /*artifactData=*/ ImmutableMap.of(),
- treeArtifactData,
- /*outputSymlinks=*/ null,
- /*discoveredModules=*/ null,
- /*actionDependsOnBuildId=*/ false);
- }
-
private static ActionExecutionValue actionValueWithTreeArtifact(
SpecialArtifact output, TreeArtifactValue tree) {
return ActionExecutionValue.create(
@@ -923,9 +872,8 @@
// Test that injected remote artifacts are trusted by the FileSystemValueChecker
// if it is configured to trust remote artifacts, and that local files always take precedence
// over remote files.
- ActionLookupKey actionLookupKey = new SimpleActionLookupKey();
- SkyKey actionKey1 = ActionLookupData.create(actionLookupKey, 0);
- SkyKey actionKey2 = ActionLookupData.create(actionLookupKey, 1);
+ SkyKey actionKey1 = ActionLookupData.create(ACTION_LOOKUP_KEY, 0);
+ SkyKey actionKey2 = ActionLookupData.create(ACTION_LOOKUP_KEY, 1);
Artifact out1 = createDerivedArtifact("foo");
Artifact out2 = createDerivedArtifact("bar");
@@ -975,8 +923,7 @@
public void testRemoteAndLocalTreeArtifacts() throws Exception {
// Test that injected remote tree artifacts are trusted by the FileSystemValueChecker
// and that local files always takes preference over remote files.
- ActionLookupKey actionLookupKey = new SimpleActionLookupKey();
- SkyKey actionKey = ActionLookupData.create(actionLookupKey, 0);
+ SkyKey actionKey = ActionLookupData.create(ACTION_LOOKUP_KEY, 0);
SpecialArtifact treeArtifact = createTreeArtifact("dir");
treeArtifact.getPath().createDirectoryAndParents();
@@ -1053,27 +1000,6 @@
.containsExactlyElementsIn(Arrays.asList(keysWithNewValues));
}
- private static final class MockFileSystem extends InMemoryFileSystem {
- boolean statThrowsRuntimeException;
- boolean readlinkThrowsIoException;
-
- @Override
- public FileStatus statIfFound(Path path, boolean followSymlinks) throws IOException {
- if (statThrowsRuntimeException) {
- throw new RuntimeException("bork");
- }
- return super.statIfFound(path, followSymlinks);
- }
-
- @Override
- protected PathFragment readSymbolicLink(Path path) throws IOException {
- if (readlinkThrowsIoException) {
- throw new IOException("readlink failed");
- }
- return super.readSymbolicLink(path);
- }
- }
-
private static FileStatusWithDigest statWithDigest(final Path path, final FileStatus stat) {
return new FileStatusWithDigest() {
@Nullable
@@ -1128,17 +1054,4 @@
FilesystemValueChecker checker) throws InterruptedException {
return checker.getDirtyKeys(evaluator.getValues(), new BasicFilesystemDirtinessChecker());
}
-
- private static final class SimpleActionLookupKey implements ActionLookupKey {
- @Override
- public SkyFunctionName functionName() {
- return SkyFunctionName.FOR_TESTING;
- }
-
- @Nullable
- @Override
- public Label getLabel() {
- return null;
- }
- }
}
