diff --git a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java index 8e38796cba86c1..2c13cb346f226d 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java +++ b/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 inputs, - ImmutableSet outputs, - String description, - MiddlemanType middlemanType) { + ImmutableSet 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 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 inputs, - Artifact stampFile, - String purpose, - MiddlemanType middlemanType) { - MiddlemanAction action = - new MiddlemanAction(owner, inputs, ImmutableSet.of(stampFile), purpose, middlemanType); - env.registerAction(action); - return action; - } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java index c903b80f67c334..c56487c188b856 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanFactory.java +++ b/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 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 depsBuilder = NestedSetBuilder.stableOrder(); + depsBuilder.addTransitive(runfilesTree.getArtifacts()); + if (runfilesManifest != null) { + depsBuilder.add(runfilesManifest); + } + if (repoMappingManifest != null) { + depsBuilder.add(repoMappingManifest); + } + + NestedSet 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. - * - *

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 createMiddleman( - ActionOwner owner, - String middlemanName, - String purpose, - NestedSet 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; } + } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java index ec658ffbd9dfc5..4284c4a111c8e1 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java +++ b/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 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()); } /** diff --git a/src/test/java/com/google/devtools/build/lib/exec/MiddlemanActionTest.java b/src/test/java/com/google/devtools/build/lib/exec/MiddlemanActionTest.java deleted file mode 100644 index 711ea8a7bcfce4..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/exec/MiddlemanActionTest.java +++ /dev/null @@ -1,112 +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.exec; - -import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth.assertWithMessage; -import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.NULL_ACTION_OWNER; - -import com.google.common.collect.Sets; -import com.google.devtools.build.lib.actions.Action; -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.MiddlemanAction; -import com.google.devtools.build.lib.actions.MiddlemanFactory; -import com.google.devtools.build.lib.analysis.util.AnalysisTestUtil; -import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; -import com.google.devtools.build.lib.cmdline.RepositoryName; -import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** - * A test for {@link MiddlemanAction}. - */ -@RunWith(JUnit4.class) -public class MiddlemanActionTest extends BuildViewTestCase { - - private AnalysisTestUtil.CollectingAnalysisEnvironment analysisEnvironment; - private MiddlemanFactory middlemanFactory; - private Artifact a; - private Artifact b; - - @Before - public final void initializeMiddleman() throws Exception { - scratch.file("a/BUILD", - "testing_dummy_rule(name='a', outs=['a.out'])"); - scratch.file("b/BUILD", - "testing_dummy_rule(name='b', outs=['b.out'])"); - a = getFilesToBuild(getConfiguredTarget("//a")).toList().get(0); - b = getFilesToBuild(getConfiguredTarget("//b")).toList().get(0); - analysisEnvironment = - new AnalysisTestUtil.CollectingAnalysisEnvironment( - AnalysisTestUtil.STUB_ANALYSIS_ENVIRONMENT); - middlemanFactory = new MiddlemanFactory(view.getArtifactFactory(), analysisEnvironment); - } - - @Test - public void testActionIsAMiddleman() throws Exception { - Artifact middle = - middlemanFactory.createRunfilesMiddleman( - NULL_ACTION_OWNER, - null, - NestedSetBuilder.stableOrder().add(a).add(b).build(), - targetConfig.getMiddlemanDirectory(RepositoryName.MAIN), - "runfiles"); - analysisEnvironment.registerWith(getMutableActionGraph()); - Action middleman = getGeneratingAction(middle); - - assertWithMessage("Encountered instance of " + middleman.getClass()) - .that(middleman.getActionType().isMiddleman()) - .isTrue(); - assertThat(middleman.getInputs().toList()).containsExactly(a, b); - assertThat(middleman.getOutputs()).containsExactly(middle); - } - - @Test - public void testDifferentExecutablesForRunfilesMiddleman() throws Exception { - scratch.file("c/BUILD", - "testing_dummy_rule(name='c', outs=['c.out', 'd.out', 'common.out'])"); - - Artifact c = getFilesToBuild(getConfiguredTarget("//c:c.out")).toList().get(0); - Artifact d = getFilesToBuild(getConfiguredTarget("//c:d.out")).toList().get(0); - Artifact common = getFilesToBuild(getConfiguredTarget("//c:common.out")).toList().get(0); - - analysisEnvironment.clear(); - Artifact middlemanForC = - middlemanFactory.createRunfilesMiddleman( - NULL_ACTION_OWNER, - c, - NestedSetBuilder.stableOrder().add(c).add(common).build(), - targetConfig.getMiddlemanDirectory(RepositoryName.MAIN), - "runfiles"); - Artifact middlemanForD = - middlemanFactory.createRunfilesMiddleman( - NULL_ACTION_OWNER, - d, - NestedSetBuilder.stableOrder().add(d).add(common).build(), - targetConfig.getMiddlemanDirectory(RepositoryName.MAIN), - "runfiles"); - analysisEnvironment.registerWith(getMutableActionGraph()); - - MiddlemanAction middlemanActionForC = (MiddlemanAction) getGeneratingAction(middlemanForC); - MiddlemanAction middlemanActionForD = (MiddlemanAction) getGeneratingAction(middlemanForD); - - assertThat(Sets.newHashSet(middlemanActionForD.getInputs())) - .isNotEqualTo(Sets.newHashSet(middlemanActionForC.getInputs())); - assertThat(Sets.newHashSet(middlemanActionForD.getOutputs())) - .isNotEqualTo(Sets.newHashSet(middlemanActionForC.getOutputs())); - } -}