diff options
7 files changed, 55 insertions, 50 deletions
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 d074c399de..5524e29077 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 @@ package com.google.devtools.build.lib.actions; 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 @@ import java.util.TreeSet; * 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 @@ public final class ActionEnvironment { 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 @@ public final class ActionEnvironment { * 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 final class ActionEnvironment { } 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 da119ab009..750fcfe80e 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.ImmutableList; 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 @@ import java.util.Map; // 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 @@ public class CompositeRunfilesSupplier implements RunfilesSupplier { /** * 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 9cf6d1b5ff..ddab7c995e 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 @@ package com.google.devtools.build.lib.actions; 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 9668eba2cb..3d1c749364 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 @@ package com.google.devtools.build.lib.actions; 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 @@ public class ResourceSet { * 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 @@ public class ResourceSet { /** * 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 c609d3d787..89c6741106 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.common.collect.ImmutableSet; 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 @@ import javax.annotation.Nullable; /** {@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 @@ public class RunfilesSupplierImpl implements RunfilesSupplier { * @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 badf8ef784..23005b7b6c 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.analysis.config.BuildConfiguration; 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.devtools.build.lib.vfs.PathFragment; 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.CheckReturnValue; 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 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie 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 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie * @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 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie 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 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie private CharSequence progressMessage; private String mnemonic = "Unknown"; - protected ExtraActionInfoSupplier<?> extraActionInfoSupplier = null; + protected ExtraActionInfoSupplier extraActionInfoSupplier = null; private boolean disableSandboxing = false; /** @@ -1346,9 +1337,8 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie 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 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie * 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 89ede26f7d..04aa0d77ee 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.ExtendedEventHandler; 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 @@ public final class ActionsTestUtil { 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() { |