diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java index 1eb91467de20d4..5456729c20c351 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java @@ -85,6 +85,20 @@ public static class TestOptions extends FragmentOptions { ) public int testResultExpiration; + @Option( + name = "trim_test_configuration", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION, + effectTags = { + OptionEffectTag.LOADING_AND_ANALYSIS, + OptionEffectTag.LOSES_INCREMENTAL_STATE, + }, + help = "When enabled, test-related options will be cleared below the top level of the build. " + + "When this flag is active, tests cannot be built as dependencies of non-test rules, " + + "but changes to test-related options will not cause non-test rules to be re-analyzed." + ) + public boolean trimTestConfiguration; + @Option( name = "test_arg", allowMultiple = true, @@ -193,6 +207,9 @@ public static class Loader implements ConfigurationFragmentFactory { @Override public Fragment create(BuildOptions buildOptions) throws InvalidConfigurationException { + if (!buildOptions.contains(TestOptions.class)) { + return null; + } return new TestConfiguration(buildOptions.get(TestOptions.class)); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java new file mode 100644 index 00000000000000..ed8f625ad31011 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java @@ -0,0 +1,85 @@ +// Copyright 2018 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.test; + +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.analysis.config.BuildOptions; +import com.google.devtools.build.lib.analysis.config.FragmentOptions; +import com.google.devtools.build.lib.analysis.config.transitions.NoTransition; +import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; +import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions; +import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.packages.RuleClass; +import com.google.devtools.build.lib.packages.RuleTransitionFactory; +import com.google.devtools.common.options.Options; +import java.util.LinkedHashSet; +import java.util.Set; + +/** + * Trimming transition factory which removes the test config fragment when entering a non-test rule. + */ +public final class TestTrimmingTransitionFactory implements RuleTransitionFactory { + + private static final Set TEST_OPTIONS = + ImmutableSet.copyOf(Options.getDefaults(TestOptions.class).asMap().keySet()); + + /** + * Trimming transition which removes the test config fragment if --trim_test_configuration is on. + */ + public static enum TestTrimmingTransition implements PatchTransition { + INSTANCE; + + @Override + public BuildOptions patch(BuildOptions originalOptions) { + if (!originalOptions.contains(TestOptions.class)) { + // nothing to do, already trimmed this fragment + return originalOptions; + } + TestOptions originalTestOptions = originalOptions.get(TestOptions.class); + if (!originalTestOptions.trimTestConfiguration) { + // nothing to do, trimming is disabled + return originalOptions; + } + BuildOptions.Builder builder = BuildOptions.builder(); + for (FragmentOptions options : originalOptions.getOptions()) { + if (!(options instanceof TestOptions)) { + builder.add(options); + } + } + return builder.build(); + } + } + + @Override + public PatchTransition buildTransitionFor(Rule rule) { + RuleClass ruleClass = rule.getRuleClassObject(); + if (ruleClass + .getConfigurationFragmentPolicy() + .isLegalConfigurationFragment(TestConfiguration.class)) { + // Test rule; no need to trim here. + return NoTransition.INSTANCE; + } + + Set referencedTestOptions = + new LinkedHashSet(ruleClass.getOptionReferenceFunction().apply(rule)); + referencedTestOptions.retainAll(TEST_OPTIONS); + if (!referencedTestOptions.isEmpty()) { + // Test-option-referencing config_setting; no need to trim here. + return NoTransition.INSTANCE; + } + + // Non-test rule. Trim it! + return TestTrimmingTransition.INSTANCE; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/core/CoreRules.java b/src/main/java/com/google/devtools/build/lib/rules/core/CoreRules.java index 90d44a5bd6cc27..a89b3c5f1da673 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/core/CoreRules.java +++ b/src/main/java/com/google/devtools/build/lib/rules/core/CoreRules.java @@ -18,6 +18,7 @@ import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider.RuleSet; import com.google.devtools.build.lib.analysis.test.TestConfiguration; +import com.google.devtools.build.lib.analysis.test.TestTrimmingTransitionFactory; /** A set of basic rules - Bazel won't work correctly without these. */ public final class CoreRules implements RuleSet { @@ -30,6 +31,7 @@ private CoreRules() { @Override public void init(ConfiguredRuleClassProvider.Builder builder) { builder.addConfig(TestConfiguration.TestOptions.class, new TestConfiguration.Loader()); + builder.addTrimmingTransitionFactory(new TestTrimmingTransitionFactory()); builder.addRuleDefinition(new BaseRuleClasses.RootRule()); builder.addRuleDefinition(new BaseRuleClasses.BaseRule()); builder.addRuleDefinition(new BaseRuleClasses.RuleBase()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestSuiteRule.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestSuiteRule.java index 20e1d0fc770a74..45ed77bf730240 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/TestSuiteRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestSuiteRule.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; +import com.google.devtools.build.lib.analysis.test.TestConfiguration; import com.google.devtools.build.lib.packages.RuleClass; /** Rule object implementing "test_suite". */ @@ -27,6 +28,9 @@ public final class TestSuiteRule implements RuleDefinition { @Override public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) { return builder + // Technically, test_suite does not use TestConfiguration. But the tests it depends on + // will always depend on TestConfiguration, so requiring it here simply acknowledges that. + .requiresConfigurationFragments(TestConfiguration.class) .override( attr("testonly", BOOLEAN) .value(true) diff --git a/src/test/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionTest.java new file mode 100644 index 00000000000000..74d8cdba85d082 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionTest.java @@ -0,0 +1,587 @@ +// Copyright 2018 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.test; + +import static com.google.common.collect.ImmutableMap.toImmutableMap; +import static com.google.common.collect.ImmutableMultiset.toImmutableMultiset; +import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.packages.Attribute.attr; +import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; +import static org.junit.Assert.fail; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableMultiset; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; +import com.google.devtools.build.lib.analysis.BaseRuleClasses; +import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; +import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.RuleDefinition; +import com.google.devtools.build.lib.analysis.Runfiles; +import com.google.devtools.build.lib.analysis.RunfilesProvider; +import com.google.devtools.build.lib.analysis.RunfilesSupport; +import com.google.devtools.build.lib.analysis.ViewCreationFailedException; +import com.google.devtools.build.lib.analysis.actions.FileWriteAction; +import com.google.devtools.build.lib.analysis.config.HostTransition; +import com.google.devtools.build.lib.analysis.util.AnalysisTestCase; +import com.google.devtools.build.lib.analysis.util.MockRule; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; +import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; +import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; +import com.google.devtools.build.skyframe.SkyKey; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for the trimming of the test configuration fragment. */ +@RunWith(JUnit4.class) +public final class TestTrimmingTransitionTest extends AnalysisTestCase { + + /** Simple native test rule. */ + public static final class NativeTest implements RuleConfiguredTargetFactory { + @Override + public ConfiguredTarget create(RuleContext context) throws ActionConflictException { + Artifact executable = context.getBinArtifact(context.getLabel().getName()); + context.registerAction(FileWriteAction.create(context, executable, "#!/bin/true", true)); + Runfiles runfiles = + new Runfiles.Builder(context.getWorkspaceName()).addArtifact(executable).build(); + return new RuleConfiguredTargetBuilder(context) + .setFilesToBuild(NestedSetBuilder.create(Order.STABLE_ORDER, executable)) + .add(RunfilesProvider.class, RunfilesProvider.simple(runfiles)) + .setRunfilesSupport( + RunfilesSupport.withExecutable(context, runfiles, executable), executable) + .build(); + } + } + + private static final RuleDefinition NATIVE_TEST_RULE = + (MockRule) + () -> + MockRule.ancestor(BaseRuleClasses.TestBaseRule.class, BaseRuleClasses.BaseRule.class) + .factory(NativeTest.class) + .type(RuleClassType.TEST) + .define( + "native_test", + attr("deps", LABEL_LIST).allowedFileTypes(), + attr("host_deps", LABEL_LIST) + .cfg(HostTransition.INSTANCE) + .allowedFileTypes()); + + private static final RuleDefinition NATIVE_LIB_RULE = + (MockRule) + () -> + MockRule.ancestor(BaseRuleClasses.BaseRule.class) + .define( + "native_lib", + attr("deps", LABEL_LIST).allowedFileTypes(), + attr("host_deps", LABEL_LIST) + .cfg(HostTransition.INSTANCE) + .allowedFileTypes()); + + @Before + public void setUp() throws Exception { + setRulesAvailableInTests(NATIVE_TEST_RULE, NATIVE_LIB_RULE); + scratch.file( + "test/test.bzl", + "def _skylark_test_impl(ctx):", + " executable = ctx.actions.declare_file(ctx.label.name)", + " ctx.actions.write(executable, '#!/bin/true', is_executable=True)", + " return DefaultInfo(", + " executable=executable,", + " )", + "skylark_test = rule(", + " implementation = _skylark_test_impl,", + " test = True,", + " executable = True,", + " attrs = {", + " 'deps': attr.label_list(),", + " 'host_deps': attr.label_list(cfg='host'),", + " },", + ")"); + scratch.file( + "test/lib.bzl", + "def _skylark_lib_impl(ctx):", + " pass", + "skylark_lib = rule(", + " implementation = _skylark_lib_impl,", + " attrs = {", + " 'deps': attr.label_list(),", + " 'host_deps': attr.label_list(cfg='host'),", + " },", + ")"); + } + + private void assertNumberOfConfigurationsOfTargets( + Set keys, Map targetsWithCounts) { + ImmutableMultiset