From b8f751b305f7e373335a087d2cca4f9831dafa25 Mon Sep 17 00:00:00 2001 From: Greg Estren Date: Thu, 4 Aug 2016 00:19:01 +0000 Subject: Late-bound split attribute configs weren't being properly propagated to deps with dynamic configs. The old code incorrectly applied a no-op instead of hard-setting a specific config. Testing: This is a prereq piece for the main change adding dynamic split transitions. Once we have that change, standard Bazel tests over implemented late-bound split attributes (e.g. AppleBinaryRule: ":cc_toolchain") will provide proper coverage. There's no easy way to test this directly since the affected code won't really work until the dynamic split change is in. -- MOS_MIGRATED_REVID=129278253 --- .../devtools/build/lib/analysis/DependencyResolver.java | 16 ++++++++++++++-- .../build/lib/skyframe/ConfiguredTargetFunction.java | 4 ++-- 2 files changed, 16 insertions(+), 4 deletions(-) (limited to 'src') 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 ec52681412..66360f4828 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 @@ -616,11 +616,23 @@ public abstract class DependencyResolver { return; // Skip this round: this is either a loading error or unevaluated Skyframe dep. } BuildConfiguration.TransitionApplier transitionApplier = config.getTransitionApplier(); + boolean applyNullTransition = false; if (BuildConfiguration.usesNullConfiguration(toTarget)) { transitionApplier.applyTransition(Attribute.ConfigurationTransition.NULL); + applyNullTransition = true; } - Dependency dep = Iterables.getOnlyElement(transitionApplier.getDependencies(depLabel, - requiredAspects(aspect, attribute, toTarget, rule))); + + ImmutableSet aspects = requiredAspects(aspect, attribute, toTarget, rule); + Dependency dep; + if (config.useDynamicConfigurations() && !applyNullTransition) { + // Since we feed a pre-prepared configuration directly to the dep, it won't get trimmed to + // the dep's fragments. + // TODO(gregce): properly trim this configuration, too. + dep = Dependency.withConfigurationAndAspects(depLabel, config, aspects); + } else { + dep = Iterables.getOnlyElement(transitionApplier.getDependencies(depLabel, aspects)); + } + outgoingEdges.put(attribute, dep); } } 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 22955612a1..b9f839c977 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 @@ -453,8 +453,8 @@ final class ConfiguredTargetFunction implements SkyFunction { new AttributeAndLabel(depsEntry.getKey(), dep.getLabel()); if (dep.hasStaticConfiguration()) { - // Certain targets (like output files) trivially pass their configurations to their deps. - // So no need to transform them in any way. + // Certain targets (like output files and late-bound splits) trivially pass their + // configurations to their deps. So no need to transform them in any way. putOnlyEntry(trimmedDeps, attributeAndLabel, dep); continue; } else if (dep.getTransition() == Attribute.ConfigurationTransition.NULL) { -- cgit v1.2.3