Skip to content
Snippets Groups Projects
Unverified Commit 72cc6c7a authored by Tiago Quelhas's avatar Tiago Quelhas Committed by GitHub
Browse files

[7.2.0] Correctly handle ParamFileActionInput when writing to the compact execution log. (#22247)

A ParamFileActionInput isn't present in the InputMetadataProvider and
doesn't exist in the filesystem if the respective action is running
remotely. Therefore, the isInputDirectory call as previously written
would always crash on these inputs (which is reported as a warning, not
an error; this is arguably a bad idea and will be fixed in a followup).

Fixes #21820.

PiperOrigin-RevId: 631012628
Change-Id: Ie7eb039d51b2c1465f04a18a432623b3df8e25a4
parent c569d78e
Tags
No related merge requests found
......@@ -121,6 +121,10 @@ public abstract class SpawnLogContext implements ActionContext {
if (input.isSymlink()) {
return false;
}
// Virtual action inputs are always files.
if (input instanceof VirtualActionInput) {
return false;
}
// There are two cases in which an input's declared type may disagree with the filesystem:
// (1) a source artifact pointing to a directory;
// (2) an output artifact declared as a file but materialized as a directory, when allowed by
......
......@@ -30,9 +30,11 @@ import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput;
import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.InputMetadataProvider;
import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnMetrics;
......@@ -467,6 +469,36 @@ public abstract class SpawnLogContextTestBase {
.build());
}
@Test
public void testParamFileInput() throws Exception {
ParamFileActionInput paramFileInput =
new ParamFileActionInput(
PathFragment.create("foo.params"),
ImmutableList.of("a", "b", "c"),
ParameterFileType.UNQUOTED,
UTF_8);
// Do not materialize the file on disk, which would be the case when running remotely.
SpawnBuilder spawn = defaultSpawnBuilder().withInputs(paramFileInput);
SpawnLogContext context = createSpawnLogContext();
context.logSpawn(
spawn.build(),
// ParamFileActionInputs appear in the input map but not in the metadata provider.
createInputMetadataProvider(),
createInputMap(paramFileInput),
fs,
defaultTimeout(),
defaultSpawnResult());
closeAndAssertLog(
context,
defaultSpawnExecBuilder()
.addInputs(File.newBuilder().setPath("foo.params").setDigest(getDigest("a\nb\nc\n")))
.build());
}
@Test
public void testFileOutput(
@TestParameter OutputsMode outputsMode, @TestParameter OutputIndirection indirection)
......@@ -963,13 +995,13 @@ public abstract class SpawnLogContextTestBase {
return new StaticInputMetadataProvider(builder.buildOrThrow());
}
protected static SortedMap<PathFragment, ActionInput> createInputMap(Artifact... artifacts)
protected static SortedMap<PathFragment, ActionInput> createInputMap(ActionInput... actionInputs)
throws Exception {
return createInputMap(EmptyRunfilesSupplier.INSTANCE, artifacts);
return createInputMap(EmptyRunfilesSupplier.INSTANCE, actionInputs);
}
protected static SortedMap<PathFragment, ActionInput> createInputMap(
RunfilesSupplier runfilesSupplier, Artifact... artifacts) throws Exception {
RunfilesSupplier runfilesSupplier, ActionInput... actionInputs) throws Exception {
ImmutableSortedMap.Builder<PathFragment, ActionInput> builder =
ImmutableSortedMap.naturalOrder();
for (Map.Entry<PathFragment, Map<PathFragment, Artifact>> entry :
......@@ -982,8 +1014,8 @@ public abstract class SpawnLogContextTestBase {
builder.put(execPath, artifact != null ? artifact : VirtualActionInput.EMPTY_MARKER);
}
}
for (Artifact artifact : artifacts) {
if (artifact.isTreeArtifact()) {
for (ActionInput actionInput : actionInputs) {
if (actionInput instanceof Artifact artifact && artifact.isTreeArtifact()) {
// Emulate SpawnInputExpander: expand to children, preserve if empty.
TreeArtifactValue treeMetadata = createTreeArtifactValue(artifact);
if (treeMetadata.getChildren().isEmpty()) {
......@@ -994,7 +1026,7 @@ public abstract class SpawnLogContextTestBase {
}
}
} else {
builder.put(artifact.getExecPath(), artifact);
builder.put(actionInput.getExecPath(), actionInput);
}
}
return builder.buildOrThrow();
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment