Put a reference to RunfilesTree into MiddlemanAction and simplify the…

… creation of the latter.

The plan is that it will be plumbed to RunfilesArtifactValue, thereby making RunfilesSupplier unnecessary.

I don't know yet how that will work; the most likely approach is an ugly downcast of Action to MiddlemanAction in ArtifactFunction, but I hope I can come up with something more palatable. So for now, the private field remains unused.

Since now there is only one kind of middleman, it was possible to delete a pleasant amount of code, including MiddlemanActionTest.

RELNOTES: None.
PiperOrigin-RevId: 597526236
Change-Id: I392fc662288cc5dbee3a058d246182916343f8cc

src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java

@@ -18,6 +18,7 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
+import com.google.devtools.build.lib.actions.RunfilesSupplier.RunfilesTree;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -33,25 +34,23 @@
 @Immutable
 public final class MiddlemanAction extends AbstractAction {
  public static final String MIDDLEMAN_MNEMONIC = "Middleman";
- private final String description;
- private final MiddlemanType middlemanType;
 
- private MiddlemanAction(
+ /** The runfiles tree this middleman stands for. */
+ private final RunfilesTree runfilesTree;
+
+ public MiddlemanAction(
  ActionOwner owner,
+ RunfilesTree runfilesTree,
  NestedSet<Artifact> inputs,
- ImmutableSet<Artifact> outputs,
- String description,
- MiddlemanType middlemanType) {
+ ImmutableSet<Artifact> outputs) {
  super(owner, inputs, outputs);
- Preconditions.checkNotNull(middlemanType);
+
+ this.runfilesTree = runfilesTree;
  Preconditions.checkArgument(Iterables.getOnlyElement(outputs).isMiddlemanArtifact(), outputs);
- Preconditions.checkNotNull(description);
- this.description = description;
- this.middlemanType = middlemanType;
  }
 
  @Override
- public final ActionResult execute(ActionExecutionContext actionExecutionContext) {
+ public ActionResult execute(ActionExecutionContext actionExecutionContext) {
  throw new IllegalStateException("MiddlemanAction should never be executed");
  }
 
@@ -70,7 +69,7 @@ protected void computeKey(
  */
  @Override
  public MiddlemanType getActionType() {
- return middlemanType;
+ return MiddlemanType.RUNFILES_MIDDLEMAN;
  }
 
  @Nullable
@@ -81,7 +80,7 @@ protected String getRawProgressMessage() {
 
  @Override
  public String prettyPrint() {
- return description + " for " + Label.print(getOwner().getLabel());
+ return "runfiles for " + Label.print(getOwner().getLabel());
  }
 
  @Override
@@ -106,18 +105,4 @@ public ImmutableMap<String, String> getExecProperties() {
  // Middleman actions do not execute actual actions, and therefore have no execution properties.
  return ImmutableMap.of();
  }
-
- /** Creates a new middleman action. */
- public static Action create(
- ActionRegistry env,
- ActionOwner owner,
- NestedSet<Artifact> inputs,
- Artifact stampFile,
- String purpose,
- MiddlemanType middlemanType) {
- MiddlemanAction action =
- new MiddlemanAction(owner, inputs, ImmutableSet.of(stampFile), purpose, middlemanType);
- env.registerAction(action);
- return action;
- }
 }

src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java

@@ -14,10 +14,12 @@
 package com.google.devtools.build.lib.actions;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.actions.RunfilesSupplier.RunfilesTree;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
-import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import javax.annotation.Nullable;
 
@@ -48,59 +50,44 @@ public MiddlemanFactory(
  * Further, if the owning Artifact is non-null, the owning Artifacts' root-relative path must
  * be unique and the artifact must be part of the runfiles tree for which this middleman is
  * created. Usually this artifact will be an executable program.
- * @param inputs the set of artifacts for which the created artifact is to be the middleman.
+ * @param runfilesTree the runfiles tree for which the middleman being created for
+ * @param runfilesManifest the runfiles manifest for the runfiles tree
+ * @param repoMappingManifest the repo mapping manifest for the runfiles tree
  * @param middlemanDir the directory in which to place the middleman.
  */
  public Artifact createRunfilesMiddleman(
  ActionOwner owner,
  @Nullable Artifact owningArtifact,
- NestedSet<Artifact> inputs,
- ArtifactRoot middlemanDir,
- String tag) {
+ RunfilesTree runfilesTree,
+ @Nullable Artifact runfilesManifest,
+ @Nullable Artifact repoMappingManifest,
+ ArtifactRoot middlemanDir) {
  Preconditions.checkArgument(middlemanDir.isMiddlemanRoot());
- if (inputs.isSingleton()) { // Optimization: No middleman for just one input.
- return inputs.getSingleton();
+
+ NestedSetBuilder<Artifact> depsBuilder = NestedSetBuilder.stableOrder();
+ depsBuilder.addTransitive(runfilesTree.getArtifacts());
+ if (runfilesManifest != null) {
+ depsBuilder.add(runfilesManifest);
+ }
+ if (repoMappingManifest != null) {
+ depsBuilder.add(repoMappingManifest);
+ }
+
+ NestedSet<Artifact> deps = depsBuilder.build();
+ if (deps.isSingleton()) { // Optimization: No middleman for just one input.
+ return deps.getSingleton();
  }
  String middlemanPath = owningArtifact == null
  ? Label.print(owner.getLabel())
  : owningArtifact.getRootRelativePath().getPathString();
- return createMiddleman(owner, middlemanPath, tag, inputs, middlemanDir,
- MiddlemanType.RUNFILES_MIDDLEMAN).getFirst();
- }
-
- /**
- * Creates middlemen.
- *
- * <p>Note: there's no need to synchronize this method; the only use of a field is via a call to
- * another synchronized method (getArtifact()).
- *
- * @return null iff {@code inputs} is null or empty; the middleman file and the middleman action
- * otherwise
- */
- @Nullable
- private Pair<Artifact, Action> createMiddleman(
- ActionOwner owner,
- String middlemanName,
- String purpose,
- NestedSet<Artifact> inputs,
- ArtifactRoot middlemanDir,
- MiddlemanType middlemanType) {
- if (inputs == null || inputs.isEmpty()) {
- return null;
- }
-
- Artifact stampFile = getStampFileArtifact(middlemanName, purpose, middlemanDir);
- Action action =
- MiddlemanAction.create(actionRegistry, owner, inputs, stampFile, purpose, middlemanType);
- return Pair.of(stampFile, action);
- }
-
- private Artifact.DerivedArtifact getStampFileArtifact(
- String middlemanName, String purpose, ArtifactRoot middlemanDir) {
- String escapedFilename = Actions.escapedPath(middlemanName);
- PathFragment stampName = PathFragment.create("_middlemen/" + escapedFilename + "-" + purpose);
+ String escapedFilename = Actions.escapedPath(middlemanPath);
+ PathFragment stampName = PathFragment.create("_middlemen/" + escapedFilename + "-runfiles");
  Artifact.DerivedArtifact stampFile =
  artifactFactory.getDerivedArtifact(stampName, middlemanDir, actionRegistry.getOwner());
+ MiddlemanAction middlemanAction =
+ new MiddlemanAction(owner, runfilesTree, deps, ImmutableSet.of(stampFile));
+ actionRegistry.registerAction(middlemanAction);
  return stampFile;
  }
+
 }

src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java

@@ -31,7 +31,6 @@
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue.RunfileSymlinksMode;
 import com.google.devtools.build.lib.analysis.config.RunUnder;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.packages.Type;
@@ -210,7 +209,7 @@ private static RunfilesSupport create(
 
  Artifact runfilesMiddleman =
  createRunfilesMiddleman(
- ruleContext, owningExecutable, runfiles, runfilesManifest, repoMappingManifest);
+ ruleContext, owningExecutable, runfilesTree, runfilesManifest, repoMappingManifest);
 
  return new RunfilesSupport(
  runfilesTree,
@@ -346,26 +345,19 @@ public Artifact getRunfilesMiddleman() {
  private static Artifact createRunfilesMiddleman(
  ActionConstructionContext context,
  Artifact owningExecutable,
- Runfiles runfiles,
+ RunfilesTree runfilesTree,
  @Nullable Artifact runfilesManifest,
  Artifact repoMappingManifest) {
- NestedSetBuilder<Artifact> deps = NestedSetBuilder.stableOrder();
- deps.addTransitive(runfiles.getAllArtifacts());
- if (runfilesManifest != null) {
- deps.add(runfilesManifest);
- }
- if (repoMappingManifest != null) {
- deps.add(repoMappingManifest);
- }
  return context
  .getAnalysisEnvironment()
  .getMiddlemanFactory()
  .createRunfilesMiddleman(
  context.getActionOwner(),
  owningExecutable,
- deps.build(),
- context.getMiddlemanDirectory(),
- "runfiles");
+ runfilesTree,
+ runfilesManifest,
+ repoMappingManifest,
+ context.getMiddlemanDirectory());
  }
 
  /**