Skip to content

Commit

Permalink
Allow accessing the configuration of a dependency during resolution.
Browse files Browse the repository at this point in the history
This is necessary because with trimming, the configuration that a configured
target was analyzed with may not be the configuration that it was requested
with.

Progress on #6524.

PiperOrigin-RevId: 244209432
  • Loading branch information
mstaib authored and copybara-github committed Apr 18, 2019
1 parent 402d56e commit dc242d4
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -736,12 +736,32 @@ private static Map<SkyKey, ConfiguredTargetAndData> resolveConfiguredTargetDepen
}
}
try {
BuildConfiguration depConfiguration = dep.getConfiguration();
BuildConfigurationValue.Key depKey =
depValue.getConfiguredTarget().getConfigurationKey();
// Retroactive trimming may change the configuration associated with the dependency.
// If it does, we need to get that instance.
// TODO(mstaib): doing these individually instead of doing them all at once may end up
// being wasteful use of Skyframe. Although these configurations are guaranteed to be
// in the Skyframe cache (because the dependency would have had to retrieve them to be
// created in the first place), looking them up repeatedly may be slower than just
// keeping a local cache and assigning the same configuration to all the CTs which
// need it. Profile this and see if there's a better way.
if (depKey != null && !depKey.equals(BuildConfigurationValue.key(depConfiguration))) {
if (!depConfiguration.trimConfigurationsRetroactively()) {
throw new AssertionError(
"Loading configurations mid-dependency resolution should ONLY happen when "
+ "retroactive trimming is enabled.");
}
depConfiguration =
((BuildConfigurationValue) env.getValue(depKey)).getConfiguration();
}
result.put(
key,
new ConfiguredTargetAndData(
depValue.getConfiguredTarget(),
pkgValue.getPackage().getTarget(depLabel.getName()),
dep.getConfiguration()));
depConfiguration));
} catch (NoSuchTargetException e) {
throw new IllegalStateException("Target already verified for " + dep, e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1739,15 +1739,34 @@ private ImmutableMultimap<Dependency, ConfiguredTargetAndData> getConfiguredTarg
try {
ConfiguredTarget mergedTarget =
MergedConfiguredTarget.of(configuredTarget, configuredAspects);
BuildConfigurationValue.Key configKey = mergedTarget.getConfigurationKey();
BuildConfiguration resolvedConfig = depConfig;
if (configKey == null) {
// Unfortunately, it's possible to get a configured target with a null configuration
// when depConfig is non-null, so we need to explicitly override it in that case.
resolvedConfig = null;
} else if (!configKey.equals(BuildConfigurationValue.key(depConfig))) {
// Retroactive trimming may change the configuration associated with the dependency.
// If it does, we need to get that instance.
// TODO(mstaib): doing these individually instead of doing them all at once may end
// up being wasteful use of Skyframe. Although these configurations are guaranteed
// to be in the Skyframe cache (because the dependency would have had to retrieve
// them to be created in the first place), looking them up repeatedly may be slower
// than just keeping a local cache and assigning the same configuration to all the
// CTs which need it. Profile this and see if there's a better way.
if (!depConfig.trimConfigurationsRetroactively()) {
throw new AssertionError(
"Loading configurations mid-dependency resolution should ONLY happen when "
+ "retroactive trimming is enabled.");
}
resolvedConfig = getConfiguration(eventHandler, mergedTarget.getConfigurationKey());
}
cts.put(
key,
new ConfiguredTargetAndData(
mergedTarget,
packageValue.getPackage().getTarget(configuredTarget.getLabel().getName()),
// This is terrible, but our tests' use of configurations is terrible. It's only
// by accident that getting a null-configuration ConfiguredTarget works even if
// depConfig is not null.
mergedTarget.getConfigurationKey() == null ? null : depConfig));
resolvedConfig));

} catch (DuplicateException | NoSuchTargetException e) {
throw new IllegalStateException(
Expand Down

0 comments on commit dc242d4

Please sign in to comment.