Switch to RunfilesSuppliers for communicating runfiles
ActionSpawn/SpawnAction now deal exclusively in RunfilesSuppliers, manifests
maps are no more.
There is some lingering awkwardness, in particular:
- Manifests still need to be tracked in some places, we can work out if this is
still necessary on a case by case basis.
- Skylark makes actions' runfiles available via 'resolve_command' where they are
consumed by 'action'. I've updated the documentation, though the name isn't
entirely accurate anymore. That being said these interfaces _are_ marked as
experimental, so we _should_ be able to be flexible here.
Overall, I think the benefits consolidating runfiles into suppliers, from both
code cleanliness and performance perspectives (no longer needing to parse
manifests), outweights the awkwardnesses.
RELNOTES: resolve_command/action's input_manifest return/parameter is now list
--
PiperOrigin-RevId: 145817429
MOS_MIGRATED_REVID=145817429
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 dd2d5de..6208db6 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 @@
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 @@
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 @@
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 @@
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 @@
}
@Override
- public ImmutableMap<PathFragment, Artifact> getRunfilesManifests() {
- return runfilesManifests;
- }
-
- @Override
public RunfilesSupplier getRunfilesSupplier() {
return runfilesSupplier;
}
@@ -234,10 +179,8 @@
/** @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/actions/CompositeRunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java
new file mode 100644
index 0000000..da119ab
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java
@@ -0,0 +1,83 @@
+// Copyright 2015 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+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.vfs.PathFragment;
+import java.io.IOException;
+import java.util.Map;
+
+/** 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 ImmutableList<RunfilesSupplier> suppliers;
+
+ /** Create an instance with {@code first} taking precedence over {@code second}. */
+ public CompositeRunfilesSupplier(RunfilesSupplier first, RunfilesSupplier 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();
+ for (RunfilesSupplier supplier : suppliers) {
+ result.addAll(supplier.getArtifacts());
+ }
+ return result.build();
+ }
+
+ @Override
+ public ImmutableSet<PathFragment> getRunfilesDirs() {
+ ImmutableSet.Builder<PathFragment> result = ImmutableSet.builder();
+ 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();
+ 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 45a1496..821407b 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.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 @@
}
@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 03da101..9cf6d1b 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.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 @@
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 3393833..9b05cb1 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 @@
* @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 a069db6..ae2358f 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.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 @@
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 c24f659..c711787 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.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 @@
@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 @@
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 @@
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 @@
}
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 @@
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/CompositeRunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/analysis/CompositeRunfilesSupplier.java
deleted file mode 100644
index ae2c465..0000000
--- a/src/main/java/com/google/devtools/build/lib/analysis/CompositeRunfilesSupplier.java
+++ /dev/null
@@ -1,63 +0,0 @@
-// Copyright 2015 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.analysis;
-
-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. */
-public class CompositeRunfilesSupplier implements RunfilesSupplier {
-
- private final RunfilesSupplier first;
- private final RunfilesSupplier second;
-
- /** 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);
- }
-
- @Override
- public Iterable<Artifact> getArtifacts() {
- ImmutableSet.Builder<Artifact> result = ImmutableSet.builder();
- result.addAll(first.getArtifacts());
- result.addAll(second.getArtifacts());
- return result.build();
- }
-
- @Override
- public ImmutableSet<PathFragment> getRunfilesDirs() {
- ImmutableSet.Builder<PathFragment> result = ImmutableSet.builder();
- result.addAll(first.getRunfilesDirs());
- result.addAll(second.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());
- return ImmutableMap.copyOf(result);
- }
-}
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 474784c..79ecce1 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.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 @@
}
/**
- * 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 ff4231f..7004e29 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 @@
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 4339a02..ba7da94 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.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.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 @@
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 @@
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 @@
outputTreeArtifact,
entries,
spawnCommandLine(),
- zipperExecutableRunfilesManifest(),
+ zipper.getRunfilesSupplier(),
this);
}
@@ -269,16 +269,6 @@
"@" + 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 b7a094a..cf0f4eb 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.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 @@
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 @@
ImmutableSet.copyOf(clientEnvironmentVariables),
ImmutableMap.<String, String>of(),
progressMessage,
- ImmutableMap.<PathFragment, Artifact>of(),
+ EmptyRunfilesSupplier.INSTANCE,
mnemonic,
false,
null);
@@ -169,8 +170,7 @@
* 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 @@
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 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 @@
*
* 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 @@
// 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 @@
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 @@
// 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 @@
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 @@
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 @@
.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 @@
ImmutableSet.copyOf(clientEnv),
ImmutableMap.copyOf(executionInfo),
progressMessage,
- ImmutableMap.copyOf(inputAndToolManifests),
+ new CompositeRunfilesSupplier(
+ Iterables.concat(this.inputRunfilesSuppliers, this.toolRunfilesSuppliers)),
mnemonic);
}
@@ -705,7 +698,7 @@
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 @@
clientEnvironmentVariables,
executionInfo,
progressMessage,
- inputAndToolManifests,
+ runfilesSupplier,
mnemonic,
executeUnconditionally,
extraActionInfoSupplier);
@@ -790,13 +783,8 @@
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 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 c58334d..6792e4e 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.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 @@
+ "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 @@
"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 @@
throws EvalException, ConversionException {
RuleContext ruleContext = ctx.getRuleContext();
Action action =
- new PseudoAction<SpawnInfo>(
+ new PseudoAction<>(
generateUuid(ruleContext),
ruleContext.getActionOwner(),
convertInputs(inputs),
@@ -661,7 +659,7 @@
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 @@
@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 @@
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 c483878..7b03534 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 @@
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 @@
.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 3dadd0a..b5a8c5f 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.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.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 @@
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 @@
}
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 @@
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 e7f9411..9e7b856 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.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 @@
Set<String> clientEnvironmentVariables,
Map<String, String> executionInfo,
String progressMessage,
- ImmutableMap<PathFragment, Artifact> inputManifests,
+ RunfilesSupplier runfilesSupplier,
String mnemonic) {
super(
owner,
@@ -90,7 +91,7 @@
ImmutableSet.copyOf(clientEnvironmentVariables),
ImmutableMap.copyOf(executionInfo),
progressMessage,
- ImmutableMap.copyOf(inputManifests),
+ runfilesSupplier,
mnemonic,
false,
null);
@@ -201,10 +202,10 @@
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 @@
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 @@
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 d80c581..f9e4616 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.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 @@
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 ExtraAction(
+ ExtraAction(
ImmutableSet<Artifact> extraActionInputs,
- Map<PathFragment, Artifact> runfilesManifests,
+ RunfilesSupplier runfilesSupplier,
Collection<Artifact> outputs,
Action shadowedAction,
boolean createDummyOutput,
@@ -86,7 +88,7 @@
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 @@
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 @@
}
}
- 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 @@
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 @@
}
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 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 e1b3c11b..ef70623 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 @@
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.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 @@
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 748a7a5..81f1a86 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.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 @@
@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 @@
ExtraActionSpec(
Iterable<Artifact> resolvedTools,
- Map<PathFragment, Artifact> manifests,
+ RunfilesSupplier runfilesSupplier,
Iterable<Artifact> resolvedData,
Iterable<String> outputTemplates,
String command,
@@ -62,7 +63,7 @@
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 @@
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 d77b894..a45b382 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.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 @@
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 @@
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 c990b15..932d580 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.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 @@
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 faad216..4eb7a69 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.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 @@
* re-introduced into action inputs.
*/
public class ObjcCompileActionSpawn extends ActionSpawn {
-
+
public ObjcCompileActionSpawn(Map<String, String> clientEnv) {
super(clientEnv);
}
@@ -84,10 +85,10 @@
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 @@
// 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 @@
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 @@
ImmutableSet.<String>of(),
executionInfo,
progressMessage,
- inputManifests,
+ runfilesSupplier,
mnemonic,
executeUnconditionally,
extraActionInfoSupplier);
@@ -151,7 +152,7 @@
inputs.add(headerArtifact);
}
}
- return inputs.build();
+ return inputs.build();
}
/** Returns the DotdPruningPlan for this compile */
@@ -164,12 +165,12 @@
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 Iterable<Artifact> getInputsWhenSkippingInputDiscovery() {
return filterHeaderFiles();
}
-
+
// Keep in sync with {@link CppCompileAction#resolveInputsFromCache}
@Override
public Iterable<Artifact> resolveInputsFromCache(
@@ -225,17 +226,17 @@
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 @@
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 @@
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 @@
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 ecffd79..44a05f0 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 @@
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 b5b920c..8e3c26f 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 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)
diff --git a/src/test/java/com/google/devtools/build/lib/actions/BaseSpawnTest.java b/src/test/java/com/google/devtools/build/lib/actions/BaseSpawnTest.java
index 07abe52..34deb6e 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/BaseSpawnTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/BaseSpawnTest.java
@@ -21,61 +21,30 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.Runfiles;
import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl;
-import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.vfs.PathFragment;
-
-import org.junit.Before;
+import java.util.Map;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-import java.io.IOException;
-import java.util.Map;
-
/** Tests for BaseSpawns less trivial aspects. */
@RunWith(JUnit4.class)
public class BaseSpawnTest {
- private Root rootDir;
-
- @Before
- public final void setup() throws IOException {
- Scratch scratch = new Scratch();
- rootDir = Root.asDerivedRoot(scratch.dir("/fake/root/dont/matter"));
- }
-
@Test
public void testGetEnvironmentDoesntAddRunfilesVarsWhenSourcesAreEmpty() {
Map<String, String> baseEnviron = ImmutableMap.of("HELLO", "world");
- BaseSpawn underTest = minimalBaseSpawn(baseEnviron, ImmutableMap.<PathFragment, Artifact>of(),
- EmptyRunfilesSupplier.INSTANCE);
+ BaseSpawn underTest = minimalBaseSpawn(baseEnviron, EmptyRunfilesSupplier.INSTANCE);
assertThat(underTest.getEnvironment()).isEqualTo(baseEnviron);
}
@Test
- public void testGetEnvironmentAddsRunfilesWhenOnlyOneSuppliedViaManifests() {
- Map<String, String> baseEnviron = ImmutableMap.of("HELLO", "world");
- final String runfilesDir = "runfilesdir";
- BaseSpawn underTest = minimalBaseSpawn(baseEnviron,
- ImmutableMap.of(new PathFragment(runfilesDir), mkArtifact("dontmatter", rootDir)),
- EmptyRunfilesSupplier.INSTANCE);
-
- Map<String, String> expected = ImmutableMap.<String, String>builder()
- .putAll(baseEnviron)
- .put("PYTHON_RUNFILES", runfilesDir)
- .put("JAVA_RUNFILES", runfilesDir)
- .build();
-
- assertThat(underTest.getEnvironment()).isEqualTo(expected);
- }
-
- @Test
public void testGetEnvironmentAddsRunfilesWhenOnlyOneSuppliedViaRunfilesSupplier() {
Map<String, String> baseEnviron = ImmutableMap.of("HELLO", "world");
final String runfilesDir = "runfilesdir";
- BaseSpawn underTest = minimalBaseSpawn(baseEnviron,
- ImmutableMap.<PathFragment, Artifact>of(),
+ BaseSpawn underTest = minimalBaseSpawn(
+ baseEnviron,
new RunfilesSupplierImpl(new PathFragment(runfilesDir), Runfiles.EMPTY));
Map<String, String> expected = ImmutableMap.<String, String>builder()
@@ -88,55 +57,26 @@
}
@Test
- public void testGetEnvironmentDoesntAddRunfilesWhenSupplierAndManifestsSupplied() {
- Map<String, String> baseEnviron = ImmutableMap.of("HELLO", "world");
- BaseSpawn underTest = minimalBaseSpawn(baseEnviron,
- ImmutableMap.of(new PathFragment("runfilesdir"), mkArtifact("dontmatter", rootDir)),
- new RunfilesSupplierImpl(new PathFragment("runfilesdir2"), Runfiles.EMPTY));
-
- assertThat(underTest.getEnvironment()).isEqualTo(baseEnviron);
- }
-
- @Test
public void testGetEnvironmentDoesntAddRunfilesWhenMultipleManifestsSupplied() {
Map<String, String> baseEnviron = ImmutableMap.of("HELLO", "world");
BaseSpawn underTest = minimalBaseSpawn(baseEnviron,
- ImmutableMap.of(
- new PathFragment("runfilesdir1"), mkArtifact("dontmatter", rootDir),
- new PathFragment("runfilesdir2"), mkArtifact("stilldontmatter", rootDir)),
- EmptyRunfilesSupplier.INSTANCE);
-
- assertThat(underTest.getEnvironment()).isEqualTo(baseEnviron);
- }
-
- @Test
- public void testGetEnvironmentDoesntAddRunfilesWhenMultipleSuppliersSupplied() {
- Map<String, String> baseEnviron = ImmutableMap.of("HELLO", "world");
- BaseSpawn underTest = minimalBaseSpawn(baseEnviron,
- ImmutableMap.<PathFragment, Artifact>of(),
- new RunfilesSupplierImpl(ImmutableMap.of(
- new PathFragment("runfilesdir1"), Runfiles.EMPTY,
- new PathFragment("runfilesdir2"), Runfiles.EMPTY)));
+ new CompositeRunfilesSupplier(
+ new RunfilesSupplierImpl(new PathFragment("rfdir1"), Runfiles.EMPTY),
+ new RunfilesSupplierImpl(new PathFragment("rfdir2"), Runfiles.EMPTY)));
assertThat(underTest.getEnvironment()).isEqualTo(baseEnviron);
}
private static BaseSpawn minimalBaseSpawn(
Map<String, String> environment,
- Map<PathFragment, Artifact> runfilesManifests,
RunfilesSupplier runfilesSupplier) {
return new BaseSpawn(
ImmutableList.<String>of(),
environment,
ImmutableMap.<String, String>of(),
- runfilesManifests,
runfilesSupplier,
null,
null,
ImmutableSet.<PathFragment>of());
}
-
- private static Artifact mkArtifact(String path, Root rootDir) {
- return new Artifact(new PathFragment(path), rootDir);
- }
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/CompositeRunfilesSupplierTest.java b/src/test/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplierTest.java
similarity index 72%
rename from src/test/java/com/google/devtools/build/lib/analysis/CompositeRunfilesSupplierTest.java
rename to src/test/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplierTest.java
index 4f7627b..507fc8b 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/CompositeRunfilesSupplierTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplierTest.java
@@ -12,7 +12,7 @@
// 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 static com.google.common.truth.Truth.assertThat;
import static org.mockito.Mockito.mock;
@@ -21,21 +21,16 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.Root;
-import com.google.devtools.build.lib.actions.RunfilesSupplier;
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.vfs.PathFragment;
-
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-import java.io.IOException;
-import java.util.List;
-import java.util.Map;
-
/** Tests for CompositeRunfilesSupplier */
@RunWith(JUnit4.class)
public class CompositeRunfilesSupplierTest {
@@ -45,8 +40,6 @@
private Root rootDir;
- private CompositeRunfilesSupplier underTest;
-
@Before
public final void createMocks() throws IOException {
Scratch scratch = new Scratch();
@@ -54,7 +47,6 @@
mockFirst = mock(RunfilesSupplier.class);
mockSecond = mock(RunfilesSupplier.class);
- underTest = new CompositeRunfilesSupplier(mockFirst, mockSecond);
}
@Test
@@ -62,6 +54,7 @@
when(mockFirst.getArtifacts()).thenReturn(mkArtifacts(rootDir, "first", "shared"));
when(mockSecond.getArtifacts()).thenReturn(mkArtifacts(rootDir, "second", "shared"));
+ CompositeRunfilesSupplier underTest = new CompositeRunfilesSupplier(mockFirst, mockSecond);
assertThat(underTest.getArtifacts()).containsExactlyElementsIn(
mkArtifacts(rootDir, "first", "second", "shared"));
}
@@ -75,6 +68,7 @@
when(mockFirst.getRunfilesDirs()).thenReturn(ImmutableSet.of(first, shared));
when(mockSecond.getRunfilesDirs()).thenReturn(ImmutableSet.of(second, shared));
+ CompositeRunfilesSupplier underTest = new CompositeRunfilesSupplier(mockFirst, mockSecond);
assertThat(underTest.getRunfilesDirs()).containsExactlyElementsIn(
ImmutableSet.of(first, second, shared));
}
@@ -99,12 +93,42 @@
shared, secondSharedMappings));
// We expect the mappings for shared added by mockSecond to be dropped.
- assertThat(underTest.getMappings()).isEqualTo(ImmutableMap.of(
+ CompositeRunfilesSupplier underTest = new CompositeRunfilesSupplier(mockFirst, mockSecond);
+ assertThat(underTest.getMappings()).containsExactly(
first, firstMappings,
second, secondMappings,
- shared, firstSharedMappings));
+ shared, firstSharedMappings);
}
+ @Test
+ public void testGetMappingsViaListConstructorReturnsMappingsWithFirstPrecedenceOverSecond()
+ throws IOException {
+ PathFragment first = new PathFragment("first");
+ Map<PathFragment, Artifact> firstMappings = mkMappings(rootDir, "first1", "first2");
+
+ PathFragment second = new PathFragment("second");
+ Map<PathFragment, Artifact> secondMappings = mkMappings(rootDir, "second1", "second2");
+
+ PathFragment shared = new PathFragment("shared");
+ Map<PathFragment, Artifact> firstSharedMappings = mkMappings(rootDir, "shared1", "shared2");
+ Map<PathFragment, Artifact> secondSharedMappings = mkMappings(rootDir, "lost1", "lost2");
+
+ when(mockFirst.getMappings()).thenReturn(ImmutableMap.of(
+ first, firstMappings,
+ shared, firstSharedMappings));
+ when(mockSecond.getMappings()).thenReturn(ImmutableMap.of(
+ second, secondMappings,
+ shared, secondSharedMappings));
+
+ // We expect the mappings for shared added by mockSecond to be dropped.
+ CompositeRunfilesSupplier underTest =
+ new CompositeRunfilesSupplier(ImmutableList.of(mockFirst, mockSecond));
+ assertThat(underTest.getMappings()).containsExactly(
+ first, firstMappings,
+ second, secondMappings,
+ shared, firstSharedMappings);
+ }
+
private static Map<PathFragment, Artifact> mkMappings(Root rootDir, String... paths) {
ImmutableMap.Builder<PathFragment, Artifact> builder = ImmutableMap.builder();
for (String path : paths) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImplTest.java b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImplTest.java
index 70024c4..0234699 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImplTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesSupplierImplTest.java
@@ -17,7 +17,6 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Root;
@@ -25,15 +24,13 @@
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
-
+import java.io.IOException;
+import java.util.List;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-import java.io.IOException;
-import java.util.List;
-
/** Tests for RunfilesSupplierImpl */
@RunWith(JUnit4.class)
public class RunfilesSupplierImplTest {
@@ -54,37 +51,39 @@
public void testGetArtifactsWithSingleMapping() {
List<Artifact> artifacts = mkArtifacts(rootDir, "thing1", "thing2");
- RunfilesSupplierImpl underTest = new RunfilesSupplierImpl(
- ImmutableMap.of(new PathFragment("notimportant"), mkRunfiles(artifacts)));
+ RunfilesSupplierImpl underTest =
+ new RunfilesSupplierImpl(new PathFragment("notimportant"), mkRunfiles(artifacts));
assertThat(underTest.getArtifacts()).containsExactlyElementsIn(artifacts);
}
@Test
- public void testGetArtifactsWithMultipleMappings() {
- List<Artifact> artifacts1 = mkArtifacts(rootDir, "thing_1", "thing_2", "duplicated");
- List<Artifact> artifacts2 = mkArtifacts(rootDir, "thing_3", "thing_4", "duplicated");
-
- RunfilesSupplierImpl underTest = new RunfilesSupplierImpl(ImmutableMap.of(
- new PathFragment("notimportant"), mkRunfiles(artifacts1),
- new PathFragment("stillnotimportant"), mkRunfiles(artifacts2)));
-
- assertThat(underTest.getArtifacts()).containsExactlyElementsIn(
- mkArtifacts(rootDir, "thing_1", "thing_2", "thing_3", "thing_4", "duplicated"));
- }
-
- @Test
public void testGetArtifactsFilterMiddlemen() {
List<Artifact> artifacts = mkArtifacts(rootDir, "thing1", "thing2");
Artifact middleman = new Artifact(new PathFragment("middleman"), middlemanRoot);
Runfiles runfiles = mkRunfiles(Iterables.concat(artifacts, ImmutableList.of(middleman)));
- RunfilesSupplier underTest = new RunfilesSupplierImpl(
- ImmutableMap.of(new PathFragment("notimportant"), runfiles));
+ RunfilesSupplier underTest =
+ new RunfilesSupplierImpl(new PathFragment("notimportant"), runfiles);
assertThat(underTest.getArtifacts()).containsExactlyElementsIn(artifacts);
}
+ @Test
+ public void testGetManifestsWhenNone() {
+ RunfilesSupplier underTest =
+ new RunfilesSupplierImpl(new PathFragment("ignored"), Runfiles.EMPTY, null);
+ assertThat(underTest.getManifests()).isEmpty();
+ }
+
+ @Test
+ public void testGetManifestsWhenSupplied() {
+ Artifact manifest = new Artifact(new PathFragment("manifest"), rootDir);
+ RunfilesSupplier underTest =
+ new RunfilesSupplierImpl(new PathFragment("ignored"), Runfiles.EMPTY, manifest);
+ assertThat(underTest.getManifests()).containsExactly(manifest);
+ }
+
private static Runfiles mkRunfiles(Iterable<Artifact> artifacts) {
return new Runfiles.Builder("TESTING", false).addArtifacts(artifacts).build();
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java
index 2097997..150ed7a 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java
@@ -30,10 +30,13 @@
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
+import com.google.devtools.build.lib.actions.RunfilesSupplier;
import com.google.devtools.build.lib.actions.extra.EnvironmentVariable;
import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
import com.google.devtools.build.lib.actions.extra.SpawnInfo;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
+import com.google.devtools.build.lib.analysis.Runfiles;
+import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl;
import com.google.devtools.build.lib.analysis.util.ActionTester;
import com.google.devtools.build.lib.analysis.util.ActionTester.ActionCombinationFactory;
import com.google.devtools.build.lib.analysis.util.AnalysisTestUtil;
@@ -345,11 +348,12 @@
}
@Test
- public void testInputManifest() throws Exception {
+ public void testInputManifestsRemovedIfSupplied() throws Exception {
Artifact manifest = getSourceArtifact("MANIFEST");
Action[] actions = builder()
.addInput(manifest)
- .addInputManifest(manifest, new PathFragment("/destination/"))
+ .addRunfilesSupplier(
+ new RunfilesSupplierImpl(new PathFragment("/destination/"), Runfiles.EMPTY, manifest))
.addOutput(getBinArtifactWithNoOwner("output"))
.setExecutable(scratch.file("/bin/xxx").asFragment())
.setProgressMessage("Test")
@@ -381,15 +385,15 @@
builder.setMnemonic((i & 4) == 0 ? "a" : "b");
if ((i & 8) == 0) {
- builder.addInputManifest(artifactA, new PathFragment("a"));
+ builder.addRunfilesSupplier(runfilesSupplier(artifactA, new PathFragment("a")));
} else {
- builder.addInputManifest(artifactB, new PathFragment("a"));
+ builder.addRunfilesSupplier(runfilesSupplier(artifactB, new PathFragment("a")));
}
if ((i & 16) == 0) {
- builder.addInputManifest(artifactA, new PathFragment("aa"));
+ builder.addRunfilesSupplier(runfilesSupplier(artifactA, new PathFragment("aa")));
} else {
- builder.addInputManifest(artifactA, new PathFragment("ab"));
+ builder.addRunfilesSupplier(runfilesSupplier(artifactA, new PathFragment("ab")));
}
Map<String, String> env = new HashMap<>();
@@ -462,4 +466,8 @@
.containsExactly(
"parameter", ExtraActionInfo.StringList.newBuilder().addValue("param_value").build());
}
+
+ private static RunfilesSupplier runfilesSupplier(Artifact manifest, PathFragment dir) {
+ return new RunfilesSupplierImpl(dir, Runfiles.EMPTY, manifest);
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LTOBackendActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LTOBackendActionTest.java
index 21922f5..bef984f 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LTOBackendActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LTOBackendActionTest.java
@@ -23,6 +23,8 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Executor;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
+import com.google.devtools.build.lib.analysis.Runfiles;
+import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl;
import com.google.devtools.build.lib.analysis.util.ActionTester;
import com.google.devtools.build.lib.analysis.util.ActionTester.ActionCombinationFactory;
import com.google.devtools.build.lib.analysis.util.AnalysisTestUtil;
@@ -160,9 +162,11 @@
builder.setMnemonic((i & 4) == 0 ? "a" : "b");
if ((i & 8) == 0) {
- builder.addInputManifest(artifactA, new PathFragment("a"));
+ builder.addRunfilesSupplier(
+ new RunfilesSupplierImpl(new PathFragment("a"), Runfiles.EMPTY, artifactA));
} else {
- builder.addInputManifest(artifactB, new PathFragment("a"));
+ builder.addRunfilesSupplier(
+ new RunfilesSupplierImpl(new PathFragment("a"), Runfiles.EMPTY, artifactB));
}
if ((i & 16) == 0) {
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
index 3749437..28743a93 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
@@ -26,6 +26,8 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.CompositeRunfilesSupplier;
+import com.google.devtools.build.lib.actions.RunfilesSupplier;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.Runfiles;
@@ -532,13 +534,15 @@
public void testResolveCommandInputs() throws Exception {
evalRuleContextCode(
createRuleContext("//foo:resolve_me"),
- "inputs, argv, manifests = ruleContext.resolve_command(",
+ "inputs, argv, input_manifests = ruleContext.resolve_command(",
" tools=ruleContext.attr.tools)");
@SuppressWarnings("unchecked")
List<Artifact> inputs = (List<Artifact>) (List<?>) (MutableList) lookup("inputs");
assertArtifactFilenames(inputs, "mytool.sh", "mytool", "foo_Smytool-runfiles", "t.exe");
- Map<?, ?> manifests = (Map<?, ?>) lookup("manifests");
- assertThat(manifests).hasSize(1);
+ @SuppressWarnings("unchecked")
+ CompositeRunfilesSupplier runfilesSupplier =
+ new CompositeRunfilesSupplier((List<RunfilesSupplier>) lookup("input_manifests"));
+ assertThat(runfilesSupplier.getMappings()).hasSize(1);
}
@Test
