Remove support for late-bound split attributes. RELNOTES: None. PiperOrigin-RevId: 227691308
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java index 6062d06..330d145 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
@@ -23,14 +23,11 @@ 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.ConfigMatchingProvider; -import com.google.devtools.build.lib.analysis.config.FragmentClassSet; import com.google.devtools.build.lib.analysis.config.HostTransition; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.TransitionResolver; import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition; import com.google.devtools.build.lib.analysis.config.transitions.NullTransition; import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; -import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition; import com.google.devtools.build.lib.causes.Cause; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -95,8 +92,6 @@ * @param aspect the aspect applied to this target (if any) * @param configConditions resolver for config_setting labels * @param toolchainLabels required toolchain labels - * @param defaultBuildOptions default build options provided to the server to use for creating - * diffs during SkyKey construction * @param trimmingTransitionFactory the transition factory used to trim rules (note: this is a * temporary feature; see the corresponding methods in ConfiguredRuleClassProvider) * @return a mapping of each attribute in this rule or aspects to its dependent nodes @@ -107,10 +102,8 @@ @Nullable Aspect aspect, ImmutableMap<Label, ConfigMatchingProvider> configConditions, ImmutableSet<Label> toolchainLabels, - BuildOptions defaultBuildOptions, @Nullable RuleTransitionFactory trimmingTransitionFactory) - throws EvalException, InvalidConfigurationException, InterruptedException, - InconsistentAspectOrderException { + throws EvalException, InterruptedException, InconsistentAspectOrderException { NestedSetBuilder<Cause> rootCauses = NestedSetBuilder.stableOrder(); OrderedSetMultimap<Attribute, Dependency> outgoingEdges = dependentNodeMap( @@ -120,7 +113,6 @@ configConditions, toolchainLabels, rootCauses, - defaultBuildOptions, trimmingTransitionFactory); if (!rootCauses.isEmpty()) { throw new IllegalStateException(rootCauses.build().iterator().next().toString()); @@ -157,8 +149,6 @@ * @param trimmingTransitionFactory the transition factory used to trim rules (note: this is a * temporary feature; see the corresponding methods in ConfiguredRuleClassProvider) * @param rootCauses collector for dep labels that can't be (loading phase) loaded - * @param defaultBuildOptions default build options provided by the server to use for creating - * diffs during SkyKey construction * @return a mapping of each attribute in this rule or aspects to its dependent nodes */ public final OrderedSetMultimap<Attribute, Dependency> dependentNodeMap( @@ -168,10 +158,8 @@ ImmutableMap<Label, ConfigMatchingProvider> configConditions, ImmutableSet<Label> toolchainLabels, NestedSetBuilder<Cause> rootCauses, - BuildOptions defaultBuildOptions, @Nullable RuleTransitionFactory trimmingTransitionFactory) - throws EvalException, InvalidConfigurationException, InterruptedException, - InconsistentAspectOrderException { + throws EvalException, InterruptedException, InconsistentAspectOrderException { Target target = node.getTarget(); BuildConfiguration config = node.getConfiguration(); OrderedSetMultimap<Attribute, Dependency> outgoingEdges = OrderedSetMultimap.create(); @@ -193,7 +181,6 @@ toolchainLabels, rootCauses, outgoingEdges, - defaultBuildOptions, trimmingTransitionFactory); } else if (target instanceof PackageGroup) { visitPackageGroup(node, (PackageGroup) target, rootCauses, outgoingEdges.get(null)); @@ -212,10 +199,8 @@ ImmutableSet<Label> toolchainLabels, NestedSetBuilder<Cause> rootCauses, OrderedSetMultimap<Attribute, Dependency> outgoingEdges, - BuildOptions defaultBuildOptions, @Nullable RuleTransitionFactory trimmingTransitionFactory) - throws EvalException, InvalidConfigurationException, InconsistentAspectOrderException, - InterruptedException { + throws EvalException, InconsistentAspectOrderException, InterruptedException { Preconditions.checkArgument(node.getTarget() instanceof Rule, node); BuildConfiguration ruleConfig = Preconditions.checkNotNull(node.getConfiguration(), node); Rule rule = (Rule) node.getTarget(); @@ -234,7 +219,7 @@ visitTargetVisibility(node, rootCauses, outgoingEdges.get(null)); resolveEarlyBoundAttributes(depResolver); - resolveLateBoundAttributes(depResolver, ruleConfig, hostConfig, defaultBuildOptions); + resolveLateBoundAttributes(depResolver, ruleConfig, hostConfig); Attribute toolchainsAttribute = attributeMap.getAttributeDefinition(PlatformSemantics.RESOLVED_TOOLCHAINS_ATTR); @@ -242,11 +227,11 @@ } /** - * Resolves the dependencies for all attributes in this rule except late-bound attributes - * (which require special processing: see {@link #resolveLateBoundAttributes}). + * Resolves the dependencies for all attributes in this rule except late-bound attributes (which + * require special processing: see {@link #resolveLateBoundAttributes}). */ private void resolveEarlyBoundAttributes(RuleResolver depResolver) - throws EvalException, InterruptedException, InconsistentAspectOrderException { + throws InterruptedException, InconsistentAspectOrderException { Rule rule = depResolver.rule; resolveExplicitAttributes(depResolver); @@ -411,16 +396,10 @@ * @param depResolver the resolver for this rule's deps * @param ruleConfig the rule's configuration * @param hostConfig the equivalent host configuration - * @param defaultBuildOptions default build options provided by the server to use for creating - * diffs during SkyKey construction */ private void resolveLateBoundAttributes( - RuleResolver depResolver, - BuildConfiguration ruleConfig, - BuildConfiguration hostConfig, - BuildOptions defaultBuildOptions) - throws EvalException, InvalidConfigurationException, InconsistentAspectOrderException, - InterruptedException { + RuleResolver depResolver, BuildConfiguration ruleConfig, BuildConfiguration hostConfig) + throws EvalException, InconsistentAspectOrderException, InterruptedException { ConfiguredAttributeMapper attributeMap = depResolver.attributeMap; Set<Label> labelsToFetch = new HashSet<>(); Map<Label, Target> targetLookupResult = null; @@ -432,66 +411,25 @@ } LateBoundDefault<?, ?> lateBoundDefault = attribute.getLateBoundDefault(); + // This is also verified as an assertion in ImmutableAttributeFactory#build() and signaled + // as a regular error instead of an assertion in SkylarkAttr#createAttribute() but better + // have two precondition checks than zero + Preconditions.checkState(!attribute.hasSplitConfigurationTransition()); - boolean hasSplitTransition = false; - List<BuildOptions> splitOptions = null; - if (attribute.hasSplitConfigurationTransition()) { - splitOptions = attribute.getSplitTransition(attributeMap).split(ruleConfig.getOptions()); - hasSplitTransition = !SplitTransition.equals(ruleConfig.getOptions(), splitOptions); - } - - if (hasSplitTransition && !ruleConfig.isHostConfiguration()) { - // Late-bound attribute with a split transition: - // Since we want to get the same results as TransitionResolver.evaluateTransition (but - // skip it since we've already applied the split), we want to make sure this logic - // doesn't do anything differently. TransitionResolver.evaluateTransition has additional - // logic for host configs. So when we're in the host configuration we fall back to the - // non-split branch, which calls TransitionResolver.evaluateTransition, which returns its - // "host mode" result without ever looking at the split. - Iterable<BuildConfiguration> splitConfigs = - getConfigurations(ruleConfig.fragmentClasses(), splitOptions, defaultBuildOptions); - if (splitConfigs == null) { - Preconditions.checkState(collectingLabels, attributeAndOwner); - continue; // Need Skyframe deps. - } - for (BuildConfiguration splitConfig : splitConfigs) { - for (Label dep : - resolveLateBoundAttribute( - depResolver.rule, - attribute, - lateBoundDefault.useHostConfiguration() ? hostConfig : splitConfig, - attributeMap)) { - if (collectingLabels) { - labelsToFetch.add(dep); - } else { - // Skip the normal config transition pipeline and directly feed the split config. - // This is because the split already had to be applied to determine the attribute's - // value. This makes the split logic in the normal pipeline redundant and - // potentially incorrect. - Target target = targetLookupResult.get(dep); - if (target != null) { - depResolver.registerEdge(attributeAndOwner, target, splitConfig); - } - } - } - } + List<Label> deps = + resolveLateBoundAttribute( + depResolver.rule, + attribute, + lateBoundDefault.useHostConfiguration() ? hostConfig : ruleConfig, + attributeMap); + if (collectingLabels) { + labelsToFetch.addAll(deps); } else { - List<Label> deps = - resolveLateBoundAttribute( - depResolver.rule, - attribute, - lateBoundDefault.useHostConfiguration() ? hostConfig : ruleConfig, - attributeMap); - if (collectingLabels) { - labelsToFetch.addAll(deps); - } else { - // Late-bound attribute without a split transition: - for (Label dep : deps) { - Target target = targetLookupResult.get(dep); - if (target != null) { - // Process this dep like a normal attribute. - depResolver.registerEdge(attributeAndOwner, target); - } + for (Label dep : deps) { + Target target = targetLookupResult.get(dep); + if (target != null) { + // Process this dep like a normal attribute. + depResolver.registerEdge(attributeAndOwner, target); } } } @@ -847,28 +785,6 @@ toTarget.getLabel(), transition, requiredAspects(attributeAndOwner, toTarget))); } - /** - * Resolves the given dep for the given attribute using a pre-prepared configuration. - * - * <p>Use this method with care: it skips Bazel's standard config transition semantics ({@link - * TransitionResolver#evaluateTransition}). That means attributes passed through here won't obey - * standard rules on which configurations apply to their deps. This should only be done for - * special circumstances that really justify the difference. When in doubt, use {@link - * #registerEdge(AttributeAndOwner, Target)}. - */ - void registerEdge( - AttributeAndOwner attributeAndOwner, Target toTarget, BuildConfiguration config) - throws InconsistentAspectOrderException { - outgoingEdges.put( - attributeAndOwner.attribute, - TransitionResolver.usesNullConfiguration(toTarget) - ? Dependency.withNullConfiguration(toTarget.getLabel()) - : Dependency.withTransitionAndAspects( - toTarget.getLabel(), - new FixedTransition(config.getOptions()), - requiredAspects(attributeAndOwner, toTarget))); - } - private AspectCollection requiredAspects(AttributeAndOwner attributeAndOwner, final Target target) throws InconsistentAspectOrderException { if (!(target instanceof Rule)) { @@ -981,24 +897,6 @@ NestedSetBuilder<Cause> rootCauses, int labelsSizeHint) throws InterruptedException; - /** - * Returns the build configurations with the given fragments and {@link - * BuildOptions.OptionsDiffForReconstruction} resulting from calling {@link - * BuildOptions#diffForReconstruction} between the {@code defaultBuildOptions} and the provided - * {@code buildOptions}. Results will be returned in the order the {@code buildOptions} are - * provided. - * - * <p>Returns null if any configurations aren't ready to be returned at this moment. If - * getConfigurations returns null once or more during a {@link #dependentNodeMap} call, the - * results of that call will be incomplete. For use within Skyframe, where several iterations may - * be needed to discover all dependencies. - */ - @Nullable - protected abstract List<BuildConfiguration> getConfigurations( - FragmentClassSet fragments, - Iterable<BuildOptions> buildOptions, - BuildOptions defaultBuildOptions) - throws InvalidConfigurationException, InterruptedException; /** * Signals an inconsistency on aspect path: an aspect occurs twice on the path and
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java index 0399edc..4d0d865 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
@@ -442,6 +442,7 @@ Preconditions.checkState(!name.isEmpty(), "name has not been set"); if (valueSource == AttributeValueSource.LATE_BOUND) { Preconditions.checkState(isLateBound(name)); + Preconditions.checkState(splitTransitionProvider == null); } // TODO(bazel-team): Set the default to be no file type, then remove this check, and also // remove all allowedFileTypes() calls without parameters.
diff --git a/src/main/java/com/google/devtools/build/lib/query2/TransitionsOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/TransitionsOutputFormatterCallback.java index dda2be4..f321dc7 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/TransitionsOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/TransitionsOutputFormatterCallback.java
@@ -13,14 +13,10 @@ // limitations under the License. package com.google.devtools.build.lib.query2; -import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.common.collect.Streams; -import com.google.devtools.build.lib.analysis.AspectCollection; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.Dependency; import com.google.devtools.build.lib.analysis.DependencyResolver; @@ -31,8 +27,6 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.BuildOptions.OptionsDiff; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; -import com.google.devtools.build.lib.analysis.config.FragmentClassSet; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition; import com.google.devtools.build.lib.analysis.config.transitions.NoTransition; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget; @@ -125,7 +119,6 @@ OrderedSetMultimap<Attribute, Dependency> deps; ImmutableMap<Label, ConfigMatchingProvider> configConditions = ((RuleConfiguredTarget) configuredTarget).getConfigConditions(); - BuildOptions fromOptions = config.getOptions(); try { // Note: Being able to pull the $resolved_toolchain_internal attr unconditionally from the // mapper relies on the fact that {@link PlatformSemantics.RESOLVED_TOOLCHAINS_ATTR} exists @@ -133,7 +126,7 @@ // DependencyResolver but passing to avoid passing a null and since we have the information // anyway. deps = - new FormatterDependencyResolver(configuredTarget, eventHandler) + new FormatterDependencyResolver(eventHandler) .dependentNodeMap( new TargetAndConfiguration(target, config), hostConfiguration, @@ -142,9 +135,8 @@ ImmutableSet.copyOf( ConfiguredAttributeMapper.of(target.getAssociatedRule(), configConditions) .get(PlatformSemantics.RESOLVED_TOOLCHAINS_ATTR, BuildType.LABEL_LIST)), - fromOptions, trimmingTransitionFactory); - } catch (EvalException | InvalidConfigurationException | InconsistentAspectOrderException e) { + } catch (EvalException | InconsistentAspectOrderException e) { throw new InterruptedException(e.getMessage()); } for (Map.Entry<Attribute, Dependency> attributeAndDep : deps.entries()) { @@ -153,6 +145,7 @@ continue; } Dependency dep = attributeAndDep.getValue(); + BuildOptions fromOptions = config.getOptions(); List<BuildOptions> toOptions = dep.getTransition().apply(fromOptions); String hostConfigurationChecksum = hostConfiguration.checksum(); addResult( @@ -203,20 +196,12 @@ } private class FormatterDependencyResolver extends DependencyResolver { - - private ConfiguredTarget ct; private final ExtendedEventHandler eventHandler; - private FormatterDependencyResolver(ConfiguredTarget ct, ExtendedEventHandler eventHandler) { - this.ct = ct; + private FormatterDependencyResolver(ExtendedEventHandler eventHandler) { this.eventHandler = eventHandler; } - protected FormatterDependencyResolver setCt(ConfiguredTarget ct) { - this.ct = ct; - return this; - } - @Override protected void invalidVisibilityReferenceHook(TargetAndConfiguration node, Label label) { eventHandler.handle( @@ -253,30 +238,6 @@ .filter(partialResultMap::containsKey) .collect(Collectors.toMap(Function.identity(), partialResultMap::get)); } - - @Override - protected List<BuildConfiguration> getConfigurations( - FragmentClassSet fragments, - Iterable<BuildOptions> buildOptions, - BuildOptions defaultOptions) { - Preconditions.checkArgument( - ct.getConfigurationKey().getFragments().equals(fragments.fragmentClasses()), - "Mismatch: %s %s", - ct, - fragments); - Dependency asDep = - Dependency.withTransitionAndAspects( - ct.getLabel(), NoTransition.INSTANCE, AspectCollection.EMPTY); - ImmutableList.Builder<BuildConfiguration> builder = ImmutableList.builder(); - for (BuildOptions options : buildOptions) { - builder.add( - Iterables.getOnlyElement( - skyframeExecutor - .getConfigurations(eventHandler, options, ImmutableList.<Dependency>of(asDep)) - .values())); - } - return builder.build(); - } } }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java index 87763a2..6f2d6b5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -471,7 +471,6 @@ configConditions, toolchainLabels, transitiveRootCauses, - defaultBuildOptions, ((ConfiguredRuleClassProvider) ruleClassProvider).getTrimmingTransitionFactory()); } catch (EvalException e) { // EvalException can only be thrown by computed Skylark attributes in the current rule. @@ -479,8 +478,6 @@ throw new DependencyEvaluationException( new ConfiguredValueCreationException( e.print(), ctgValue.getLabel(), ctgValue.getConfiguration())); - } catch (InvalidConfigurationException e) { - throw new DependencyEvaluationException(e); } catch (InconsistentAspectOrderException e) { env.getListener().handle(Event.error(e.getLocation(), e.getMessage())); throw new DependencyEvaluationException(e);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java index 9e05215..20898fd 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
@@ -28,7 +28,6 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.analysis.config.ConfigurationResolver; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.BuildType; @@ -126,15 +125,15 @@ /*aspect=*/ null, configConditions, /*toolchainLabels=*/ ImmutableSet.of(), - defaultBuildOptions, ((ConfiguredRuleClassProvider) ruleClassProvider).getTrimmingTransitionFactory()); if (configuredTargetAndData.getConfiguration() != null) { deps = ConfigurationResolver.resolveConfigurations( env, ctgValue, deps, hostConfiguration, ruleClassProvider, defaultBuildOptions); } - } catch (EvalException | ConfiguredTargetFunction.DependencyEvaluationException - | InvalidConfigurationException | InconsistentAspectOrderException e) { + } catch (EvalException + | ConfiguredTargetFunction.DependencyEvaluationException + | InconsistentAspectOrderException e) { throw new PostConfiguredTargetFunctionException(e); }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java index 1e78a41..dabc649 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java
@@ -14,15 +14,10 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.devtools.build.lib.analysis.DependencyResolver; import com.google.devtools.build.lib.analysis.TargetAndConfiguration; -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.FragmentClassSet; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.causes.Cause; import com.google.devtools.build.lib.causes.LoadingFailedCause; import com.google.devtools.build.lib.cmdline.Label; @@ -37,10 +32,8 @@ import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.ValueOrException; -import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; -import java.util.List; import java.util.Map; import javax.annotation.Nullable; @@ -141,29 +134,4 @@ } return result; } - - @Nullable - @Override - protected List<BuildConfiguration> getConfigurations( - FragmentClassSet fragments, - Iterable<BuildOptions> buildOptions, - BuildOptions defaultBuildOptions) - throws InvalidConfigurationException, InterruptedException { - List<SkyKey> keys = new ArrayList<>(); - for (BuildOptions options : buildOptions) { - keys.add( - BuildConfigurationValue.key( - fragments, BuildOptions.diffForReconstruction(defaultBuildOptions, options))); - } - Map<SkyKey, ValueOrException<InvalidConfigurationException>> configValues = - env.getValuesOrThrow(keys, InvalidConfigurationException.class); - if (env.valuesMissing()) { - return null; - } - ImmutableList.Builder<BuildConfiguration> result = ImmutableList.builder(); - for (SkyKey key : keys) { - result.add(((BuildConfigurationValue) configValues.get(key).get()).getConfiguration()); - } - return result.build(); - } }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java index 3da6225..cd1a3de 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurationsForLateBoundTargetsTest.java
@@ -18,7 +18,6 @@ import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.analysis.config.BuildOptions; @@ -76,7 +75,6 @@ public void setupCustomLateBoundRules() throws Exception { ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder(); TestRuleClassProvider.addStandardRules(builder); - builder.addRuleDefinition(LateBoundSplitUtil.RULE_WITH_LATEBOUND_SPLIT_ATTR); builder.addRuleDefinition(LateBoundSplitUtil.RULE_WITH_TEST_FRAGMENT); builder.addConfigurationFragment(new LateBoundSplitUtil.FragmentLoader()); builder.addConfigurationOptions(LateBoundSplitUtil.TestOptions.class); @@ -125,49 +123,4 @@ // configuration doesn't happen to match what the patch would have done. assertThat(LateBoundSplitUtil.getOptions(getConfiguration(dep)).fooFlag).isEmpty(); } - - @Test - public void lateBoundSplitAttributeInTargetConfiguration() throws Exception { - scratch.file("foo/BUILD", - "rule_with_latebound_split(", - " name = 'foo')", - "rule_with_test_fragment(", - " name = 'latebound_dep')"); - // if the target fails to analyze, this iterable will be empty - assertThat(update("//foo:foo").getTargetsToBuild()).isNotEmpty(); - Iterable<ConfiguredTarget> deps = - SkyframeExecutorTestUtils.getExistingConfiguredTargets( - skyframeExecutor, Label.parseAbsolute("//foo:latebound_dep", ImmutableMap.of())); - assertThat(deps).hasSize(2); - assertThat( - ImmutableList.of( - LateBoundSplitUtil.getOptions(getConfiguration(Iterables.get(deps, 0))).fooFlag, - LateBoundSplitUtil.getOptions(getConfiguration(Iterables.get(deps, 1))).fooFlag)) - .containsExactly("one", "two"); - } - - @Test - public void lateBoundSplitAttributeInHostConfiguration() throws Exception { - scratch.file("foo/BUILD", - "genrule(", - " name = 'gen',", - " srcs = [],", - " outs = ['gen.out'],", - " cmd = 'echo hi > $@',", - " tools = [':foo'])", - "rule_with_latebound_split(", - " name = 'foo')", - "rule_with_test_fragment(", - " name = 'latebound_dep')"); - update("//foo:gen"); - assertThat(getConfiguredTarget("//foo:foo", getHostConfiguration())).isNotNull(); - ConfiguredTarget dep = - Iterables.getOnlyElement( - SkyframeExecutorTestUtils.getExistingConfiguredTargets( - skyframeExecutor, Label.parseAbsolute("//foo:latebound_dep", ImmutableMap.of()))); - assertThat(getConfiguration(dep)).isEqualTo(getHostConfiguration()); - // This is technically redundant, but slightly stronger in sanity checking that the host - // configuration doesn't happen to match what the split would have done. - assertThat(LateBoundSplitUtil.getOptions(getConfiguration(dep)).fooFlag).isEmpty(); - } }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java index 4ee313c..c653195 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java
@@ -19,13 +19,8 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Streams; -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.ConfigMatchingProvider; -import com.google.devtools.build.lib.analysis.config.FragmentClassSet; import com.google.devtools.build.lib.analysis.util.AnalysisTestCase; import com.google.devtools.build.lib.analysis.util.TestAspects; -import com.google.devtools.build.lib.bazel.rules.DefaultBuildOptionsForDiffing; import com.google.devtools.build.lib.causes.Cause; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -40,11 +35,9 @@ import com.google.devtools.build.lib.testutil.Suite; import com.google.devtools.build.lib.testutil.TestSpec; import com.google.devtools.build.lib.util.OrderedSetMultimap; -import java.util.List; import java.util.Map; import java.util.function.Function; import java.util.stream.Collectors; -import javax.annotation.Nullable; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -105,16 +98,6 @@ } })); } - - @Nullable - @Override - protected List<BuildConfiguration> getConfigurations( - FragmentClassSet fragments, - Iterable<BuildOptions> buildOptions, - BuildOptions defaultBuildOptions) { - throw new UnsupportedOperationException( - "this functionality is covered by analysis-phase integration tests"); - } }; } @@ -130,10 +113,8 @@ new TargetAndConfiguration(target, getTargetConfiguration()), getHostConfiguration(), aspect != null ? Aspect.forNative(aspect) : null, - ImmutableMap.<Label, ConfigMatchingProvider>of(), + ImmutableMap.of(), /*toolchainLabels=*/ ImmutableSet.of(), - DefaultBuildOptionsForDiffing.getDefaultBuildOptionsForFragments( - ruleClassProvider.getConfigurationOptions()), /*trimmingTransitionFactory=*/ null); }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java index c608526..b099c93 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/LateBoundSplitUtil.java
@@ -14,22 +14,15 @@ package com.google.devtools.build.lib.analysis; -import static com.google.devtools.build.lib.packages.Attribute.attr; -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.ConfigurationFragmentFactory; import com.google.devtools.build.lib.analysis.config.FragmentOptions; -import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition; import com.google.devtools.build.lib.analysis.util.MockRule; -import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.packages.Attribute; -import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; -import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; @@ -49,18 +42,6 @@ public String fooFlag; } - /** The split. */ - @AutoCodec @AutoCodec.VisibleForSerialization - static final SplitTransition SIMPLE_SPLIT = - (SplitTransition) - buildOptions -> { - BuildOptions split1 = buildOptions.clone(); - split1.get(TestOptions.class).fooFlag = "one"; - BuildOptions split2 = buildOptions.clone(); - split2.get(TestOptions.class).fooFlag = "two"; - return ImmutableList.of(split1, split2); - }; - /** The {@link BuildConfiguration.Fragment} that contains the options. */ @AutoCodec static class TestFragment extends BuildConfiguration.Fragment { @@ -86,25 +67,6 @@ } } - /** A custom rule that applies a late-bound split attribute. */ - static final RuleDefinition RULE_WITH_LATEBOUND_SPLIT_ATTR = - (MockRule) - () -> - MockRule.define( - "rule_with_latebound_split", - (builder, env) -> { - builder - .add( - attr(":latebound_split_attr", BuildType.LABEL) - .allowedFileTypes(FileTypeSet.ANY_FILE) - .allowedRuleClasses(Attribute.ANY_RULE) - .cfg(SIMPLE_SPLIT) - .value( - Attribute.LateBoundDefault.fromConstantForTesting( - Label.parseAbsoluteUnchecked("//foo:latebound_dep")))) - .requiresConfigurationFragments(TestFragment.class); - }); - /** * A custom rule that requires {@link TestFragment}. */ @@ -118,7 +80,6 @@ static ConfiguredRuleClassProvider getRuleClassProvider() { ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder(); TestRuleClassProvider.addStandardRules(builder); - builder.addRuleDefinition(RULE_WITH_LATEBOUND_SPLIT_ATTR); builder.addRuleDefinition(RULE_WITH_TEST_FRAGMENT); builder.addConfigurationFragment(new FragmentLoader()); builder.addConfigurationOptions(TestOptions.class);
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java index 48252d7..67e1da8 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
@@ -21,7 +21,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; import com.google.common.collect.Streams; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.actions.ArtifactFactory; @@ -30,7 +29,6 @@ import com.google.devtools.build.lib.analysis.AnalysisOptions; import com.google.devtools.build.lib.analysis.AnalysisResult; import com.google.devtools.build.lib.analysis.AnalysisUtils; -import com.google.devtools.build.lib.analysis.AspectCollection; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.BuildView; import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment; @@ -52,7 +50,6 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.analysis.config.ConfigurationResolver; -import com.google.devtools.build.lib.analysis.config.FragmentClassSet; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.TransitionResolver; import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition; @@ -275,8 +272,7 @@ final ConfiguredTarget ct, BuildConfigurationCollection configurations, ImmutableSet<Label> toolchainLabels) - throws EvalException, InvalidConfigurationException, InterruptedException, - InconsistentAspectOrderException { + throws EvalException, InterruptedException, InconsistentAspectOrderException { Target target = null; try { @@ -333,29 +329,6 @@ } })); } - - @Override - protected List<BuildConfiguration> getConfigurations( - FragmentClassSet fragments, - Iterable<BuildOptions> buildOptions, - BuildOptions defaultBuildOptions) { - Preconditions.checkArgument( - fragments.fragmentClasses().equals(ct.getConfigurationKey().getFragments()), - "Mismatch: %s %s", - ct, - fragments); - Dependency asDep = Dependency.withTransitionAndAspects(ct.getLabel(), - NoTransition.INSTANCE, AspectCollection.EMPTY); - ImmutableList.Builder<BuildConfiguration> builder = ImmutableList.builder(); - for (BuildOptions options : buildOptions) { - builder.add(Iterables.getOnlyElement( - skyframeExecutor - .getConfigurations(eventHandler, options, ImmutableList.<Dependency>of(asDep)) - .values() - )); - } - return builder.build(); - } } DependencyResolver dependencyResolver = new SilentDependencyResolver(); @@ -368,7 +341,6 @@ /*aspect=*/ null, getConfigurableAttributeKeysForTesting(eventHandler, ctgNode), toolchainLabels, - skyframeExecutor.getDefaultBuildOptions(), ruleClassProvider.getTrimmingTransitionFactory()); }