Flag to disable AggregatingMiddleman in rules.
The actual clean up of code will be done later.
RELNOTES: None
PiperOrigin-RevId: 302623425
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 2932cb3..a3412fa 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -845,6 +845,10 @@
return options.inprocessSymlinkCreation;
}
+ public boolean enableAggregatingMiddleman() {
+ return options.enableAggregatingMiddleman;
+ }
+
public boolean skipRunfilesManifests() {
return options.skipRunfilesManifests;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
index f21e2a0..abd92bc 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
@@ -826,6 +826,15 @@
help = "Whether to make source manifest actions remotable")
public boolean remotableSourceManifestActions;
+ @Option(
+ name = "experimental_enable_aggregating_middleman",
+ defaultValue = "true",
+ documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+ effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+ metadataTags = {OptionMetadataTag.EXPERIMENTAL},
+ help = "Whether to enable the use of AggregatingMiddleman in rules.")
+ public boolean enableAggregatingMiddleman;
+
/** Ways configured targets may provide the {@link BuildConfiguration.Fragment}s they require. */
public enum IncludeConfigFragmentsEnum {
// Don't offer the provider at all. This is best for most builds, which don't use this
@@ -864,6 +873,7 @@
host.cpu = hostCpu;
host.inmemoryUnusedInputsList = inmemoryUnusedInputsList;
host.includeRequiredConfigFragmentsProvider = includeRequiredConfigFragmentsProvider;
+ host.enableAggregatingMiddleman = enableAggregatingMiddleman;
// === Runfiles ===
host.buildRunfilesManifests = buildRunfilesManifests;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java
index 3205910..c0713a6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.cpp;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -43,6 +44,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import javax.annotation.Nullable;
/** Helper responsible for creating CcToolchainProvider */
public class CcToolchainProviderHelper {
@@ -106,22 +108,16 @@
TransitiveInfoCollection staticRuntimeLib = attributes.getStaticRuntimeLib();
final NestedSet<Artifact> staticRuntimeLinkInputs;
final Artifact staticRuntimeLinkMiddleman;
+ boolean enableAggregatingMiddleman = configuration.enableAggregatingMiddleman();
if (staticRuntimeLib != null) {
staticRuntimeLinkInputs = staticRuntimeLib.getProvider(FileProvider.class).getFilesToBuild();
- if (!staticRuntimeLinkInputs.isEmpty()) {
- NestedSet<Artifact> staticRuntimeLinkMiddlemanSet =
- CompilationHelper.getAggregatingMiddleman(
- ruleContext, purposePrefix + "static_runtime_link", staticRuntimeLib);
- staticRuntimeLinkMiddleman =
- staticRuntimeLinkMiddlemanSet.isEmpty()
- ? null
- : staticRuntimeLinkMiddlemanSet.getSingleton();
- } else {
- staticRuntimeLinkMiddleman = null;
- }
+ staticRuntimeLinkMiddleman =
+ getStaticRuntimeLinkMiddleman(
+ ruleContext, purposePrefix, staticRuntimeLib, staticRuntimeLinkInputs);
Preconditions.checkState(
- (staticRuntimeLinkMiddleman == null) == staticRuntimeLinkInputs.isEmpty());
+ (staticRuntimeLinkMiddleman == null)
+ == (staticRuntimeLinkInputs.isEmpty() || !enableAggregatingMiddleman));
} else {
staticRuntimeLinkInputs = null;
staticRuntimeLinkMiddleman = null;
@@ -153,26 +149,19 @@
dynamicRuntimeLinkSymlinks = null;
}
- if (!dynamicRuntimeLinkInputs.isEmpty()) {
- List<Artifact> dynamicRuntimeLinkMiddlemanSet =
- CppHelper.getAggregatingMiddlemanForCppRuntimes(
- ruleContext,
- purposePrefix + "dynamic_runtime_link",
- dynamicRuntimeLinkInputs.build(),
- solibDirectory,
- runtimeSolibDirBase,
- configuration);
- dynamicRuntimeLinkMiddleman =
- dynamicRuntimeLinkMiddlemanSet.isEmpty()
- ? null
- : Iterables.getOnlyElement(dynamicRuntimeLinkMiddlemanSet);
- } else {
- dynamicRuntimeLinkMiddleman = null;
- }
+ dynamicRuntimeLinkMiddleman =
+ getDynamicRuntimeLinkMiddleman(
+ ruleContext,
+ purposePrefix,
+ runtimeSolibDirBase,
+ solibDirectory,
+ dynamicRuntimeLinkInputs);
Preconditions.checkState(
(dynamicRuntimeLinkMiddleman == null)
- == (dynamicRuntimeLinkSymlinks == null || dynamicRuntimeLinkSymlinks.isEmpty()));
+ == (dynamicRuntimeLinkSymlinks == null
+ || dynamicRuntimeLinkSymlinks.isEmpty()
+ || !enableAggregatingMiddleman));
CcCompilationContext.Builder ccCompilationContextBuilder =
CcCompilationContext.builder(
@@ -258,6 +247,55 @@
computeLegacyCcFlagsMakeVariable(toolchainConfigInfo));
}
+ @Nullable
+ @VisibleForTesting
+ static Artifact getDynamicRuntimeLinkMiddleman(
+ RuleContext ruleContext,
+ String purposePrefix,
+ String runtimeSolibDirBase,
+ String solibDirectory,
+ NestedSetBuilder<Artifact> dynamicRuntimeLinkInputs) {
+ BuildConfiguration configuration = Preconditions.checkNotNull(ruleContext.getConfiguration());
+ boolean enableAggregatingMiddleman = configuration.enableAggregatingMiddleman();
+
+ if (dynamicRuntimeLinkInputs.isEmpty() || !enableAggregatingMiddleman) {
+ return null;
+ }
+ List<Artifact> dynamicRuntimeLinkMiddlemanSet =
+ CppHelper.getAggregatingMiddlemanForCppRuntimes(
+ ruleContext,
+ purposePrefix + "dynamic_runtime_link",
+ dynamicRuntimeLinkInputs.build(),
+ solibDirectory,
+ runtimeSolibDirBase,
+ configuration);
+ return dynamicRuntimeLinkMiddlemanSet.isEmpty()
+ ? null
+ : Iterables.getOnlyElement(dynamicRuntimeLinkMiddlemanSet);
+ }
+
+ @Nullable
+ @VisibleForTesting
+ static Artifact getStaticRuntimeLinkMiddleman(
+ RuleContext ruleContext,
+ String purposePrefix,
+ TransitiveInfoCollection staticRuntimeLib,
+ NestedSet<Artifact> staticRuntimeLinkInputs) {
+ boolean enableAggregatingMiddleman =
+ Preconditions.checkNotNull(ruleContext.getConfiguration()).enableAggregatingMiddleman();
+
+ if (staticRuntimeLinkInputs.isEmpty() || !enableAggregatingMiddleman) {
+ return null;
+ }
+
+ NestedSet<Artifact> staticRuntimeLinkMiddlemanSet =
+ CompilationHelper.getAggregatingMiddleman(
+ ruleContext, purposePrefix + "static_runtime_link", staticRuntimeLib);
+ return staticRuntimeLinkMiddlemanSet.isEmpty()
+ ? null
+ : staticRuntimeLinkMiddlemanSet.getSingleton();
+ }
+
/**
* Resolve the given include directory.
*
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
index cbdce17..9f90295 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -1249,7 +1249,6 @@
/** Sets the C++ runtime library inputs for the action. */
public CppLinkActionBuilder setRuntimeInputs(
ArtifactCategory runtimeType, Artifact middleman, NestedSet<Artifact> inputs) {
- Preconditions.checkArgument((middleman == null) == inputs.isEmpty());
this.toolchainLibrariesType = runtimeType;
this.runtimeMiddleman = middleman;
this.toolchainLibrariesInputs = inputs;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java b/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java
index 47d33bc..ede0114 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.rules.filegroup;
+import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.devtools.build.lib.analysis.OutputGroupInfo.INTERNAL_SUFFIX;
import com.google.devtools.build.lib.actions.Actions;
@@ -30,6 +31,7 @@
import com.google.devtools.build.lib.analysis.Runfiles;
import com.google.devtools.build.lib.analysis.RunfilesProvider;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector;
import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector.InstrumentationSpec;
@@ -55,7 +57,7 @@
public ConfiguredTarget create(RuleContext ruleContext)
throws InterruptedException, RuleErrorException, ActionConflictException {
String outputGroupName = ruleContext.attributes().get("output_group", Type.STRING);
-
+ BuildConfiguration configuration = checkNotNull(ruleContext.getConfiguration());
if (outputGroupName.endsWith(INTERNAL_SUFFIX)) {
ruleContext.throwWithAttributeError(
"output_group", String.format(ILLEGAL_OUTPUT_GROUP_ERROR, outputGroupName));
@@ -67,10 +69,6 @@
: getArtifactsForOutputGroup(
outputGroupName, ruleContext.getPrerequisites("srcs", Mode.TARGET));
- NestedSet<Artifact> middleman =
- CompilationHelper.getAggregatingMiddleman(
- ruleContext, Actions.escapeLabel(ruleContext.getLabel()), filesToBuild);
-
InstrumentedFilesInfo instrumentedFilesProvider =
InstrumentedFilesCollector.collectTransitive(
ruleContext,
@@ -78,27 +76,37 @@
new InstrumentationSpec(FileTypeSet.ANY_FILE, "srcs", "deps", "data"),
/* reportedToActualSources= */ NestedSetBuilder.create(Order.STABLE_ORDER));
- RunfilesProvider runfilesProvider = RunfilesProvider.withData(
- new Runfiles.Builder(
- ruleContext.getWorkspaceName(),
- ruleContext.getConfiguration().legacyExternalRunfiles())
- .addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES)
- .build(),
- // If you're visiting a filegroup as data, then we also visit its data as data.
- new Runfiles.Builder(
- ruleContext.getWorkspaceName(),
- ruleContext.getConfiguration().legacyExternalRunfiles())
- .addTransitiveArtifacts(filesToBuild)
- .addDataDeps(ruleContext).build());
+ RunfilesProvider runfilesProvider =
+ RunfilesProvider.withData(
+ new Runfiles.Builder(
+ ruleContext.getWorkspaceName(), configuration.legacyExternalRunfiles())
+ .addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES)
+ .build(),
+ // If you're visiting a filegroup as data, then we also visit its data as data.
+ new Runfiles.Builder(
+ ruleContext.getWorkspaceName(), configuration.legacyExternalRunfiles())
+ .addTransitiveArtifacts(filesToBuild)
+ .addDataDeps(ruleContext)
+ .build());
- return new RuleConfiguredTargetBuilder(ruleContext)
- .add(RunfilesProvider.class, runfilesProvider)
- .setFilesToBuild(filesToBuild)
- .setRunfilesSupport(null, getExecutable(filesToBuild))
- .addNativeDeclaredProvider(instrumentedFilesProvider)
- .add(MiddlemanProvider.class, new MiddlemanProvider(middleman))
- .add(FilegroupPathProvider.class, new FilegroupPathProvider(getFilegroupPath(ruleContext)))
- .build();
+ RuleConfiguredTargetBuilder builder =
+ new RuleConfiguredTargetBuilder(ruleContext)
+ .addProvider(RunfilesProvider.class, runfilesProvider)
+ .setFilesToBuild(filesToBuild)
+ .setRunfilesSupport(null, getExecutable(filesToBuild))
+ .addNativeDeclaredProvider(instrumentedFilesProvider)
+ .addProvider(
+ FilegroupPathProvider.class,
+ new FilegroupPathProvider(getFilegroupPath(ruleContext)));
+
+ if (configuration.enableAggregatingMiddleman()) {
+ builder.addProvider(
+ MiddlemanProvider.class,
+ new MiddlemanProvider(
+ CompilationHelper.getAggregatingMiddleman(
+ ruleContext, Actions.escapeLabel(ruleContext.getLabel()), filesToBuild)));
+ }
+ return builder.build();
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java
index 6a15ac4..8fbcb1f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.rules.java;
+import static com.google.common.base.Preconditions.checkNotNull;
+
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
@@ -27,11 +29,11 @@
import com.google.devtools.build.lib.analysis.Runfiles;
import com.google.devtools.build.lib.analysis.RunfilesProvider;
import com.google.devtools.build.lib.analysis.TemplateVariableInfo;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
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.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.util.OsUtils;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -46,6 +48,7 @@
throws InterruptedException, RuleErrorException, ActionConflictException {
JavaCommon.checkRuleLoadedThroughMacro(ruleContext);
NestedSetBuilder<Artifact> filesBuilder = NestedSetBuilder.stableOrder();
+ BuildConfiguration configuration = checkNotNull(ruleContext.getConfiguration());
filesBuilder.addTransitive(PrerequisiteArtifacts.nestedSet(ruleContext, "srcs", Mode.TARGET));
boolean siblingRepositoryLayout =
ruleContext
@@ -92,8 +95,11 @@
NestedSet<Artifact> filesToBuild = filesBuilder.build();
NestedSet<Artifact> middleman =
- CompilationHelper.getAggregatingMiddleman(
- ruleContext, Actions.escapeLabel(ruleContext.getLabel()), filesToBuild);
+ configuration.enableAggregatingMiddleman()
+ ? CompilationHelper.getAggregatingMiddleman(
+ ruleContext, Actions.escapeLabel(ruleContext.getLabel()), filesToBuild)
+ : filesToBuild;
+
// TODO(cushon): clean up uses of java_runtime in data deps and remove this
Runfiles runfiles =
new Runfiles.Builder(ruleContext.getWorkspaceName())
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
index 0e4f6f5..ef73c25 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
@@ -161,6 +161,21 @@
)
java_test(
+ name = "CcToolchainProviderHelperTest",
+ srcs = ["CcToolchainProviderHelperTest.java"],
+ deps = [
+ "//src/main/java/com/google/devtools/build/lib:build-base",
+ "//src/main/java/com/google/devtools/build/lib/actions",
+ "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
+ "//src/main/java/com/google/devtools/build/lib/rules/cpp",
+ "//src/test/java/com/google/devtools/build/lib/analysis/util",
+ "//third_party:junit4",
+ "//third_party:mockito",
+ "//third_party:truth",
+ ],
+)
+
+java_test(
name = "CppSysrootTest",
srcs = ["CppSysrootTest.java"],
deps = [
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelperTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelperTest.java
new file mode 100644
index 0000000..26c07ea
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelperTest.java
@@ -0,0 +1,71 @@
+// Copyright 2020 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.rules.cpp;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import java.io.IOException;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
+
+/** Unit tests for {@code CcToolchainProviderHelper} */
+@RunWith(JUnit4.class)
+public class CcToolchainProviderHelperTest extends BuildViewTestCase {
+ @Rule public final MockitoRule mockito = MockitoJUnit.rule();
+
+ @Before
+ public void createFooFooCcLibraryForRuleContext() throws IOException {
+ scratch.file("foo/BUILD", "cc_library(name = 'foo')");
+ }
+
+ private RuleContext getRuleContext() throws Exception {
+ return getRuleContext(getConfiguredTarget("//foo:foo"));
+ }
+
+ @Test
+ public void getDynamicRuntimeLinkMiddleman_disableMiddlemanArtifacts() throws Exception {
+ useConfiguration("--noexperimental_enable_aggregating_middleman");
+ RuleContext ruleContext = getRuleContext();
+
+ NestedSetBuilder<Artifact> nonEmptyBuilder = NestedSetBuilder.stableOrder();
+ nonEmptyBuilder.add(getSourceArtifact("foo.h"));
+ Artifact middleman =
+ CcToolchainProviderHelper.getDynamicRuntimeLinkMiddleman(
+ ruleContext, "purposePrefix", "runtimeSolibDirBase", "solibDirectory", nonEmptyBuilder);
+ assertThat(middleman).isNull();
+ }
+
+ @Test
+ public void getStaticRuntimeLinkMiddleman_disableMiddlemanArtifacts() throws Exception {
+ useConfiguration("--noexperimental_enable_aggregating_middleman");
+ RuleContext ruleContext = getRuleContext();
+
+ NestedSet<Artifact> nonEmptySet =
+ NestedSetBuilder.<Artifact>stableOrder().add(getSourceArtifact("foo.h")).build();
+ Artifact middleman =
+ CcToolchainProviderHelper.getStaticRuntimeLinkMiddleman(
+ ruleContext, "purposePrefix", /* staticRuntimeLib= */ null, nonEmptySet);
+ assertThat(middleman).isNull();
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/filegroup/FilegroupConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/filegroup/FilegroupConfiguredTargetTest.java
index 44626b7..e4aa385 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/filegroup/FilegroupConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/filegroup/FilegroupConfiguredTargetTest.java
@@ -18,6 +18,7 @@
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.MiddlemanProvider;
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
import com.google.devtools.build.lib.analysis.configuredtargets.FileConfiguredTarget;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
@@ -183,4 +184,22 @@
.contains(
String.format(Filegroup.ILLEGAL_OUTPUT_GROUP_ERROR, OutputGroupInfo.HIDDEN_TOP_LEVEL));
}
+
+ @Test
+ public void create_disableMiddlemanArtifact() throws Exception {
+ useConfiguration("--noexperimental_enable_aggregating_middleman");
+ scratch.file("foo/BUILD", "filegroup(name = 'foo', srcs = ['foo/spam.txt'])");
+ ConfiguredTarget target = getConfiguredTarget("//foo:foo");
+
+ assertThat(target.getProvider(MiddlemanProvider.class)).isNull();
+ }
+
+ @Test
+ public void create_enableMiddlemanArtifact() throws Exception {
+ useConfiguration("--experimental_enable_aggregating_middleman");
+ scratch.file("foo/BUILD", "filegroup(name = 'foo', srcs = ['foo/spam.txt'])");
+ ConfiguredTarget target = getConfiguredTarget("//foo:foo");
+
+ assertThat(target.getProvider(MiddlemanProvider.class)).isNotNull();
+ }
}
