diff options
Diffstat (limited to 'src')
7 files changed, 305 insertions, 23 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index 5a45dc6bca..4c22da39fb 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java @@ -40,6 +40,7 @@ import com.google.devtools.build.lib.analysis.DependencyResolver.InconsistentAsp import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection; import com.google.devtools.build.lib.analysis.config.BuildOptions; +import com.google.devtools.build.lib.analysis.config.ComposingSplitTransition; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.cmdline.Label; @@ -840,12 +841,19 @@ public class BuildView { for (BuildConfiguration config : configurations.getTargetConfigurations()) { for (Target target : targets) { nodes.add(new TargetAndConfiguration(target, - BuildConfigurationCollection.configureTopLevelTarget(config, target))); + config.useDynamicConfigurations() + // Dynamic configurations apply top-level transitions through a different code path: + // BuildConfiguration#topLevelConfigurationHook. That path has the advantages of a) + // not requiring a global transitions table and b) making its choices outside core + // Bazel code. + ? (target.isConfigurable() ? config : null) + : BuildConfigurationCollection.configureTopLevelTarget(config, target))); } } - return configurations.useDynamicConfigurations() - ? getDynamicConfigurations(nodes, eventHandler) - : ImmutableList.copyOf(nodes); + return ImmutableList.copyOf( + configurations.useDynamicConfigurations() + ? getDynamicConfigurations(nodes, eventHandler) + : nodes); } /** @@ -855,7 +863,8 @@ public class BuildView { * * <p>Else returns configurations that unconditionally include all fragments. * - * <p>Preserves the original input order. Uses original (untrimmed) configurations for targets + * <p>Preserves the original input order (but merges duplicate nodes that might occur due to + * top-level configuration transitions) . Uses original (untrimmed) configurations for targets * that can't be evaluated (e.g. due to loading phase errors). * * <p>This is suitable for feeding {@link ConfiguredTargetValue} keys: as general principle {@link @@ -864,7 +873,7 @@ public class BuildView { */ // TODO(bazel-team): error out early for targets that fail - untrimmed configurations should // never make it through analysis (and especially not seed ConfiguredTargetValues) - private List<TargetAndConfiguration> getDynamicConfigurations( + private LinkedHashSet<TargetAndConfiguration> getDynamicConfigurations( Iterable<TargetAndConfiguration> inputs, ExtendedEventHandler eventHandler) throws InterruptedException { Map<Label, Target> labelsToTargets = new LinkedHashMap<>(); @@ -876,23 +885,10 @@ public class BuildView { for (TargetAndConfiguration targetAndConfig : inputs) { labelsToTargets.put(targetAndConfig.getLabel(), targetAndConfig.getTarget()); - - Attribute.Transition ruleclassTransition = null; - if (targetAndConfig.getTarget().getAssociatedRule() != null) { - Rule associatedRule = targetAndConfig.getTarget().getAssociatedRule(); - RuleTransitionFactory transitionFactory = - associatedRule.getRuleClassObject().getTransitionFactory(); - if (transitionFactory != null) { - ruleclassTransition = transitionFactory.buildTransitionFor(associatedRule); - } - } if (targetAndConfig.getConfiguration() != null) { asDeps.put(targetAndConfig.getConfiguration(), Dependency.withTransitionAndAspects( - targetAndConfig.getLabel(), - ruleclassTransition == null - ? Attribute.ConfigurationTransition.NONE - : ruleclassTransition, + targetAndConfig.getLabel(), getTopLevelTransition(targetAndConfig), // TODO(bazel-team): support top-level aspects AspectCollection.EMPTY)); } @@ -916,8 +912,7 @@ public class BuildView { } } - ImmutableList.Builder<TargetAndConfiguration> result = - ImmutableList.<TargetAndConfiguration>builder(); + LinkedHashSet<TargetAndConfiguration> result = new LinkedHashSet<>(); for (TargetAndConfiguration originalInput : inputs) { if (successfullyEvaluatedTargets.containsKey(originalInput)) { // The configuration was successfully trimmed. @@ -927,9 +922,46 @@ public class BuildView { result.add(originalInput); } } - return result.build(); + return result; + } + + /** + * Returns the transition to apply to the top-level configuration before applying it to this + * target. This enables support for rule-triggered top-level configuration hooks. + */ + private static Attribute.Transition getTopLevelTransition( + TargetAndConfiguration targetAndConfig) { + Target target = targetAndConfig.getTarget(); + BuildConfiguration fromConfig = targetAndConfig.getConfiguration(); + Preconditions.checkArgument(fromConfig.useDynamicConfigurations()); + + // Top-level transitions (chosen by configuration fragments): + Transition topLevelTransition = fromConfig.topLevelConfigurationHook(target); + if (topLevelTransition == null) { + topLevelTransition = ConfigurationTransition.NONE; + } + + // Rule class transitions (chosen by rule class definitions): + if (target.getAssociatedRule() == null) { + return topLevelTransition; + } + Rule associatedRule = target.getAssociatedRule(); + RuleTransitionFactory transitionFactory = + associatedRule.getRuleClassObject().getTransitionFactory(); + if (transitionFactory == null) { + return topLevelTransition; + } + Attribute.Transition ruleClassTransition = transitionFactory.buildTransitionFor(associatedRule); + if (ruleClassTransition == null) { + return topLevelTransition; + } else if (topLevelTransition == ConfigurationTransition.NONE) { + return ruleClassTransition; + } else { + return new ComposingSplitTransition(topLevelTransition, ruleClassTransition); + } } + /** * Gets a dynamic configuration for the given target. * 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 e942556160..8514cdd7cd 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 @@ -212,6 +212,20 @@ public final class BuildConfiguration { public PatchTransition getArtifactOwnerTransition() { return null; } + + /** + * Returns an extra transition that should apply to top-level targets in this + * configuration. Returns null if no transition is needed. + * + * <p>Overriders should not change {@link FragmentOptions} not associated with their fragment. + * + * <p>If multiple fragments specify a transition, they're composed together in a + * deterministic but undocumented order (so don't write code expecting a specific order). + */ + @Nullable + public PatchTransition topLevelConfigurationHook(Target toTarget) { + return null; + } } private static final Label convertLabel(String input) throws OptionsParsingException { @@ -2679,4 +2693,24 @@ public final class BuildConfiguration { public ImmutableCollection<String> getSkylarkFragmentNames() { return skylarkVisibleFragments.keySet(); } + + /** + * Returns an extra transition that should apply to top-level targets in this + * configuration. Returns null if no transition is needed. + */ + @Nullable + public PatchTransition topLevelConfigurationHook(Target toTarget) { + PatchTransition currentTransition = null; + for (Fragment fragment : fragments.values()) { + PatchTransition fragmentTransition = fragment.topLevelConfigurationHook(toTarget); + if (fragmentTransition == null) { + continue; + } else if (currentTransition == null) { + currentTransition = fragmentTransition; + } else { + currentTransition = new ComposingPatchTransition(currentTransition, fragmentTransition); + } + } + return currentTransition; + } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingPatchTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingPatchTransition.java new file mode 100644 index 0000000000..fb4c7e5d6c --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingPatchTransition.java @@ -0,0 +1,42 @@ +// Copyright 2017 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.config; + +import com.google.common.collect.Iterables; + +/** + * A {@link ComposingSplitTransition} that only supports {@link PatchTransition}s + * + * <p>Calling code that doesn't want to have to handle splits should prefer this version. + */ +public class ComposingPatchTransition implements PatchTransition { + private final ComposingSplitTransition delegate; + + public ComposingPatchTransition(PatchTransition transition1, PatchTransition transition2) { + this.delegate = new ComposingSplitTransition(transition1, transition2); + } + + @Override + public boolean defaultsToSelf() { + throw new UnsupportedOperationException( + "dynamic configurations don't use global transition tables"); + } + + @Override + public BuildOptions apply(BuildOptions options) { + return Iterables.getOnlyElement(delegate.split(options)); + } +} + diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index b911026c94..4388571311 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -40,10 +40,13 @@ import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.packages.OutputFile; +import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.rules.cpp.CppConfigurationLoader.CppConfigurationParameters; import com.google.devtools.build.lib.rules.cpp.CppLinkActionConfigs.CppLinkPlatform; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; import com.google.devtools.build.lib.rules.cpp.transitions.ContextCollectorOwnerTransition; +import com.google.devtools.build.lib.rules.cpp.transitions.DisableLipoTransition; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; @@ -2253,4 +2256,19 @@ public class CppConfiguration extends BuildConfiguration.Fragment { public PatchTransition getArtifactOwnerTransition() { return isLipoContextCollector() ? ContextCollectorOwnerTransition.INSTANCE : null; } + + @Nullable + @Override + public PatchTransition topLevelConfigurationHook(Target toTarget) { + // Top-level output files that aren't outputs of the LIPO context should be built in + // the data config. This is so their output path prefix doesn't have "-lipo" in it, which + // is a confusing and unnecessary deviation from how they would normally look. + if (toTarget instanceof OutputFile + && isLipoOptimization() + && !toTarget.getAssociatedRule().getLabel().equals(getLipoContextLabel())) { + return DisableLipoTransition.INSTANCE; + } else { + return null; + } + } } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java index 164e3a510e..21988605e5 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java @@ -330,6 +330,32 @@ public class BuildViewTest extends BuildViewTestBase { } @Test + public void topLevelConfigurationHook() throws Exception { + setConfigFragmentsAvailableInTests(TestConfigFragments.FragmentWithTopLevelConfigHook1Factory); + scratch.file( + "package/BUILD", + "sh_binary(name = 'binary', srcs = ['binary.sh'])"); + ConfiguredTarget ct = Iterables.getOnlyElement(update("//package:binary").getTargetsToBuild()); + BuildConfiguration.Options options = + ct.getConfiguration().getOptions().get(BuildConfiguration.Options.class); + assertThat(options.testArguments).containsExactly("CONFIG HOOK 1"); + } + + @Test + public void topLevelComposedConfigurationHooks() throws Exception { + setConfigFragmentsAvailableInTests( + TestConfigFragments.FragmentWithTopLevelConfigHook1Factory, + TestConfigFragments.FragmentWithTopLevelConfigHook2Factory); + scratch.file( + "package/BUILD", + "sh_binary(name = 'binary', srcs = ['binary.sh'])"); + ConfiguredTarget ct = Iterables.getOnlyElement(update("//package:binary").getTargetsToBuild()); + BuildConfiguration.Options options = + ct.getConfiguration().getOptions().get(BuildConfiguration.Options.class); + assertThat(options.testArguments).containsExactly("CONFIG HOOK 1", "CONFIG HOOK 2"); + } + + @Test public void testGetDirectPrerequisites() throws Exception { scratch.file( "package/BUILD", diff --git a/src/test/java/com/google/devtools/build/lib/analysis/TestConfigFragments.java b/src/test/java/com/google/devtools/build/lib/analysis/TestConfigFragments.java new file mode 100644 index 0000000000..e0b862c851 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/analysis/TestConfigFragments.java @@ -0,0 +1,113 @@ +// Copyright 2017 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.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.analysis.config.BuildOptions; +import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment; +import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; +import com.google.devtools.build.lib.analysis.config.FragmentOptions; +import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; +import com.google.devtools.build.lib.analysis.config.PatchTransition; +import com.google.devtools.build.lib.packages.Target; + +/** + * Grab bag file for test configuration fragments and fragment factories. + */ +public class TestConfigFragments { + /** + * A {@link PatchTransition} that appends a given value to + * {@link BuildConfiguration.Options#testArguments}. + */ + private static class TestArgTransition implements PatchTransition { + + private final String patchMessage; + + TestArgTransition(String patchMessage) { + this.patchMessage = patchMessage; + } + + @Override + public boolean defaultsToSelf() { + throw new UnsupportedOperationException( + "dynamic configurations don't use global transition tables"); + } + + @Override + public BuildOptions apply(BuildOptions options) { + BuildOptions toOptions = options.clone(); + BuildConfiguration.Options coreOptions = + toOptions.get(BuildConfiguration.Options.class); + coreOptions.testArguments = new ImmutableList.Builder<String>() + .addAll(coreOptions.testArguments).add(patchMessage).build(); + return toOptions; + } + } + + /** + * A {@link ConfigurationFragmentFactory} that trivially returns a given fragment. + */ + private static class SimpleFragmentFactory implements ConfigurationFragmentFactory { + private final BuildConfiguration.Fragment fragment; + + public SimpleFragmentFactory(BuildConfiguration.Fragment fragment) { + this.fragment = fragment; + } + + @Override + public BuildConfiguration.Fragment create(ConfigurationEnvironment env, + BuildOptions buildOptions) throws InvalidConfigurationException { + return fragment; + } + + @Override + public Class<? extends BuildConfiguration.Fragment> creates() { + return fragment.getClass(); + } + + @Override + public ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() { + return ImmutableSet.<Class<? extends FragmentOptions>>of(); + } + } + + /** + * Factory for a test fragment with a top-level configuration hook. + */ + public static SimpleFragmentFactory FragmentWithTopLevelConfigHook1Factory = + new SimpleFragmentFactory(new BuildConfiguration.Fragment() { + @Override + public PatchTransition topLevelConfigurationHook(Target toTarget) { + return new TestArgTransition("CONFIG HOOK 1"); + } + }); + + /** + * Factory for a test fragment with a top-level configuration hook. + */ + public static SimpleFragmentFactory FragmentWithTopLevelConfigHook2Factory = + // The anonymous class definition for the BuildConfiguration.Fragment needs to be different + // than the one of its peer above. This is because Bazel indexes configuration fragments + // by class name. So we need to make sure all fragment definitions retain distinct class + // names (i.e. "TestConfigFragments$1", "TestConfigFragments$2", etc). + new SimpleFragmentFactory(new BuildConfiguration.Fragment() { + @Override + public PatchTransition topLevelConfigurationHook(Target toTarget) { + return new TestArgTransition("CONFIG HOOK 2"); + } + }); +} diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java index a7ddfa9228..5b49663020 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java @@ -35,6 +35,7 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigurationFactory; +import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; import com.google.devtools.build.lib.buildtool.BuildRequest.BuildRequestOptions; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; @@ -164,6 +165,10 @@ public abstract class AnalysisTestCase extends FoundationTestCase { protected void useRuleClassProvider(ConfiguredRuleClassProvider ruleClassProvider) throws Exception { this.ruleClassProvider = ruleClassProvider; + useConfigurationFactory( + new ConfigurationFactory( + ruleClassProvider.getConfigurationCollectionFactory(), + ruleClassProvider.getConfigurationFragments())); PackageFactory pkgFactory = analysisMock .getPackageFactoryForTesting() @@ -504,4 +509,16 @@ public abstract class AnalysisTestCase extends FoundationTestCase { update(); } + /** + * Makes custom configuration fragments available in tests. + */ + protected final void setConfigFragmentsAvailableInTests( + ConfigurationFragmentFactory... factories) throws Exception { + ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder(); + TestRuleClassProvider.addStandardRules(builder); + for (ConfigurationFragmentFactory factory : factories) { + builder.addConfigurationFragment(factory); + } + useRuleClassProvider(builder.build()); + } } |