Add @AutoCodec to SpawnAction. Refactor the SpawnAction API for extra actions so that it is non-generic and does not persist a GeneratedExtension, which is hard to serialize.
PiperOrigin-RevId: 186627440
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java b/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java
index d074c39..5524e29 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionEnvironment.java
@@ -16,6 +16,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.util.Fingerprint;
import java.util.Map;
import java.util.Set;
@@ -26,15 +27,17 @@
* Environment variables for build or test actions.
*
* <p>The action environment consists of two parts.
+ *
* <ol>
* <li>All the environment variables with a fixed value, stored in a map.
* <li>All the environment variables inherited from the client environment, stored in a set.
* </ol>
*
- * <p>Inherited environment variables must be declared in the Action interface
- * (see {@link Action#getClientEnvironmentVariables}), so that the dependency on the client
- * environment is known to the execution framework for correct incremental builds.
+ * <p>Inherited environment variables must be declared in the Action interface (see {@link
+ * Action#getClientEnvironmentVariables}), so that the dependency on the client environment is known
+ * to the execution framework for correct incremental builds.
*/
+@AutoCodec
public final class ActionEnvironment {
/**
* An empty environment, mainly for testing. Production code should never use this, but instead
@@ -63,15 +66,16 @@
inheritedEnv.add(key);
}
}
- return create(fixedEnv, inheritedEnv);
+ return create(ImmutableMap.copyOf(fixedEnv), ImmutableSet.copyOf(inheritedEnv));
}
private final ImmutableMap<String, String> fixedEnv;
private final ImmutableSet<String> inheritedEnv;
- private ActionEnvironment(Map<String, String> fixedEnv, Set<String> inheritedEnv) {
- this.fixedEnv = ImmutableMap.copyOf(fixedEnv);
- this.inheritedEnv = ImmutableSet.copyOf(inheritedEnv);
+ private ActionEnvironment(
+ ImmutableMap<String, String> fixedEnv, ImmutableSet<String> inheritedEnv) {
+ this.fixedEnv = fixedEnv;
+ this.inheritedEnv = inheritedEnv;
}
/**
@@ -79,7 +83,9 @@
* undefined, so callers need to take care that the key set of the {@code fixedEnv} map and the
* set of {@code inheritedEnv} elements are disjoint.
*/
- public static ActionEnvironment create(Map<String, String> fixedEnv, Set<String> inheritedEnv) {
+ @AutoCodec.Instantiator
+ public static ActionEnvironment create(
+ ImmutableMap<String, String> fixedEnv, ImmutableSet<String> inheritedEnv) {
if (fixedEnv.isEmpty() && inheritedEnv.isEmpty()) {
return EMPTY;
}
@@ -87,7 +93,7 @@
}
public static ActionEnvironment create(Map<String, String> fixedEnv) {
- return new ActionEnvironment(fixedEnv, ImmutableSet.of());
+ return new ActionEnvironment(ImmutableMap.copyOf(fixedEnv), ImmutableSet.of());
}
public ImmutableMap<String, String> getFixedEnv() {
diff --git a/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java
index da119ab..750fcfe 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.util.Map;
@@ -26,6 +27,7 @@
// TODO(michajlo): Move creation to factory constructor and optimize structure of the returned
// supplier, ie: [] -> EmptyRunfilesSupplier, [singleSupplier] -> supplier,
// [Empty, single] -> single, and so on.
+@AutoCodec
public class CompositeRunfilesSupplier implements RunfilesSupplier {
private final ImmutableList<RunfilesSupplier> suppliers;
@@ -38,6 +40,7 @@
/**
* Create an instance combining all of {@code suppliers}, with earlier elements taking precedence.
*/
+ @AutoCodec.Instantiator
public CompositeRunfilesSupplier(Iterable<RunfilesSupplier> suppliers) {
this.suppliers = ImmutableList.copyOf(suppliers);
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java
index 9cf6d1b..ddab7c9 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java
@@ -17,10 +17,13 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.Strategy;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Map;
/** Empty implementation of RunfilesSupplier */
+@AutoCodec(strategy = Strategy.SINGLETON)
public class EmptyRunfilesSupplier implements RunfilesSupplier {
public static final EmptyRunfilesSupplier INSTANCE = new EmptyRunfilesSupplier();
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ResourceSet.java b/src/main/java/com/google/devtools/build/lib/actions/ResourceSet.java
index 9668eba..3d1c749 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ResourceSet.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ResourceSet.java
@@ -16,19 +16,20 @@
import com.google.common.base.Splitter;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.common.options.Converter;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.Iterator;
import java.util.NoSuchElementException;
/**
- * Instances of this class represent an estimate of the resource consumption
- * for a particular Action, or the total available resources. We plan to
- * use this to do smarter scheduling of actions, for example making sure
- * that we don't schedule jobs concurrently if they would use so much
- * memory as to cause the machine to thrash.
+ * Instances of this class represent an estimate of the resource consumption for a particular
+ * Action, or the total available resources. We plan to use this to do smarter scheduling of
+ * actions, for example making sure that we don't schedule jobs concurrently if they would use so
+ * much memory as to cause the machine to thrash.
*/
@Immutable
+@AutoCodec
public class ResourceSet {
/** For actions that consume negligible resources. */
@@ -48,7 +49,7 @@
* workstation.
*/
private final double ioUsage;
-
+
private ResourceSet(double memoryMb, double cpuUsage, double ioUsage, int localTestCount) {
this.memoryMb = memoryMb;
this.cpuUsage = cpuUsage;
@@ -79,12 +80,13 @@
/**
* Returns a new ResourceSet with the provided values for memoryMb, cpuUsage, ioUsage, and
- * localTestCount. Most action resource definitions should use
- * {@link #createWithRamCpuIo(double, double, double)} or {@link #createWithLocalTestCount(int)}.
- * Use this method primarily when constructing ResourceSets that represent available resources.
+ * localTestCount. Most action resource definitions should use {@link #createWithRamCpuIo(double,
+ * double, double)} or {@link #createWithLocalTestCount(int)}. Use this method primarily when
+ * constructing ResourceSets that represent available resources.
*/
- public static ResourceSet create(double memoryMb, double cpuUsage, double ioUsage,
- int localTestCount) {
+ @AutoCodec.Instantiator
+ public static ResourceSet create(
+ double memoryMb, double cpuUsage, double ioUsage, int localTestCount) {
if (memoryMb == 0 && cpuUsage == 0 && ioUsage == 0 && localTestCount == 0) {
return ZERO;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImpl.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImpl.java
index c609d3d..89c6741 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImpl.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImpl.java
@@ -21,6 +21,7 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.BaseSpawn;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.util.Map;
@@ -28,6 +29,7 @@
/** {@link RunfilesSupplier} implementation wrapping a single {@link Runfiles} directory mapping. */
// TODO(bazel-team): Consider renaming to SingleRunfilesSupplierImpl.
+@AutoCodec
public class RunfilesSupplierImpl implements RunfilesSupplier {
private final PathFragment runfilesDir;
private final Runfiles runfiles;
@@ -60,10 +62,9 @@
* @param runfiles the runfiles for runilesDir.
* @param manifest runfiles' associated runfiles manifest artifact, if present.
*/
+ @AutoCodec.Instantiator
public RunfilesSupplierImpl(
- PathFragment runfilesDir,
- Runfiles runfiles,
- @Nullable Artifact manifest) {
+ PathFragment runfilesDir, Runfiles runfiles, @Nullable Artifact manifest) {
Preconditions.checkArgument(!runfilesDir.isAbsolute());
this.runfilesDir = Preconditions.checkNotNull(runfilesDir);
this.runfiles = Preconditions.checkNotNull(runfiles);
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
index badf8ef..23005b7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
@@ -60,6 +60,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.NestedSetView;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.LazyString;
@@ -68,7 +69,6 @@
import com.google.errorprone.annotations.CompileTimeConstant;
import com.google.errorprone.annotations.FormatMethod;
import com.google.errorprone.annotations.FormatString;
-import com.google.protobuf.GeneratedMessage.GeneratedExtension;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
@@ -79,22 +79,12 @@
import javax.annotation.Nullable;
/** An Action representing an arbitrary subprocess to be forked and exec'd. */
+@AutoCodec
public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifier, CommandAction {
-
- /** Sets extensions on ExtraActionInfo **/
- protected static class ExtraActionInfoSupplier<T> {
- private final GeneratedExtension<ExtraActionInfo, T> extension;
- private final T value;
-
- protected ExtraActionInfoSupplier(GeneratedExtension<ExtraActionInfo, T> extension, T value) {
- this.extension = extension;
- this.value = value;
- }
-
- void extend(ExtraActionInfo.Builder builder) {
- builder.setExtension(extension, value);
- }
+ /** Sets extensions on {@link ExtraActionInfo}. */
+ public interface ExtraActionInfoSupplier {
+ void extend(ExtraActionInfo.Builder builder);
}
private static final String GUID = "ebd6fce3-093e-45ee-adb6-bf513b602f0d";
@@ -109,7 +99,7 @@
private final ResourceSet resourceSet;
private final ImmutableMap<String, String> executionInfo;
- private final ExtraActionInfoSupplier<?> extraActionInfoSupplier;
+ private final ExtraActionInfoSupplier extraActionInfoSupplier;
/**
* Constructs a SpawnAction using direct initialization arguments.
@@ -131,6 +121,7 @@
* @param progressMessage the message printed during the progression of the build.
* @param mnemonic the mnemonic that is reported in the master log.
*/
+ @AutoCodec.Instantiator
public SpawnAction(
ActionOwner owner,
Iterable<Artifact> tools,
@@ -197,7 +188,7 @@
RunfilesSupplier runfilesSupplier,
String mnemonic,
boolean executeUnconditionally,
- ExtraActionInfoSupplier<?> extraActionInfoSupplier) {
+ ExtraActionInfoSupplier extraActionInfoSupplier) {
super(owner, tools, inputs, runfilesSupplier, outputs, env);
this.resourceSet = resourceSet;
this.executionInfo = executionInfo;
@@ -611,7 +602,7 @@
private CharSequence progressMessage;
private String mnemonic = "Unknown";
- protected ExtraActionInfoSupplier<?> extraActionInfoSupplier = null;
+ protected ExtraActionInfoSupplier extraActionInfoSupplier = null;
private boolean disableSandboxing = false;
/**
@@ -1346,9 +1337,8 @@
return this;
}
- public <T> Builder setExtraActionInfo(
- GeneratedExtension<ExtraActionInfo, T> extension, T value) {
- this.extraActionInfoSupplier = new ExtraActionInfoSupplier<>(extension, value);
+ public <T> Builder setExtraActionInfo(ExtraActionInfoSupplier extraActionInfoSupplier) {
+ this.extraActionInfoSupplier = extraActionInfoSupplier;
return this;
}
@@ -1362,7 +1352,8 @@
* Command line implementation that optimises for containing executable args, command lines, and
* command lines spilled to param files.
*/
- private static class SpawnActionCommandLine extends CommandLine {
+ @AutoCodec
+ static class SpawnActionCommandLine extends CommandLine {
private final Object[] values;
SpawnActionCommandLine(Object[] values) {
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
index 89ede26..04aa0d7 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
@@ -64,8 +64,8 @@
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.exec.SingleBuildFileCache;
import com.google.devtools.build.lib.packages.AspectDescriptor;
-import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
-import com.google.devtools.build.lib.skyframe.serialization.SingletonCodec;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.Strategy;
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.util.ResourceUsage;
import com.google.devtools.build.lib.util.io.FileOutErr;
@@ -267,11 +267,10 @@
null,
null);
+ @AutoCodec(strategy = Strategy.SINGLETON)
static class NullArtifactOwner implements ArtifactOwner {
- private static final ActionsTestUtil.NullArtifactOwner INSTANCE =
+ public static final ActionsTestUtil.NullArtifactOwner INSTANCE =
new ActionsTestUtil.NullArtifactOwner();
- static final ObjectCodec<ActionsTestUtil.NullArtifactOwner> CODEC =
- SingletonCodec.of(INSTANCE, "null_artifact_owner");
@Override
public Label getLabel() {
