diff options
author | hlopko <hlopko@google.com> | 2017-07-24 12:37:02 +0200 |
---|---|---|
committer | Jakob Buchgraber <buchgr@google.com> | 2017-07-24 13:19:00 +0200 |
commit | 65c0872bdf451992fe2b62c2e308b5cc548212f5 (patch) | |
tree | 5c60b01cd941ec5f69aceea7c63b84fea19383b9 /src/main/java/com/google/devtools/build/lib | |
parent | 83c267f4867b6f0b629f59d3e823c06ac5339a40 (diff) |
Handle multiple crosstool features providing the same symbol cleanly
It's bad to crash with an exception, let's show proper rule error instead.
RELNOTES: None.
PiperOrigin-RevId: 162916138
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
3 files changed, 63 insertions, 36 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java index 05d5258bcb..d3e51a418b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java @@ -36,6 +36,7 @@ import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.rules.MakeVariableProvider; import com.google.devtools.build.lib.rules.apple.ApplePlatform; import com.google.devtools.build.lib.rules.cpp.CcLibraryHelper.SourceCategory; +import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.CollidingProvidesException; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode; @@ -612,22 +613,27 @@ public final class CcCommon { FeatureSpecification currentFeatureSpecification = FeatureSpecification.create(requestedFeatures.build(), unsupportedFeatures); - FeatureConfiguration configuration = - features.getFeatureConfiguration(currentFeatureSpecification); - for (String feature : unsupportedFeatures) { - if (configuration.isEnabled(feature)) { - ruleContext.ruleError( - "The C++ toolchain '" - + ruleContext - .getPrerequisite(CcToolchain.CC_TOOLCHAIN_DEFAULT_ATTRIBUTE_NAME, Mode.TARGET) - .getLabel() - + "' unconditionally implies feature '" - + feature - + "', which is unsupported by this rule. " - + "This is most likely a misconfiguration in the C++ toolchain."); + try { + FeatureConfiguration configuration = + features.getFeatureConfiguration(currentFeatureSpecification); + for (String feature : unsupportedFeatures) { + if (configuration.isEnabled(feature)) { + ruleContext.ruleError( + "The C++ toolchain '" + + ruleContext + .getPrerequisite(CcToolchain.CC_TOOLCHAIN_DEFAULT_ATTRIBUTE_NAME, Mode.TARGET) + .getLabel() + + "' unconditionally implies feature '" + + feature + + "', which is unsupported by this rule. " + + "This is most likely a misconfiguration in the C++ toolchain."); + } } + return configuration; + } catch (CollidingProvidesException e) { + ruleContext.ruleError(e.getMessage()); + return FeatureConfiguration.EMPTY; } - return configuration; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java index ec65b31da6..2cb61198bf 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java @@ -19,6 +19,7 @@ import com.google.common.base.Joiner; import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.common.base.Throwables; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; @@ -50,6 +51,7 @@ import java.util.Map; import java.util.Queue; import java.util.Set; import java.util.Stack; +import java.util.concurrent.ExecutionException; /** * Provides access to features supported by a specific toolchain. @@ -82,10 +84,8 @@ public class CcToolchainFeatures implements Serializable { } } - /** - * Thrown when multiple features provide the same string symbol. - */ - public static class CollidingProvidesException extends RuntimeException { + /** Thrown when multiple features provide the same string symbol. */ + public static class CollidingProvidesException extends Exception { CollidingProvidesException(String message) { super(message); } @@ -1645,12 +1645,19 @@ public class CcToolchainFeatures implements Serializable { private final ImmutableMap<String, ActionConfig> actionConfigByActionName; - public FeatureConfiguration() { + /** + * {@link FeatureConfiguration} instance that doesn't produce any command lines. This is to be + * used when creation of the real {@link FeatureConfiguration} failed, the rule error was + * reported, but the analysis continues to collect more rule errors. + */ + public static final FeatureConfiguration EMPTY = new FeatureConfiguration(); + + protected FeatureConfiguration() { this( FeatureSpecification.EMPTY, - ImmutableList.<Feature>of(), - ImmutableList.<ActionConfig>of(), - ImmutableMap.<String, ActionConfig>of()); + ImmutableList.of(), + ImmutableList.of(), + ImmutableMap.of()); } private FeatureConfiguration( @@ -1949,7 +1956,8 @@ public class CcToolchainFeatures implements Serializable { .build( new CacheLoader<FeatureSpecification, FeatureConfiguration>() { @Override - public FeatureConfiguration load(FeatureSpecification featureSpecification) { + public FeatureConfiguration load(FeatureSpecification featureSpecification) + throws CollidingProvidesException { return computeFeatureConfiguration(featureSpecification); } }); @@ -1965,8 +1973,15 @@ public class CcToolchainFeatures implements Serializable { * <p>Additional features will be enabled if the toolchain supports them and they are implied by * requested features. */ - public FeatureConfiguration getFeatureConfiguration(FeatureSpecification featureSpecification) { - return configurationCache.getUnchecked(featureSpecification); + public FeatureConfiguration getFeatureConfiguration(FeatureSpecification featureSpecification) + throws CollidingProvidesException { + try { + return configurationCache.get(featureSpecification); + } catch (ExecutionException e) { + Throwables.throwIfInstanceOf(e.getCause(), CollidingProvidesException.class); + Throwables.throwIfUnchecked(e.getCause()); + throw new IllegalStateException("Unexpected checked exception encountered", e); + } } /** @@ -1979,8 +1994,8 @@ public class CcToolchainFeatures implements Serializable { * <p>Additional features will be enabled if the toolchain supports them and they are implied by * requested features. */ - public FeatureConfiguration computeFeatureConfiguration( - FeatureSpecification featureSpecification) { + public FeatureConfiguration computeFeatureConfiguration(FeatureSpecification featureSpecification) + throws CollidingProvidesException { // Command line flags will be output in the order in which they are specified in the toolchain // configuration. return new FeatureSelection(featureSpecification).run(); @@ -2084,10 +2099,10 @@ public class CcToolchainFeatures implements Serializable { } /** - * @return a {@code FeatureConfiguration} that reflects the set of activated features and - * action configs. + * @return a {@code FeatureConfiguration} that reflects the set of activated features and action + * configs. */ - private FeatureConfiguration run() { + private FeatureConfiguration run() throws CollidingProvidesException { for (CrosstoolSelectable selectable : requestedSelectables) { enableAllImpliedBy(selectable); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java index ff1d068a54..6541177500 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java @@ -45,6 +45,7 @@ import com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions.AppleBi import com.google.devtools.build.lib.rules.apple.AppleConfiguration; import com.google.devtools.build.lib.rules.cpp.CcLibraryHelper; import com.google.devtools.build.lib.rules.cpp.CcLibraryHelper.Info; +import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.CollidingProvidesException; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariablesExtension; import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider; @@ -524,12 +525,17 @@ public class CrosstoolCompilationSupport extends CompilationSupport { } activatedCrosstoolSelectables.addAll(ruleContext.getFeatures()); - return configuration - .getFragment(CppConfiguration.class) - .getFeatures() - .getFeatureConfiguration( - FeatureSpecification.create( - activatedCrosstoolSelectables.build(), ImmutableSet.<String>of())); + try { + return configuration + .getFragment(CppConfiguration.class) + .getFeatures() + .getFeatureConfiguration( + FeatureSpecification.create( + activatedCrosstoolSelectables.build(), ImmutableSet.<String>of())); + } catch (CollidingProvidesException e) { + ruleContext.ruleError(e.getMessage()); + return FeatureConfiguration.EMPTY; + } } private static ImmutableList<Artifact> getObjFiles( |