diff options
Diffstat (limited to 'src/main/java')
23 files changed, 227 insertions, 295 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java index dd2d5de509..6208db67ce 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.actions; -import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -37,21 +36,15 @@ public class BaseSpawn implements Spawn { private final ImmutableList<String> arguments; private final ImmutableMap<String, String> environment; private final ImmutableMap<String, String> executionInfo; - private final ImmutableMap<PathFragment, Artifact> runfilesManifests; private final ImmutableSet<PathFragment> optionalOutputFiles; private final RunfilesSupplier runfilesSupplier; private final ActionExecutionMetadata action; private final ResourceSet localResources; - // TODO(bazel-team): When we migrate ActionSpawn to use this constructor decide on and enforce - // policy on runfilesManifests and runfilesSupplier being non-empty (ie: are overlapping mappings - // allowed?). - @VisibleForTesting - BaseSpawn( + public BaseSpawn( List<String> arguments, Map<String, String> environment, Map<String, String> executionInfo, - Map<PathFragment, Artifact> runfilesManifests, RunfilesSupplier runfilesSupplier, ActionExecutionMetadata action, ResourceSet localResources, @@ -59,7 +52,6 @@ public class BaseSpawn implements Spawn { this.arguments = ImmutableList.copyOf(arguments); this.environment = ImmutableMap.copyOf(environment); this.executionInfo = ImmutableMap.copyOf(executionInfo); - this.runfilesManifests = ImmutableMap.copyOf(runfilesManifests); this.runfilesSupplier = runfilesSupplier; this.action = action; this.localResources = localResources; @@ -81,35 +73,12 @@ public class BaseSpawn implements Spawn { arguments, environment, executionInfo, - ImmutableMap.<PathFragment, Artifact>of(), runfilesSupplier, action, localResources, ImmutableSet.<PathFragment>of()); } - /** - * Returns a new Spawn. The caller must not modify the parameters after the call; neither will - * this method. - */ - public BaseSpawn( - List<String> arguments, - Map<String, String> environment, - Map<String, String> executionInfo, - Map<PathFragment, Artifact> runfilesManifests, - ActionExecutionMetadata action, - ResourceSet localResources) { - this( - arguments, - environment, - executionInfo, - runfilesManifests, - EmptyRunfilesSupplier.INSTANCE, - action, - localResources, - ImmutableSet.<PathFragment>of()); - } - /** Returns a new Spawn. */ public BaseSpawn( List<String> arguments, @@ -121,30 +90,11 @@ public class BaseSpawn implements Spawn { arguments, environment, executionInfo, - ImmutableMap.<PathFragment, Artifact>of(), + EmptyRunfilesSupplier.INSTANCE, action, localResources); } - public BaseSpawn( - List<String> arguments, - Map<String, String> environment, - Map<String, String> executionInfo, - RunfilesSupplier runfilesSupplier, - ActionExecutionMetadata action, - ResourceSet localResources, - Collection<PathFragment> optionalOutputFiles) { - this( - arguments, - environment, - executionInfo, - ImmutableMap.<PathFragment, Artifact>of(), - runfilesSupplier, - action, - localResources, - optionalOutputFiles); - } - public static PathFragment runfilesForFragment(PathFragment pathFragment) { return pathFragment.getParentDirectory().getChild(pathFragment.getBaseName() + ".runfiles"); } @@ -170,11 +120,6 @@ public class BaseSpawn implements Spawn { } @Override - public ImmutableMap<PathFragment, Artifact> getRunfilesManifests() { - return runfilesManifests; - } - - @Override public RunfilesSupplier getRunfilesSupplier() { return runfilesSupplier; } @@ -234,10 +179,8 @@ public class BaseSpawn implements Spawn { /** @return the runfiles directory if there is only one, otherwise null */ private PathFragment getRunfilesRoot() { Set<PathFragment> runfilesSupplierRoots = runfilesSupplier.getRunfilesDirs(); - if (runfilesSupplierRoots.size() == 1 && runfilesManifests.isEmpty()) { + if (runfilesSupplierRoots.size() == 1) { return Iterables.getOnlyElement(runfilesSupplierRoots); - } else if (runfilesManifests.size() == 1 && runfilesSupplierRoots.isEmpty()) { - return Iterables.getOnlyElement(runfilesManifests.keySet()); } else { return null; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CompositeRunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java index ae2c465bd8..da119ab009 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/CompositeRunfilesSupplier.java +++ b/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java @@ -12,52 +12,72 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.devtools.build.lib.analysis; +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.common.collect.Maps; -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.RunfilesSupplier; -import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; - import java.io.IOException; import java.util.Map; -/** RunfilesSupplier implementation composing instances. */ +/** A {@link RunfilesSupplier} implementation for composing multiple instances. */ +// TODO(michajlo): Move creation to factory constructor and optimize structure of the returned +// supplier, ie: [] -> EmptyRunfilesSupplier, [singleSupplier] -> supplier, +// [Empty, single] -> single, and so on. public class CompositeRunfilesSupplier implements RunfilesSupplier { - private final RunfilesSupplier first; - private final RunfilesSupplier second; + private final ImmutableList<RunfilesSupplier> suppliers; /** Create an instance with {@code first} taking precedence over {@code second}. */ public CompositeRunfilesSupplier(RunfilesSupplier first, RunfilesSupplier second) { - this.first = Preconditions.checkNotNull(first); - this.second = Preconditions.checkNotNull(second); + this(ImmutableList.of(first, second)); + } + + /** + * Create an instance combining all of {@code suppliers}, with earlier elements taking precedence. + */ + public CompositeRunfilesSupplier(Iterable<RunfilesSupplier> suppliers) { + this.suppliers = ImmutableList.copyOf(suppliers); } @Override public Iterable<Artifact> getArtifacts() { ImmutableSet.Builder<Artifact> result = ImmutableSet.builder(); - result.addAll(first.getArtifacts()); - result.addAll(second.getArtifacts()); + for (RunfilesSupplier supplier : suppliers) { + result.addAll(supplier.getArtifacts()); + } return result.build(); } @Override public ImmutableSet<PathFragment> getRunfilesDirs() { ImmutableSet.Builder<PathFragment> result = ImmutableSet.builder(); - result.addAll(first.getRunfilesDirs()); - result.addAll(second.getRunfilesDirs()); + for (RunfilesSupplier supplier : suppliers) { + result.addAll(supplier.getRunfilesDirs()); + } return result.build(); } @Override public ImmutableMap<PathFragment, Map<PathFragment, Artifact>> getMappings() throws IOException { Map<PathFragment, Map<PathFragment, Artifact>> result = Maps.newHashMap(); - result.putAll(second.getMappings()); - result.putAll(first.getMappings()); + for (RunfilesSupplier supplier : suppliers) { + Map<PathFragment, Map<PathFragment, Artifact>> mappings = supplier.getMappings(); + for (Map.Entry<PathFragment, Map<PathFragment, Artifact>> entry : mappings.entrySet()) { + result.putIfAbsent(entry.getKey(), entry.getValue()); + } + } return ImmutableMap.copyOf(result); } + + @Override + public ImmutableList<Artifact> getManifests() { + ImmutableList.Builder<Artifact> result = ImmutableList.builder(); + for (RunfilesSupplier supplier : suppliers) { + result.addAll(supplier.getManifests()); + } + return result.build(); + } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java index 45a1496259..821407bd86 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java @@ -19,7 +19,6 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.extra.SpawnInfo; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; - import java.util.Collection; /** @@ -60,11 +59,6 @@ public class DelegateSpawn implements Spawn { } @Override - public ImmutableMap<PathFragment, Artifact> getRunfilesManifests() { - return spawn.getRunfilesManifests(); - } - - @Override public RunfilesSupplier getRunfilesSupplier() { return spawn.getRunfilesSupplier(); } 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 03da101c7b..9cf6d1b5ff 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 @@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.vfs.PathFragment; - import java.util.Map; /** Empty implementation of RunfilesSupplier */ @@ -43,4 +42,8 @@ public class EmptyRunfilesSupplier implements RunfilesSupplier { return ImmutableMap.of(); } + @Override + public ImmutableList<Artifact> getManifests() { + return ImmutableList.<Artifact>of(); + } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/RunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/actions/RunfilesSupplier.java index 33938335a4..9b05cb16d5 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/RunfilesSupplier.java +++ b/src/main/java/com/google/devtools/build/lib/actions/RunfilesSupplier.java @@ -14,14 +14,16 @@ 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.vfs.PathFragment; - import java.io.IOException; import java.util.Map; /** Convenience wrapper around runfiles allowing lazy expansion. */ +// TODO(bazel-team): Ideally we could refer to Runfiles objects directly here, but current package +// structure makes this difficult. Consider moving things around to make this possible. public interface RunfilesSupplier { /** @return the contained artifacts */ @@ -37,4 +39,7 @@ public interface RunfilesSupplier { * @throws IOException */ ImmutableMap<PathFragment, Map<PathFragment, Artifact>> getMappings() throws IOException; + + /** @return the runfiles manifest artifacts, if any. */ + ImmutableList<Artifact> getManifests(); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java index a069db6209..ae2358f991 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java @@ -19,7 +19,6 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.extra.SpawnInfo; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; - import java.util.Collection; /** @@ -58,11 +57,6 @@ public interface Spawn { String asShellCommand(Path workingDir); /** - * Returns the runfiles data for remote execution. Format is (directory, manifest file). - */ - ImmutableMap<PathFragment, Artifact> getRunfilesManifests(); - - /** * Returns the {@link RunfilesSupplier} helper encapsulating the runfiles for this spawn. */ RunfilesSupplier getRunfilesSupplier(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java index c24f659b31..c711787225 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java @@ -21,7 +21,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.collect.Sets; 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.analysis.actions.FileWriteAction; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -65,11 +65,8 @@ public final class CommandHelper { @VisibleForTesting public static int maxCommandLength = OS.getCurrent() == OS.WINDOWS ? 8000 : 64000; - /** - * A map of remote path prefixes and corresponding runfiles manifests for tools - * used by this rule. - */ - private final SkylarkDict<PathFragment, Artifact> remoteRunfileManifestMap; + /** {@link RunfilesSupplier}s for tools used by this rule. */ + private final SkylarkList<RunfilesSupplier> toolsRunfilesSuppliers; /** * Use labelMap for heuristically expanding labels (does not include "outs") @@ -103,8 +100,7 @@ public final class CommandHelper { this.ruleContext = ruleContext; ImmutableList.Builder<Artifact> resolvedToolsBuilder = ImmutableList.builder(); - ImmutableMap.Builder<PathFragment, Artifact> remoteRunfileManifestBuilder = - ImmutableMap.builder(); + ImmutableList.Builder<RunfilesSupplier> toolsRunfilesBuilder = ImmutableList.builder(); Map<Label, Collection<Artifact>> tempLabelMap = new HashMap<>(); for (Map.Entry<Label, ? extends Iterable<Artifact>> entry : labelMap.entrySet()) { @@ -125,11 +121,7 @@ public final class CommandHelper { if (executableArtifact != null) { mapGet(tempLabelMap, label).add(executableArtifact); // Also send the runfiles when running remotely. - Artifact runfilesManifest = tool.getRunfilesManifest(); - if (runfilesManifest != null) { - remoteRunfileManifestBuilder.put( - BaseSpawn.runfilesForFragment(executableArtifact.getExecPath()), runfilesManifest); - } + toolsRunfilesBuilder.add(tool.getRunfilesSupplier()); } else { // Map all depArtifacts to the respective label using the multimaps. mapGet(tempLabelMap, label).addAll(files); @@ -137,7 +129,7 @@ public final class CommandHelper { } this.resolvedTools = SkylarkList.createImmutable(resolvedToolsBuilder.build()); - this.remoteRunfileManifestMap = SkylarkDict.copyOf(null, remoteRunfileManifestBuilder.build()); + this.toolsRunfilesSuppliers = SkylarkList.createImmutable(toolsRunfilesBuilder.build()); ImmutableMap.Builder<Label, ImmutableCollection<Artifact>> labelMapBuilder = ImmutableMap.builder(); for (Entry<Label, Collection<Artifact>> entry : tempLabelMap.entrySet()) { @@ -150,8 +142,8 @@ public final class CommandHelper { return resolvedTools; } - public SkylarkDict<PathFragment, Artifact> getRemoteRunfileManifestMap() { - return remoteRunfileManifestMap; + public SkylarkList<RunfilesSupplier> getToolsRunfilesSuppliers() { + return toolsRunfilesSuppliers; } // Returns the value in the specified corresponding to 'key', creating and 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 474784c5a7..79ecce1b46 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 @@ -14,7 +14,8 @@ package com.google.devtools.build.lib.analysis; -import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -22,17 +23,17 @@ 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.vfs.PathFragment; - import java.io.IOException; import java.util.Map; -import java.util.Map.Entry; +import javax.annotation.Nullable; -/** - * {@link RunfilesSupplier} implementation wrapping directory to {@link Runfiles} objects mappings. - */ +/** {@link RunfilesSupplier} implementation wrapping a single {@link Runfiles} directory mapping. */ +// TODO(bazel-team): Consider renaming to SingleRunfilesSupplierImpl. public class RunfilesSupplierImpl implements RunfilesSupplier { - - private final ImmutableMap<PathFragment, Runfiles> inputRunfiles; + private final PathFragment runfilesDir; + private final Runfiles runfiles; + @Nullable + private final Artifact manifest; /** * Create an instance for an executable. @@ -46,43 +47,48 @@ public class RunfilesSupplierImpl implements RunfilesSupplier { } /** - * Create an instance when you have a a single mapping. - * - * @param runfilesDir the desired runfiles directory. Should be relative. - * @param runfiles the runfiles for runfilesDir + * Create an instance. When a manifest is available consider using + * {@link #RunfilesSupplierImpl(PathFragment, Runfiles, Artifact)} instead. */ public RunfilesSupplierImpl(PathFragment runfilesDir, Runfiles runfiles) { - this.inputRunfiles = ImmutableMap.of(runfilesDir, runfiles); + this(runfilesDir, runfiles, /*manifest=*/ null); } - @VisibleForTesting - public RunfilesSupplierImpl(Map<PathFragment, Runfiles> inputRunfiles) { - this.inputRunfiles = ImmutableMap.copyOf(inputRunfiles); + /** + * Create an instance mapping {@code runfiles} to {@code runfilesDir}. + * + * @param runfilesDir the desired runfiles directory. Should be relative. + * @param runfiles the runfiles for runilesDir. + * @param manifest runfiles' associated runfiles manifest artifact, if present. + */ + public RunfilesSupplierImpl( + PathFragment runfilesDir, + Runfiles runfiles, + @Nullable Artifact manifest) { + this.runfilesDir = Preconditions.checkNotNull(runfilesDir); + this.runfiles = Preconditions.checkNotNull(runfiles); + this.manifest = manifest; } @Override public Iterable<Artifact> getArtifacts() { - ImmutableSet.Builder<Artifact> builder = ImmutableSet.builder(); - for (Entry<PathFragment, Runfiles> entry : inputRunfiles.entrySet()) { - builder.addAll( - Iterables.filter(entry.getValue().getAllArtifacts(), Artifact.MIDDLEMAN_FILTER)); - } - return builder.build(); + return Iterables.filter(runfiles.getAllArtifacts(), Artifact.MIDDLEMAN_FILTER); } @Override public ImmutableSet<PathFragment> getRunfilesDirs() { - return inputRunfiles.keySet(); + return ImmutableSet.of(runfilesDir); } @Override public ImmutableMap<PathFragment, Map<PathFragment, Artifact>> getMappings() throws IOException { - ImmutableMap.Builder<PathFragment, Map<PathFragment, Artifact>> result = - ImmutableMap.builder(); - for (Entry<PathFragment, Runfiles> entry : inputRunfiles.entrySet()) { - result.put(entry.getKey(), entry.getValue().getRunfilesInputs(null, null)); - } - return result.build(); + return ImmutableMap.of( + runfilesDir, + runfiles.getRunfilesInputs(/*eventHandler=*/ null, /*location=*/ null)); } + @Override + public ImmutableList<Artifact> getManifests() { + return manifest != null ? ImmutableList.of(manifest) : ImmutableList.<Artifact>of(); + } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java index ff4231f989..7004e29b92 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java @@ -107,6 +107,11 @@ public final class SourceManifestAction extends AbstractFileWriteAction { this.runfiles = runfiles; } + /** The {@link Runfiles} for which this action creates the symlink tree. */ + public Runfiles getGeneratedRunfiles() { + return runfiles; + } + @VisibleForTesting public void writeOutputFile(OutputStream out, EventHandler eventHandler) throws IOException { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactAction.java index 4339a02dc8..ba7da944b2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactAction.java @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.actions.BaseSpawn; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.ResourceSet; +import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.analysis.FilesToRunProvider; @@ -40,9 +41,9 @@ import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; - import java.io.IOException; import java.util.Collection; +import java.util.List; import java.util.Map; /** @@ -123,13 +124,13 @@ public final class PopulateTreeArtifactAction extends AbstractAction { Artifact treeArtifact, Iterable<PathFragment> entriesToExtract, Iterable<String> commandLine, - Map<PathFragment, Artifact> runfilesManifests, + RunfilesSupplier runfilesSupplier, ActionExecutionMetadata action) { super( ImmutableList.copyOf(commandLine), ImmutableMap.<String, String>of(), ImmutableMap.<String, String>of(), - ImmutableMap.copyOf(runfilesManifests), + runfilesSupplier, action, AbstractAction.DEFAULT_RESOURCE_SET); this.treeArtifact = treeArtifact; @@ -212,13 +213,12 @@ public final class PopulateTreeArtifactAction extends AbstractAction { f.addString(GUID); f.addString(getMnemonic()); f.addStrings(spawnCommandLine()); - Map<PathFragment, Artifact> zipperManifest = zipperExecutableRunfilesManifest(); - f.addInt(zipperManifest.size()); - for (Map.Entry<PathFragment, Artifact> input : zipperManifest.entrySet()) { - f.addString(input.getKey().getPathString() + "/"); - f.addPath(input.getValue().getExecPath()); + f.addPaths(zipper.getRunfilesSupplier().getRunfilesDirs()); + List<Artifact> runfilesManifests = zipper.getRunfilesSupplier().getManifests(); + f.addInt(runfilesManifests.size()); + for (Artifact manifest : runfilesManifests) { + f.addPath(manifest.getExecPath()); } - return f.hexDigestAndReset(); } @@ -255,7 +255,7 @@ public final class PopulateTreeArtifactAction extends AbstractAction { outputTreeArtifact, entries, spawnCommandLine(), - zipperExecutableRunfilesManifest(), + zipper.getRunfilesSupplier(), this); } @@ -269,16 +269,6 @@ public final class PopulateTreeArtifactAction extends AbstractAction { "@" + archiveManifest.getExecPathString()); } - private Map<PathFragment, Artifact> zipperExecutableRunfilesManifest() { - if (zipper.getRunfilesManifest() != null) { - return ImmutableMap.of( - BaseSpawn.runfilesForFragment(zipper.getExecutable().getExecPath()), - zipper.getRunfilesManifest()); - } else { - return ImmutableMap.<PathFragment, Artifact>of(); - } - } - private Iterable<PathFragment> readAndCheckManifestEntries() throws IOException, IllegalManifestFileException { ImmutableList.Builder<PathFragment> manifestEntries = ImmutableList.builder(); 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 b7a094a469..cf0f4ebaaf 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 @@ -32,11 +32,14 @@ import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.BaseSpawn; import com.google.devtools.build.lib.actions.CommandAction; +import com.google.devtools.build.lib.actions.CompositeRunfilesSupplier; +import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionInfoSpecifier; import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; import com.google.devtools.build.lib.actions.ResourceSet; +import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.actions.extra.ExtraActionInfo; @@ -90,8 +93,6 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie private final boolean executeUnconditionally; private final String progressMessage; private final String mnemonic; - // entries are (directory for remote execution, Artifact) - private final ImmutableMap<PathFragment, Artifact> inputManifests; private final ResourceSet resourceSet; private final ImmutableMap<String, String> environment; @@ -142,7 +143,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie ImmutableSet.copyOf(clientEnvironmentVariables), ImmutableMap.<String, String>of(), progressMessage, - ImmutableMap.<PathFragment, Artifact>of(), + EmptyRunfilesSupplier.INSTANCE, mnemonic, false, null); @@ -169,8 +170,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie * dependency checking; typically it may include the names of input and output files, but this * is not necessary. * @param progressMessage the message printed during the progression of the build - * @param inputManifests entries in inputs that are symlink manifest files. These are passed to - * remote execution in the environment rather than as inputs. + * @param runfilesSupplier {@link RunfilesSupplier}s describing the runfiles for the action * @param mnemonic the mnemonic that is reported in the master log. */ public SpawnAction( @@ -184,18 +184,17 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie ImmutableSet<String> clientEnvironmentVariables, ImmutableMap<String, String> executionInfo, String progressMessage, - ImmutableMap<PathFragment, Artifact> inputManifests, + RunfilesSupplier runfilesSupplier, String mnemonic, boolean executeUnconditionally, ExtraActionInfoSupplier<?> extraActionInfoSupplier) { - super(owner, tools, inputs, outputs); + super(owner, tools, inputs, runfilesSupplier, outputs); this.resourceSet = resourceSet; this.executionInfo = executionInfo; this.environment = environment; this.clientEnvironmentVariables = clientEnvironmentVariables; this.argv = argv; this.progressMessage = progressMessage; - this.inputManifests = inputManifests; this.mnemonic = mnemonic; this.executeUnconditionally = executeUnconditionally; this.extraActionInfoSupplier = extraActionInfoSupplier; @@ -288,10 +287,6 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie } } - public ImmutableMap<PathFragment, Artifact> getInputManifests() { - return inputManifests; - } - /** * Returns s, truncated to no more than maxLen characters, appending an * ellipsis if truncation occurred. @@ -308,7 +303,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie * * This method is final, as it is merely a shorthand use of the generic way to obtain a spawn, * which also depends on the client environment. Subclasses that which to override the way to get - * a spawn should override {@link getSpawn(Map<String, String>)} instead. + * a spawn should override {@link #getSpawn(Map)} instead. */ public final Spawn getSpawn() { return getSpawn(null); @@ -331,10 +326,11 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie // We don't need the toolManifests here, because they are a subset of the inputManifests by // definition and the output of an action shouldn't change whether something is considered a // tool or not. - f.addInt(inputManifests.size()); - for (Map.Entry<PathFragment, Artifact> input : inputManifests.entrySet()) { - f.addString(input.getKey().getPathString() + "/"); - f.addPath(input.getValue().getExecPath()); + f.addPaths(getRunfilesSupplier().getRunfilesDirs()); + ImmutableList<Artifact> runfilesManifests = getRunfilesSupplier().getManifests(); + f.addInt(runfilesManifests.size()); + for (Artifact runfilesManifest : runfilesManifests) { + f.addPath(runfilesManifest.getExecPath()); } f.addStringMap(getEnvironment()); f.addStrings(getClientEnvironmentVariables()); @@ -445,7 +441,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie super(ImmutableList.copyOf(argv.arguments()), ImmutableMap.<String, String>of(), executionInfo, - inputManifests, + SpawnAction.this.getRunfilesSupplier(), SpawnAction.this, resourceSet); for (Artifact input : getInputs()) { @@ -483,7 +479,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie // included as manifests in getEnvironment(). List<Artifact> inputs = Lists.newArrayList(getInputs()); inputs.removeAll(filesets); - inputs.removeAll(inputManifests.values()); + inputs.removeAll(this.getRunfilesSupplier().getManifests()); return inputs; } } @@ -496,8 +492,8 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie private final NestedSetBuilder<Artifact> toolsBuilder = NestedSetBuilder.stableOrder(); private final NestedSetBuilder<Artifact> inputsBuilder = NestedSetBuilder.stableOrder(); private final List<Artifact> outputs = new ArrayList<>(); - private final Map<PathFragment, Artifact> toolManifests = new LinkedHashMap<>(); - private final Map<PathFragment, Artifact> inputManifests = new LinkedHashMap<>(); + private final List<RunfilesSupplier> inputRunfilesSuppliers = new ArrayList<>(); + private final List<RunfilesSupplier> toolRunfilesSuppliers = new ArrayList<>(); private ResourceSet resourceSet = AbstractAction.DEFAULT_RESOURCE_SET; private ImmutableMap<String, String> environment = ImmutableMap.of(); private ImmutableSet<String> clientEnvironmentVariables = ImmutableSet.of(); @@ -529,8 +525,8 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie this.toolsBuilder.addTransitive(other.toolsBuilder.build()); this.inputsBuilder.addTransitive(other.inputsBuilder.build()); this.outputs.addAll(other.outputs); - this.toolManifests.putAll(other.toolManifests); - this.inputManifests.putAll(other.inputManifests); + this.inputRunfilesSuppliers.addAll(other.inputRunfilesSuppliers); + this.toolRunfilesSuppliers.addAll(other.toolRunfilesSuppliers); this.resourceSet = other.resourceSet; this.environment = other.environment; this.clientEnvironmentVariables = other.clientEnvironmentVariables; @@ -657,10 +653,6 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie .addTransitive(tools) .build(); - LinkedHashMap<PathFragment, Artifact> inputAndToolManifests = - new LinkedHashMap<>(inputManifests); - inputAndToolManifests.putAll(toolManifests); - Map<String, String> env; Set<String> clientEnv; if (useDefaultShellEnvironment) { @@ -689,7 +681,8 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie ImmutableSet.copyOf(clientEnv), ImmutableMap.copyOf(executionInfo), progressMessage, - ImmutableMap.copyOf(inputAndToolManifests), + new CompositeRunfilesSupplier( + Iterables.concat(this.inputRunfilesSuppliers, this.toolRunfilesSuppliers)), mnemonic); } @@ -705,7 +698,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie ImmutableSet<String> clientEnvironmentVariables, ImmutableMap<String, String> executionInfo, String progressMessage, - ImmutableMap<PathFragment, Artifact> inputAndToolManifests, + RunfilesSupplier runfilesSupplier, String mnemonic) { return new SpawnAction( owner, @@ -718,7 +711,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie clientEnvironmentVariables, executionInfo, progressMessage, - inputAndToolManifests, + runfilesSupplier, mnemonic, executeUnconditionally, extraActionInfoSupplier); @@ -790,13 +783,8 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie return this; } - private Builder addToolManifest(Artifact artifact, PathFragment remote) { - toolManifests.put(remote, artifact); - return this; - } - - public Builder addInputManifest(Artifact artifact, PathFragment remote) { - inputManifests.put(remote, artifact); + public Builder addRunfilesSupplier(RunfilesSupplier supplier) { + inputRunfilesSuppliers.add(supplier); return this; } @@ -1007,11 +995,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie */ public Builder addTool(FilesToRunProvider tool) { addTools(tool.getFilesToRun()); - if (tool.getRunfilesManifest() != null) { - addToolManifest( - tool.getRunfilesManifest(), - BaseSpawn.runfilesForFragment(tool.getExecutable().getExecPath())); - } + toolRunfilesSuppliers.add(tool.getRunfilesSupplier()); return this; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java index c58334d450..6792e4e3f7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.actions.extra.SpawnInfo; import com.google.devtools.build.lib.analysis.AbstractConfiguredTarget; import com.google.devtools.build.lib.analysis.CommandHelper; @@ -196,14 +197,16 @@ public class SkylarkRuleImplementationFunctions { + "for useful keys." ), @Param( + // TODO(bazel-team): The name here isn't accurate anymore. This is technically experimental, + // so folks shouldn't be too attached, but consider renaming to be more accurate/opaque. name = "input_manifests", - type = SkylarkDict.class, + type = SkylarkList.class, noneable = true, defaultValue = "None", named = true, positional = false, doc = - "sets the map of input manifests files; " + "(Experimental) sets the input runfiles metadata; " + "they are typically generated by resolve_command." ) }, @@ -317,14 +320,9 @@ public class SkylarkRuleImplementationFunctions { "execution_requirements"))); } if (inputManifestsUnchecked != Runtime.NONE) { - for (Map.Entry<PathFragment, Artifact> entry : - SkylarkDict.castSkylarkDictOrNoneToDict( - inputManifestsUnchecked, - PathFragment.class, - Artifact.class, - "input manifest file map") - .entrySet()) { - builder.addInputManifest(entry.getValue(), entry.getKey()); + for (RunfilesSupplier supplier : SkylarkList.castSkylarkListOrNoneToList( + inputManifestsUnchecked, RunfilesSupplier.class, "runfiles suppliers")) { + builder.addRunfilesSupplier(supplier); } } // Always register the action @@ -465,7 +463,7 @@ public class SkylarkRuleImplementationFunctions { throws EvalException, ConversionException { RuleContext ruleContext = ctx.getRuleContext(); Action action = - new PseudoAction<SpawnInfo>( + new PseudoAction<>( generateUuid(ruleContext), ruleContext.getActionOwner(), convertInputs(inputs), @@ -661,7 +659,7 @@ public class SkylarkRuleImplementationFunctions { private static Map<Label, Iterable<Artifact>> checkLabelDict( Map<?, ?> labelDict, Location loc) throws EvalException { - Map<Label, Iterable<Artifact>> convertedMap = new HashMap<Label, Iterable<Artifact>>(); + Map<Label, Iterable<Artifact>> convertedMap = new HashMap<>(); for (Map.Entry<?, ?> entry : labelDict.entrySet()) { Object key = entry.getKey(); if (!(key instanceof Label)) { @@ -695,12 +693,14 @@ public class SkylarkRuleImplementationFunctions { @SkylarkSignature( name = "resolve_command", + // TODO(bazel-team): The naming here isn't entirely accurate (input_manifests is no longer + // manifests), but this is experimental/should be opaque to the end user. doc = "<i>(Experimental)</i> " + "Returns a tuple <code>(inputs, command, input_manifests)</code> of the list of " - + "resolved inputs, the argv list for the resolved command, and the dict mapping " - + "locations to runfiles required to run the command, all of them suitable for passing " - + "as the same-named arguments of the <code>ctx.action</code> method.", + + "resolved inputs, the argv list for the resolved command, and the runfiles metadata" + + "required to run the command, all of them suitable for passing as the same-named " + + "arguments of the <code>ctx.action</code> method.", objectType = SkylarkRuleContext.class, returnType = Tuple.class, parameters = { @@ -823,7 +823,7 @@ public class SkylarkRuleImplementationFunctions { return Tuple.<Object>of( new MutableList(inputs, env), new MutableList(argv, env), - helper.getRemoteRunfileManifestMap()); + helper.getToolsRunfilesSuppliers()); } }; diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java index c483878551..7b03534760 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java @@ -16,12 +16,15 @@ package com.google.devtools.build.lib.rules.android; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.Runfiles; +import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.ApkSigningMethod; import com.google.devtools.build.lib.rules.java.JavaHelper; import com.google.devtools.build.lib.rules.java.JavaToolchainProvider; import com.google.devtools.build.lib.rules.java.Jvm; +import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Map; @@ -189,12 +192,19 @@ public class ApkActionsBuilder { .addInputArgument(javaResourceZip); } - Artifact nativeSymlinks = nativeLibs.createApkBuilderSymlinks(ruleContext); - if (nativeSymlinks != null) { - PathFragment nativeSymlinksDir = nativeSymlinks.getExecPath().getParentDirectory(); + Pair<Artifact, Runfiles> nativeSymlinksManifestAndRunfiles = + nativeLibs.createApkBuilderSymlinks(ruleContext); + if (nativeSymlinksManifestAndRunfiles != null) { + Artifact nativeSymlinksManifest = nativeSymlinksManifestAndRunfiles.first; + Runfiles nativeSymlinksRunfiles = nativeSymlinksManifestAndRunfiles.second; + PathFragment nativeSymlinksDir = nativeSymlinksManifest.getExecPath().getParentDirectory(); actionBuilder - .addInputManifest(nativeSymlinks, nativeSymlinksDir) - .addInput(nativeSymlinks) + .addRunfilesSupplier( + new RunfilesSupplierImpl( + nativeSymlinksDir, + nativeSymlinksRunfiles, + nativeSymlinksManifest)) + .addInput(nativeSymlinksManifest) .addInputs(nativeLibs.getAllNativeLibs()) .addArgument("-nf") // If the native libs are "foo/bar/x86/foo.so", we need to pass "foo/bar" here diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java index 3dadd0abf1..b5a8c5f6c6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java @@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Multimap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.SourceManifestAction; import com.google.devtools.build.lib.analysis.SourceManifestAction.ManifestType; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; @@ -33,6 +34,7 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider; import com.google.devtools.build.lib.rules.cpp.CppFileTypes; import com.google.devtools.build.lib.rules.cpp.LinkerInput; import com.google.devtools.build.lib.rules.nativedeps.NativeDepsHelper; +import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Collection; import java.util.HashMap; @@ -132,7 +134,7 @@ public final class NativeLibs { return result.build(); } - public Artifact createApkBuilderSymlinks(RuleContext ruleContext) { + public Pair<Artifact, Runfiles> createApkBuilderSymlinks(RuleContext ruleContext) { Map<PathFragment, Artifact> symlinks = new LinkedHashMap<>(); for (Map.Entry<String, Iterable<Artifact>> entry : nativeLibs.entrySet()) { String arch = entry.getKey(); @@ -146,11 +148,12 @@ public final class NativeLibs { } Artifact inputManifest = AndroidBinary.getDxArtifact(ruleContext, "native_symlinks.manifest"); - ruleContext.registerAction(new SourceManifestAction.Builder( + SourceManifestAction sourceManifestAction = new SourceManifestAction.Builder( ruleContext.getWorkspaceName(), ManifestType.SOURCE_SYMLINKS, ruleContext.getActionOwner(), inputManifest, ruleContext.getConfiguration().legacyExternalRunfiles()) .addRootSymlinks(symlinks) - .build()); + .build(); + ruleContext.registerAction(sourceManifestAction); Artifact outputManifest = AndroidBinary.getDxArtifact(ruleContext, "native_symlinks/MANIFEST"); Artifact nativeLibsMiddleman = ruleContext.getAnalysisEnvironment().getMiddlemanFactory().createRunfilesMiddleman( @@ -167,7 +170,7 @@ public final class NativeLibs { false, ruleContext.getConfiguration().getLocalShellEnvironment(), ruleContext.getConfiguration().runfilesEnabled())); - return outputManifest; + return Pair.of(outputManifest, sourceManifestAction.getGeneratedRunfiles()); } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendAction.java index e7f9411d0e..9e7b85684b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendAction.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.actions.ArtifactResolver; import com.google.devtools.build.lib.actions.PackageRootResolutionException; import com.google.devtools.build.lib.actions.PackageRootResolver; import com.google.devtools.build.lib.actions.ResourceSet; +import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.analysis.actions.CommandLine; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.collect.nestedset.NestedSet; @@ -77,7 +78,7 @@ public final class LTOBackendAction extends SpawnAction { Set<String> clientEnvironmentVariables, Map<String, String> executionInfo, String progressMessage, - ImmutableMap<PathFragment, Artifact> inputManifests, + RunfilesSupplier runfilesSupplier, String mnemonic) { super( owner, @@ -90,7 +91,7 @@ public final class LTOBackendAction extends SpawnAction { ImmutableSet.copyOf(clientEnvironmentVariables), ImmutableMap.copyOf(executionInfo), progressMessage, - ImmutableMap.copyOf(inputManifests), + runfilesSupplier, mnemonic, false, null); @@ -201,10 +202,10 @@ public final class LTOBackendAction extends SpawnAction { f.addString(GUID); f.addStrings(getArguments()); f.addString(getMnemonic()); - f.addInt(getInputManifests().size()); - for (Map.Entry<PathFragment, Artifact> input : getInputManifests().entrySet()) { - f.addString(input.getKey().getPathString() + "/"); - f.addPath(input.getValue().getExecPath()); + f.addPaths(getRunfilesSupplier().getRunfilesDirs()); + ImmutableList<Artifact> runfilesManifests = getRunfilesSupplier().getManifests(); + for (Artifact runfilesManifest : runfilesManifests) { + f.addPath(runfilesManifest.getExecPath()); } for (Artifact input : getMandatoryInputs()) { f.addPath(input.getExecPath()); @@ -242,7 +243,7 @@ public final class LTOBackendAction extends SpawnAction { ImmutableSet<String> clientEnvironmentVariables, ImmutableMap<String, String> executionInfo, String progressMessage, - ImmutableMap<PathFragment, Artifact> inputAndToolManifests, + RunfilesSupplier runfilesSupplier, String mnemonic) { return new LTOBackendAction( inputsAndTools.toCollection(), @@ -255,7 +256,7 @@ public final class LTOBackendAction extends SpawnAction { clientEnvironmentVariables, executionInfo, progressMessage, - inputAndToolManifests, + runfilesSupplier, mnemonic); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java index d80c5812e4..f9e461661f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java @@ -26,9 +26,11 @@ import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactResolver; +import com.google.devtools.build.lib.actions.CompositeRunfilesSupplier; import com.google.devtools.build.lib.actions.DelegateSpawn; import com.google.devtools.build.lib.actions.PackageRootResolutionException; import com.google.devtools.build.lib.actions.PackageRootResolver; +import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.analysis.actions.CommandLine; @@ -53,7 +55,7 @@ import javax.annotation.concurrent.GuardedBy; public final class ExtraAction extends SpawnAction { private final Action shadowedAction; private final boolean createDummyOutput; - private final ImmutableMap<PathFragment, Artifact> runfilesManifests; + private final RunfilesSupplier runfilesSupplier; private final ImmutableSet<Artifact> extraActionInputs; // This can be read/written from multiple threads, and so accesses should be synchronized. @GuardedBy("this") @@ -71,9 +73,9 @@ public final class ExtraAction extends SpawnAction { } }; - public ExtraAction( + ExtraAction( ImmutableSet<Artifact> extraActionInputs, - Map<PathFragment, Artifact> runfilesManifests, + RunfilesSupplier runfilesSupplier, Collection<Artifact> outputs, Action shadowedAction, boolean createDummyOutput, @@ -86,7 +88,7 @@ public final class ExtraAction extends SpawnAction { super( shadowedAction.getOwner(), ImmutableList.<Artifact>of(), - createInputs(shadowedAction.getInputs(), extraActionInputs), + createInputs(shadowedAction.getInputs(), extraActionInputs, runfilesSupplier), outputs, AbstractAction.DEFAULT_RESOURCE_SET, argv, @@ -94,12 +96,13 @@ public final class ExtraAction extends SpawnAction { ImmutableSet.copyOf(clientEnvironmentVariables), ImmutableMap.copyOf(executionInfo), progressMessage, - getManifests(shadowedAction), + // TODO(michajlo): Do we need the runfiles manifest as an input / should this be composite? + shadowedAction.getRunfilesSupplier(), mnemonic, false, null); this.shadowedAction = shadowedAction; - this.runfilesManifests = ImmutableMap.copyOf(runfilesManifests); + this.runfilesSupplier = runfilesSupplier; this.createDummyOutput = createDummyOutput; this.extraActionInputs = extraActionInputs; @@ -110,16 +113,6 @@ public final class ExtraAction extends SpawnAction { } } - private static ImmutableMap<PathFragment, Artifact> getManifests(Action shadowedAction) { - // If the shadowed action is a SpawnAction, then we also add the input manifests to this - // action's input manifests. - // TODO(bazel-team): Also handle other action classes correctly. - if (shadowedAction instanceof SpawnAction) { - return ((SpawnAction) shadowedAction).getInputManifests(); - } - return ImmutableMap.of(); - } - @Override public boolean discoversInputs() { return shadowedAction.discoversInputs(); @@ -135,7 +128,7 @@ public final class ExtraAction extends SpawnAction { if (shadowedAction.discoversInputs() && shadowedAction instanceof AbstractAction) { Iterable<Artifact> additionalInputs = ((AbstractAction) shadowedAction).getInputFilesForExtraAction(actionExecutionContext); - updateInputs(createInputs(additionalInputs, extraActionInputs)); + updateInputs(createInputs(additionalInputs, extraActionInputs, runfilesSupplier)); return ImmutableSet.copyOf(additionalInputs); } return null; @@ -147,13 +140,16 @@ public final class ExtraAction extends SpawnAction { } private static NestedSet<Artifact> createInputs( - Iterable<Artifact> shadowedActionInputs, ImmutableSet<Artifact> extraActionInputs) { + Iterable<Artifact> shadowedActionInputs, + ImmutableSet<Artifact> extraActionInputs, + RunfilesSupplier extraActionRunfilesSupplier) { NestedSetBuilder<Artifact> result = new NestedSetBuilder<>(Order.STABLE_ORDER); if (shadowedActionInputs instanceof NestedSet) { result.addTransitive((NestedSet<Artifact>) shadowedActionInputs); } else { result.addAll(shadowedActionInputs); } + result.addAll(extraActionRunfilesSupplier.getArtifacts()); return result.addAll(extraActionInputs).build(); } @@ -221,11 +217,9 @@ public final class ExtraAction extends SpawnAction { public Spawn getSpawn(Map<String, String> clientEnv) { final Spawn base = super.getSpawn(clientEnv); return new DelegateSpawn(base) { - @Override public ImmutableMap<PathFragment, Artifact> getRunfilesManifests() { - ImmutableMap.Builder<PathFragment, Artifact> builder = ImmutableMap.builder(); - builder.putAll(super.getRunfilesManifests()); - builder.putAll(runfilesManifests); - return builder.build(); + @Override + public RunfilesSupplier getRunfilesSupplier() { + return new CompositeRunfilesSupplier(super.getRunfilesSupplier(), runfilesSupplier); } @Override public String getMnemonic() { return ExtraAction.this.getMnemonic(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java index e1b3c11b99..ef7062303b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.extra; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.CompositeRunfilesSupplier; import com.google.devtools.build.lib.analysis.CommandHelper; import com.google.devtools.build.lib.analysis.ConfigurationMakeVariableContext; import com.google.devtools.build.lib.analysis.ConfiguredTarget; @@ -27,11 +28,9 @@ import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; import com.google.devtools.build.lib.syntax.Type; - import java.util.List; /** @@ -78,7 +77,7 @@ public final class ExtraActionFactory implements RuleConfiguredTargetFactory { ExtraActionSpec spec = new ExtraActionSpec( commandHelper.getResolvedTools(), - commandHelper.getRemoteRunfileManifestMap(), + new CompositeRunfilesSupplier(commandHelper.getToolsRunfilesSuppliers()), resolvedData, outputTemplates, command, diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java index 748a7a5fc8..81f1a8683d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java +++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java @@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.analysis.CommandHelper; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; @@ -44,7 +45,7 @@ import java.util.Set; @Immutable public final class ExtraActionSpec implements TransitiveInfoProvider { private final ImmutableList<Artifact> resolvedTools; - private final ImmutableMap<PathFragment, Artifact> manifests; + private final RunfilesSupplier runfilesSupplier; private final ImmutableList<Artifact> resolvedData; private final ImmutableList<String> outputTemplates; private final ImmutableMap<String, String> executionInfo; @@ -54,7 +55,7 @@ public final class ExtraActionSpec implements TransitiveInfoProvider { ExtraActionSpec( Iterable<Artifact> resolvedTools, - Map<PathFragment, Artifact> manifests, + RunfilesSupplier runfilesSupplier, Iterable<Artifact> resolvedData, Iterable<String> outputTemplates, String command, @@ -62,7 +63,7 @@ public final class ExtraActionSpec implements TransitiveInfoProvider { Map<String, String> executionInfo, boolean requiresActionOutput) { this.resolvedTools = ImmutableList.copyOf(resolvedTools); - this.manifests = ImmutableMap.copyOf(manifests); + this.runfilesSupplier = runfilesSupplier; this.resolvedData = ImmutableList.copyOf(resolvedData); this.outputTemplates = ImmutableList.copyOf(outputTemplates); this.command = command; @@ -138,7 +139,7 @@ public final class ExtraActionSpec implements TransitiveInfoProvider { owningRule.registerAction( new ExtraAction( ImmutableSet.copyOf(extraActionInputs.build()), - manifests, + runfilesSupplier, extraActionOutputs, actionToShadow, createDummyOutput, diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java index d77b89452c..a45b382f49 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java @@ -21,10 +21,10 @@ import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ResourceSet; +import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.analysis.actions.CommandLine; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.events.EventHandler; -import com.google.devtools.build.lib.vfs.PathFragment; import java.util.List; /** @@ -46,7 +46,7 @@ public class GenRuleAction extends SpawnAction { ImmutableMap<String, String> environment, ImmutableSet<String> clientEnvironmentVariables, ImmutableMap<String, String> executionInfo, - ImmutableMap<PathFragment, Artifact> runfilesManifests, + RunfilesSupplier runfilesSupplier, String progressMessage) { super( owner, @@ -59,7 +59,7 @@ public class GenRuleAction extends SpawnAction { clientEnvironmentVariables, executionInfo, progressMessage, - runfilesManifests, + runfilesSupplier, "Genrule", false, null); diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java index c990b15561..932d58040e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java @@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.CompositeRunfilesSupplier; import com.google.devtools.build.lib.analysis.CommandHelper; import com.google.devtools.build.lib.analysis.ConfigurationMakeVariableContext; import com.google.devtools.build.lib.analysis.ConfiguredTarget; @@ -190,7 +191,7 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory { env, clientEnvVars, ImmutableMap.copyOf(executionInfo), - ImmutableMap.copyOf(commandHelper.getRemoteRunfileManifestMap()), + new CompositeRunfilesSupplier(commandHelper.getToolsRunfilesSuppliers()), message + ' ' + ruleContext.getLabel())); RunfilesProvider runfilesProvider = RunfilesProvider.withData( diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java index faad216147..4eb7a6927e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.PackageRootResolutionException; import com.google.devtools.build.lib.actions.PackageRootResolver; import com.google.devtools.build.lib.actions.ResourceSet; +import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.analysis.actions.CommandLine; import com.google.devtools.build.lib.analysis.actions.SpawnAction; @@ -74,7 +75,7 @@ public class ObjcCompileAction extends SpawnAction { * re-introduced into action inputs. */ public class ObjcCompileActionSpawn extends ActionSpawn { - + public ObjcCompileActionSpawn(Map<String, String> clientEnv) { super(clientEnv); } @@ -84,10 +85,10 @@ public class ObjcCompileAction extends SpawnAction { return ImmutableList.<ActionInput>builder() .addAll(super.getInputFiles()) .addAll(filterHeaderFiles()) - .build(); + .build(); } } - + private final DotdFile dotdFile; private final Artifact sourceFile; private final NestedSet<Artifact> mandatoryInputs; @@ -97,7 +98,7 @@ public class ObjcCompileAction extends SpawnAction { // This can be read/written from multiple threads, so accesses must be synchronized. @GuardedBy("this") private boolean inputsKnown = false; - + private static final String GUID = "a00d5bac-a72c-4f0f-99a7-d5fdc6072137"; private ObjcCompileAction( @@ -110,7 +111,7 @@ public class ObjcCompileAction extends SpawnAction { ImmutableMap<String, String> environment, ImmutableMap<String, String> executionInfo, String progressMessage, - ImmutableMap<PathFragment, Artifact> inputManifests, + RunfilesSupplier runfilesSupplier, String mnemonic, boolean executeUnconditionally, ExtraActionInfoSupplier<?> extraActionInfoSupplier, @@ -130,7 +131,7 @@ public class ObjcCompileAction extends SpawnAction { ImmutableSet.<String>of(), executionInfo, progressMessage, - inputManifests, + runfilesSupplier, mnemonic, executeUnconditionally, extraActionInfoSupplier); @@ -151,7 +152,7 @@ public class ObjcCompileAction extends SpawnAction { inputs.add(headerArtifact); } } - return inputs.build(); + return inputs.build(); } /** Returns the DotdPruningPlan for this compile */ @@ -164,12 +165,12 @@ public class ObjcCompileAction extends SpawnAction { public synchronized boolean inputsKnown() { return inputsKnown; } - + @Override public final Spawn getSpawn(Map<String, String> clientEnv) { return new ObjcCompileActionSpawn(clientEnv); } - + @Override public boolean discoversInputs() { return true; @@ -184,7 +185,7 @@ public class ObjcCompileAction extends SpawnAction { public Iterable<Artifact> getInputsWhenSkippingInputDiscovery() { return filterHeaderFiles(); } - + // Keep in sync with {@link CppCompileAction#resolveInputsFromCache} @Override public Iterable<Artifact> resolveInputsFromCache( @@ -225,17 +226,17 @@ public class ObjcCompileAction extends SpawnAction { inputs.add(artifact); } } - return inputs; + return inputs; } - + @Override public synchronized void updateInputs(Iterable<Artifact> inputs) { inputsKnown = true; synchronized (this) { setInputs(inputs); } - } - + } + @Override public ImmutableSet<Artifact> getMandatoryOutputs() { return ImmutableSet.of(dotdFile.artifact()); @@ -408,7 +409,7 @@ public class ObjcCompileAction extends SpawnAction { this.dotdPruningPlan = dotdPruningPlan; return this; } - + /** Adds to the set of all possible headers that could be required by this compile action. */ public Builder addTransitiveHeaders(NestedSet<Artifact> headers) { this.headers.addTransitive(Preconditions.checkNotNull(headers)); @@ -435,7 +436,7 @@ public class ObjcCompileAction extends SpawnAction { ImmutableSet<String> clientEnvironmentVariables, ImmutableMap<String, String> executionInfo, String progressMessage, - ImmutableMap<PathFragment, Artifact> inputAndToolManifests, + RunfilesSupplier runfilesSupplier, String mnemonic) { return new ObjcCompileAction( owner, @@ -447,7 +448,7 @@ public class ObjcCompileAction extends SpawnAction { env, executionInfo, progressMessage, - inputAndToolManifests, + runfilesSupplier, mnemonic, executeUnconditionally, extraActionInfoSupplier, diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java index ecffd79e4a..44a05f055c 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java @@ -282,7 +282,6 @@ public class DarwinSandboxedStrategy extends SandboxStrategy { spawnHelpers.mountInputs(mounts, spawn, executionContext); Map<PathFragment, Path> unfinalized = new HashMap<>(); - spawnHelpers.mountRunfilesFromManifests(unfinalized, spawn); spawnHelpers.mountRunfilesFromSuppliers(unfinalized, spawn); spawnHelpers.mountFilesFromFilesetManifests(unfinalized, spawn, executionContext); mounts.putAll(finalizeLinks(unfinalized)); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java index b5b920cc31..8e3c26f93d 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java @@ -52,25 +52,12 @@ public final class SpawnHelpers { public Map<PathFragment, Path> getMounts(Spawn spawn, ActionExecutionContext executionContext) throws IOException { Map<PathFragment, Path> mounts = new HashMap<>(); - mountRunfilesFromManifests(mounts, spawn); mountRunfilesFromSuppliers(mounts, spawn); mountFilesFromFilesetManifests(mounts, spawn, executionContext); mountInputs(mounts, spawn, executionContext); return mounts; } - /** Mount all runfiles that the spawn needs as specified in its runfiles manifests. */ - void mountRunfilesFromManifests(Map<PathFragment, Path> mounts, Spawn spawn) throws IOException { - for (Map.Entry<PathFragment, Artifact> manifest : spawn.getRunfilesManifests().entrySet()) { - String manifestFilePath = manifest.getValue().getPath().getPathString(); - Preconditions.checkState(!manifest.getKey().isAbsolute()); - PathFragment targetDirectory = manifest.getKey(); - - parseManifestFile( - execRoot.getFileSystem(), mounts, targetDirectory, new File(manifestFilePath), false, ""); - } - } - /** Mount all files that the spawn needs as specified in its fileset manifests. */ void mountFilesFromFilesetManifests( Map<PathFragment, Path> mounts, Spawn spawn, ActionExecutionContext executionContext) |