From e22ef9fda9c7b40e578f5c6f2564de42d6ea3bde Mon Sep 17 00:00:00 2001 From: hlopko Date: Mon, 25 Mar 2019 11:05:15 -0700 Subject: [PATCH] Do not use CppConfiguration from CcToolchainProvider for --force_pic In order to implement this correctly (without changing the existing API) we need to thread CppConfiguration passed in cc_common.configure_feature along with FeatureConfiguration to cc_toolchain.use_pic_for_binaries. Hence FeatureConfigurationForStarlark wrapper class. This workaround will be removed once #7260 is flipped. Progress towards #6516. RELNOTES: None. PiperOrigin-RevId: 240177498 --- .../lib/bazel/rules/cpp/BazelCcModule.java | 8 +-- .../build/lib/rules/cpp/CcBinary.java | 23 ++++-- .../lib/rules/cpp/CcCompilationHelper.java | 10 +-- .../build/lib/rules/cpp/CcLibrary.java | 2 +- .../build/lib/rules/cpp/CcLinkingHelper.java | 6 +- .../build/lib/rules/cpp/CcModule.java | 72 +++++++++++-------- .../lib/rules/cpp/CcToolchainFeatures.java | 3 +- .../lib/rules/cpp/CcToolchainProvider.java | 21 ++++-- .../build/lib/rules/cpp/CppConfiguration.java | 5 +- .../build/lib/rules/cpp/CppHelper.java | 10 +-- .../lib/rules/cpp/CppLinkActionBuilder.java | 3 +- .../cpp/FeatureConfigurationForStarlark.java | 58 +++++++++++++++ .../lib/rules/cpp/LinkBuildVariables.java | 2 +- .../cpp/CcToolchainProviderApi.java | 2 +- .../build/lib/rules/cpp/CcToolchainTest.java | 5 +- .../cpp/CppLinkstampCompileHelperTest.java | 5 +- 16 files changed, 164 insertions(+), 71 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/rules/cpp/FeatureConfigurationForStarlark.java diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcModule.java index 66fe2d86bb9a2a..9c26d88833081b 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcModule.java @@ -23,9 +23,9 @@ import com.google.devtools.build.lib.rules.cpp.CcLinkingHelper.LinkingInfo; import com.google.devtools.build.lib.rules.cpp.CcModule; import com.google.devtools.build.lib.rules.cpp.CcToolchainConfigInfo; -import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider; import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables; +import com.google.devtools.build.lib.rules.cpp.FeatureConfigurationForStarlark; import com.google.devtools.build.lib.rules.cpp.LibraryToLink; import com.google.devtools.build.lib.rules.cpp.LibraryToLink.CcLinkingContext; import com.google.devtools.build.lib.skylarkbuildapi.cpp.BazelCcModuleApi; @@ -44,7 +44,7 @@ public class BazelCcModule extends CcModule Artifact, SkylarkRuleContext, CcToolchainProvider, - FeatureConfiguration, + FeatureConfigurationForStarlark, CompilationInfo, CcCompilationContext, CcCompilationOutputs, @@ -57,7 +57,7 @@ public class BazelCcModule extends CcModule @Override public CompilationInfo compile( SkylarkRuleContext skylarkRuleContext, - FeatureConfiguration skylarkFeatureConfiguration, + FeatureConfigurationForStarlark skylarkFeatureConfiguration, CcToolchainProvider skylarkCcToolchainProvider, SkylarkList sources, SkylarkList headers, @@ -87,7 +87,7 @@ public CompilationInfo compile( @Override public LinkingInfo link( SkylarkRuleContext skylarkRuleContext, - FeatureConfiguration skylarkFeatureConfiguration, + FeatureConfigurationForStarlark skylarkFeatureConfiguration, CcToolchainProvider skylarkCcToolchainProvider, CcCompilationOutputs ccCompilationOutputs, Object skylarkLinkopts, diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java index 2268dbdf55c04b..5118c660b0ffb9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java @@ -154,6 +154,7 @@ private static Runfiles collectRunfiles( RuleContext ruleContext, FeatureConfiguration featureConfiguration, CcToolchainProvider toolchain, + CppConfiguration cppConfiguration, List libraries, CcLinkingOutputs ccLibraryLinkingOutputs, CcCompilationContext ccCompilationContext, @@ -233,7 +234,7 @@ private static Runfiles collectRunfiles( builder.addSymlinksToArtifacts(ccCompilationContext.getAdditionalInputs()); builder.addSymlinksToArtifacts( ccCompilationContext.getTransitiveModules( - usePic(ruleContext, toolchain, featureConfiguration))); + usePic(ruleContext, toolchain, cppConfiguration, featureConfiguration))); } return builder.build(); } @@ -431,7 +432,7 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont } } - boolean usePic = usePic(ruleContext, ccToolchain, featureConfiguration); + boolean usePic = usePic(ruleContext, ccToolchain, cppConfiguration, featureConfiguration); // On Windows, if GENERATE_PDB_FILE feature is enabled // then a pdb file will be built along with the executable. @@ -523,7 +524,7 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont Artifact dwpFile = ruleContext.getImplicitOutputArtifact(CppRuleClasses.CC_BINARY_DEBUG_PACKAGE); createDebugPackagerActions( - ruleContext, ccToolchain, featureConfiguration, dwpFile, dwoArtifacts); + ruleContext, ccToolchain, cppConfiguration, featureConfiguration, dwpFile, dwoArtifacts); // The debug package should include the dwp file only if it was explicitly requested. Artifact explicitDwpFile = dwpFile; @@ -571,6 +572,7 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont ruleContext, featureConfiguration, ccToolchain, + cppConfiguration, libraries, ccLinkingOutputs, ccCompilationContext, @@ -843,10 +845,13 @@ private static DwoArtifactsCollector collectTransitiveDwoArtifacts( public static Iterable getDwpInputs( RuleContext context, CcToolchainProvider toolchain, + CppConfiguration cppConfiguration, FeatureConfiguration featureConfiguration, NestedSet picDwoArtifacts, NestedSet dwoArtifacts) { - return usePic(context, toolchain, featureConfiguration) ? picDwoArtifacts : dwoArtifacts; + return usePic(context, toolchain, cppConfiguration, featureConfiguration) + ? picDwoArtifacts + : dwoArtifacts; } /** @@ -855,6 +860,7 @@ public static Iterable getDwpInputs( private static void createDebugPackagerActions( RuleContext context, CcToolchainProvider toolchain, + CppConfiguration cppConfiguration, FeatureConfiguration featureConfiguration, Artifact dwpOutput, DwoArtifactsCollector dwoArtifactsCollector) { @@ -862,6 +868,7 @@ private static void createDebugPackagerActions( getDwpInputs( context, toolchain, + cppConfiguration, featureConfiguration, dwoArtifactsCollector.getPicDwoArtifacts(), dwoArtifactsCollector.getDwoArtifacts()); @@ -1053,7 +1060,7 @@ private static void addTransitiveInfoProviders( NestedSet filesToCompile = ccCompilationOutputs.getFilesToCompile( cppConfiguration.processHeadersInDependencies(), - toolchain.usePicForDynamicLibraries(featureConfiguration)); + toolchain.usePicForDynamicLibraries(cppConfiguration, featureConfiguration)); builder .setFilesToBuild(filesToBuild) @@ -1094,11 +1101,13 @@ private static NestedSet collectTransitiveCcNativeLibraries( private static boolean usePic( RuleContext ruleContext, CcToolchainProvider ccToolchainProvider, + CppConfiguration cppConfiguration, FeatureConfiguration featureConfiguration) { if (isLinkShared(ruleContext)) { - return ccToolchainProvider.usePicForDynamicLibraries(featureConfiguration); + return ccToolchainProvider.usePicForDynamicLibraries(cppConfiguration, featureConfiguration); } else { - return CppHelper.usePicForBinaries(ccToolchainProvider, featureConfiguration); + return CppHelper.usePicForBinaries( + ccToolchainProvider, cppConfiguration, featureConfiguration); } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java index 8f1594b0957800..09ebef86d1e047 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java @@ -292,11 +292,11 @@ public CcCompilationHelper( this.configuration = buildConfiguration; this.cppConfiguration = configuration.getFragment(CppConfiguration.class); setGenerateNoPicAction( - !ccToolchain.usePicForDynamicLibraries(featureConfiguration) - || !CppHelper.usePicForBinaries(ccToolchain, featureConfiguration)); + !ccToolchain.usePicForDynamicLibraries(cppConfiguration, featureConfiguration) + || !CppHelper.usePicForBinaries(ccToolchain, cppConfiguration, featureConfiguration)); setGeneratePicAction( - ccToolchain.usePicForDynamicLibraries(featureConfiguration) - || CppHelper.usePicForBinaries(ccToolchain, featureConfiguration)); + ccToolchain.usePicForDynamicLibraries(cppConfiguration, featureConfiguration) + || CppHelper.usePicForBinaries(ccToolchain, cppConfiguration, featureConfiguration)); this.ruleErrorConsumer = actionConstructionContext.getRuleErrorConsumer(); this.actionRegistry = Preconditions.checkNotNull(actionRegistry); this.label = Preconditions.checkNotNull(label); @@ -715,7 +715,7 @@ public static Map> buildOutputGroupsForEmittingCompi Map> outputGroups = new TreeMap<>(); outputGroups.put(OutputGroupInfo.TEMP_FILES, ccCompilationOutputs.getTemps()); boolean processHeadersInDependencies = cppConfiguration.processHeadersInDependencies(); - boolean usePic = ccToolchain.usePicForDynamicLibraries(featureConfiguration); + boolean usePic = ccToolchain.usePicForDynamicLibraries(cppConfiguration, featureConfiguration); outputGroups.put( OutputGroupInfo.FILES_TO_COMPILE, ccCompilationOutputs.getFilesToCompile(processHeadersInDependencies, usePic)); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java index bea0f1f5e11ffc..f4cbee7e37b2e2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java @@ -481,7 +481,7 @@ private static NestedSet collectHiddenTopLevelArtifacts( NestedSetBuilder artifactsToForceBuilder = NestedSetBuilder.stableOrder(); CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); boolean processHeadersInDependencies = cppConfiguration.processHeadersInDependencies(); - boolean usePic = toolchain.usePicForDynamicLibraries(featureConfiguration); + boolean usePic = toolchain.usePicForDynamicLibraries(cppConfiguration, featureConfiguration); artifactsToForceBuilder.addTransitive( ccCompilationOutputs.getFilesToCompile(processHeadersInDependencies, usePic)); for (OutputGroupInfo dep : diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java index 96e8a73ae3eaa7..3e3f20b0db97af 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java @@ -404,8 +404,10 @@ private CcLinkingOutputs createCcLinkActions(CcCompilationOutputs ccOutputs) "can only handle static links"); LibraryToLink.Builder libraryToLinkBuilder = LibraryToLink.builder(); - boolean usePicForBinaries = CppHelper.usePicForBinaries(ccToolchain, featureConfiguration); - boolean usePicForDynamicLibs = ccToolchain.usePicForDynamicLibraries(featureConfiguration); + boolean usePicForBinaries = + CppHelper.usePicForBinaries(ccToolchain, cppConfiguration, featureConfiguration); + boolean usePicForDynamicLibs = + ccToolchain.usePicForDynamicLibraries(cppConfiguration, featureConfiguration); PathFragment labelName = PathFragment.create(label.getName()); String libraryIdentifier = diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java index dafc947126fb38..ccfbfb15f3b069 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java @@ -43,7 +43,6 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.EnvEntry; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.EnvSet; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Feature; -import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Flag; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FlagGroup; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FlagSet; @@ -83,7 +82,7 @@ public class CcModule implements CcModuleApi< CcToolchainProvider, - FeatureConfiguration, + FeatureConfigurationForStarlark, CcCompilationContext, CcLinkingContext, LibraryToLink, @@ -132,7 +131,7 @@ public Provider getCcToolchainProvider() { } @Override - public FeatureConfiguration configureFeatures( + public FeatureConfigurationForStarlark configureFeatures( Object ruleContextOrNone, CcToolchainProvider toolchain, SkylarkList requestedFeatures, @@ -150,51 +149,62 @@ public FeatureConfiguration configureFeatures( + "Please add 'ctx' as a named parameter. See " + "https://github.com/bazelbuild/bazel/issues/7793 for details."); } - return CcCommon.configureFeaturesOrThrowEvalException( - ImmutableSet.copyOf(requestedFeatures), - ImmutableSet.copyOf(unsupportedFeatures), - toolchain, + CppConfiguration cppConfiguration = ruleContext == null ? toolchain.getCppConfigurationEvenThoughItCanBeDifferentThatWhatTargetHas() - : ruleContext.getRuleContext().getFragment(CppConfiguration.class)); + : ruleContext.getRuleContext().getFragment(CppConfiguration.class); + return FeatureConfigurationForStarlark.from( + CcCommon.configureFeaturesOrThrowEvalException( + ImmutableSet.copyOf(requestedFeatures), + ImmutableSet.copyOf(unsupportedFeatures), + toolchain, + cppConfiguration), + cppConfiguration); } @Override - public String getToolForAction(FeatureConfiguration featureConfiguration, String actionName) { - return featureConfiguration.getToolPathForAction(actionName); + public String getToolForAction( + FeatureConfigurationForStarlark featureConfiguration, String actionName) { + return featureConfiguration.getFeatureConfiguration().getToolPathForAction(actionName); } @Override - public boolean isEnabled(FeatureConfiguration featureConfiguration, String featureName) { - return featureConfiguration.isEnabled(featureName); + public boolean isEnabled( + FeatureConfigurationForStarlark featureConfiguration, String featureName) { + return featureConfiguration.getFeatureConfiguration().isEnabled(featureName); } @Override - public boolean actionIsEnabled(FeatureConfiguration featureConfiguration, String actionName) { - return featureConfiguration.actionIsConfigured(actionName); + public boolean actionIsEnabled( + FeatureConfigurationForStarlark featureConfiguration, String actionName) { + return featureConfiguration.getFeatureConfiguration().actionIsConfigured(actionName); } @Override public SkylarkList getCommandLine( - FeatureConfiguration featureConfiguration, + FeatureConfigurationForStarlark featureConfiguration, String actionName, CcToolchainVariables variables) { - return SkylarkList.createImmutable(featureConfiguration.getCommandLine(actionName, variables)); + return SkylarkList.createImmutable( + featureConfiguration.getFeatureConfiguration().getCommandLine(actionName, variables)); } @Override public SkylarkDict getEnvironmentVariable( - FeatureConfiguration featureConfiguration, + FeatureConfigurationForStarlark featureConfiguration, String actionName, CcToolchainVariables variables) { return SkylarkDict.copyOf( - null, featureConfiguration.getEnvironmentVariables(actionName, variables)); + null, + featureConfiguration + .getFeatureConfiguration() + .getEnvironmentVariables(actionName, variables)); } @Override public CcToolchainVariables getCompileBuildVariables( CcToolchainProvider ccToolchainProvider, - FeatureConfiguration featureConfiguration, + FeatureConfigurationForStarlark featureConfiguration, Object sourceFile, Object outputFile, Object userCompileFlags, @@ -206,7 +216,7 @@ public CcToolchainVariables getCompileBuildVariables( boolean addLegacyCxxOptions) throws EvalException { return CompileBuildVariables.setupVariablesOrThrowEvalException( - featureConfiguration, + featureConfiguration.getFeatureConfiguration(), ccToolchainProvider, convertFromNoneable(sourceFile, /* defaultValue= */ null), convertFromNoneable(outputFile, /* defaultValue= */ null), @@ -234,7 +244,7 @@ public CcToolchainVariables getCompileBuildVariables( @Override public CcToolchainVariables getLinkBuildVariables( CcToolchainProvider ccToolchainProvider, - FeatureConfiguration featureConfiguration, + FeatureConfigurationForStarlark featureConfiguration, Object librarySearchDirectories, Object runtimeLibrarySearchDirectories, Object userLinkFlags, @@ -257,9 +267,9 @@ public CcToolchainVariables getLinkBuildVariables( /* thinltoMergedObjectFile= */ null, mustKeepDebug, ccToolchainProvider, - // TODO(b/117917928): We cannot use cpp configuration from cc toolchain. Fix. - ccToolchainProvider.getCppConfiguration(), - featureConfiguration, + featureConfiguration + .getCppConfigurationFromFeatureConfigurationCreatedForStarlark_andIKnowWhatImDoing(), + featureConfiguration.getFeatureConfiguration(), useTestOnlyFlags, /* isLtoIndexing= */ false, userFlagsToIterable(userLinkFlags), @@ -354,8 +364,8 @@ public LibraryToLink createLibraryLinkerInput( throws EvalException, InterruptedException { SkylarkActionFactory skylarkActionFactory = nullIfNone(actionsObject, SkylarkActionFactory.class); - FeatureConfiguration featureConfiguration = - nullIfNone(featureConfigurationObject, FeatureConfiguration.class); + FeatureConfigurationForStarlark featureConfiguration = + nullIfNone(featureConfigurationObject, FeatureConfigurationForStarlark.class); CcToolchainProvider ccToolchainProvider = nullIfNone(ccToolchainProviderObject, CcToolchainProvider.class); Artifact staticLibrary = nullIfNone(staticLibraryObject, Artifact.class); @@ -425,7 +435,7 @@ public LibraryToLink createLibraryLinkerInput( Artifact resolvedSymlinkDynamicLibrary = null; Artifact resolvedSymlinkInterfaceLibrary = null; - if (!featureConfiguration.isEnabled(CppRuleClasses.TARGETS_WINDOWS)) { + if (!featureConfiguration.getFeatureConfiguration().isEnabled(CppRuleClasses.TARGETS_WINDOWS)) { if (dynamicLibrary != null) { resolvedSymlinkDynamicLibrary = dynamicLibrary; dynamicLibrary = @@ -598,7 +608,7 @@ protected static CompilationInfo compile( RuleContext ruleContext = skylarkRuleContext.getRuleContext(); SkylarkActionFactory actions = skylarkRuleContext.actions(); CcToolchainProvider ccToolchainProvider = convertFromNoneable(skylarkCcToolchainProvider, null); - FeatureConfiguration featureConfiguration = + FeatureConfigurationForStarlark featureConfiguration = convertFromNoneable(skylarkFeatureConfiguration, null); Pair, List> separatedHeadersAndSources = separateSourcesFromHeaders(sources); @@ -616,7 +626,7 @@ protected static CompilationInfo compile( ? ruleContext.getPrerequisiteArtifact("$grep_includes", Mode.HOST) : null, cppSemantics, - featureConfiguration, + featureConfiguration.getFeatureConfiguration(), ccToolchainProvider, fdoContext) .addPublicHeaders(headers) @@ -685,7 +695,7 @@ protected static LinkingInfo link( CcCommon.checkRuleWhitelisted(skylarkRuleContext); RuleContext ruleContext = skylarkRuleContext.getRuleContext(); CcToolchainProvider ccToolchainProvider = convertFromNoneable(skylarkCcToolchainProvider, null); - FeatureConfiguration featureConfiguration = + FeatureConfigurationForStarlark featureConfiguration = convertFromNoneable(skylarkFeatureConfiguration, null); FdoContext fdoContext = ccToolchainProvider.getFdoContext(); NestedSet linkopts = @@ -694,7 +704,7 @@ protected static LinkingInfo link( new CcLinkingHelper( ruleContext, cppSemantics, - featureConfiguration, + featureConfiguration.getFeatureConfiguration(), ccToolchainProvider, fdoContext, ruleContext.getConfiguration()) 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 8f6ed20e76408c..655d83269c284f 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 @@ -38,7 +38,6 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.StringValueParser; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization; -import com.google.devtools.build.lib.skylarkbuildapi.cpp.FeatureConfigurationApi; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.StringUtil; @@ -1208,7 +1207,7 @@ public String getArtifactName(String baseName) { /** Captures the set of enabled features and action configs for a rule. */ @Immutable @AutoCodec - public static class FeatureConfiguration implements FeatureConfigurationApi { + public static class FeatureConfiguration { private static final Interner FEATURE_CONFIGURATION_INTERNER = BlazeInterners.newWeakInterner(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java index eab0e5993985c1..3d6a74df4c0beb 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java @@ -48,7 +48,7 @@ @Immutable @AutoCodec public final class CcToolchainProvider extends ToolchainInfo - implements CcToolchainProviderApi, HasCcToolchainLabel { + implements CcToolchainProviderApi, HasCcToolchainLabel { /** An empty toolchain to be returned in the error case (instead of null). */ public static final CcToolchainProvider EMPTY_TOOLCHAIN_IS_ERROR = @@ -274,6 +274,19 @@ public static Map getCppBuildVariables( return result.build(); } + /** + * See {@link #usePicForDynamicLibraries(FeatureConfigurationForStarlark)}. This method is there + * only to serve Starlark callers. + */ + @Override + public boolean usePicForDynamicLibrariesFromStarlark( + FeatureConfigurationForStarlark featureConfiguration) { + return usePicForDynamicLibraries( + featureConfiguration + .getCppConfigurationFromFeatureConfigurationCreatedForStarlark_andIKnowWhatImDoing(), + featureConfiguration.getFeatureConfiguration()); + } + /** * Determines if we should apply -fPIC for this rule's C++ compilations. This determination is * generally made by the global C++ configuration settings "needsPic" and "usePicForBinaries". @@ -283,9 +296,9 @@ public static Map getCppBuildVariables( * * @return true if this rule's compilations should apply -fPIC, false otherwise */ - @Override - public boolean usePicForDynamicLibraries(FeatureConfiguration featureConfiguration) { - return forcePic + public boolean usePicForDynamicLibraries( + CppConfiguration cppConfiguration, FeatureConfiguration featureConfiguration) { + return cppConfiguration.forcePic() || toolchainNeedsPic() || featureConfiguration.isEnabled(CppRuleClasses.SUPPORTS_PIC); } 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 8548c6553f46dd..ac2dab6f01c257 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 @@ -258,10 +258,7 @@ public Label getRuleProvidingCcToolchainProvider() { return cppOptions.crosstoolTop; } - /** - * Returns the configured current compilation mode. Rules should not call this directly, but - * instead use {@code CcToolchainProvider.getCompilationMode}. - */ + /** Returns the configured current compilation mode. */ public CompilationMode getCompilationMode() { return compilationMode; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java index 5dbe4df8354758..fad7db8f86fd25 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java @@ -514,16 +514,18 @@ public static void checkLinkstampsUnique( /** Returns whether binaries must be compiled with position independent code. */ public static boolean usePicForBinaries( - CcToolchainProvider toolchain, FeatureConfiguration featureConfiguration) { + CcToolchainProvider toolchain, + CppConfiguration cppConfiguration, + FeatureConfiguration featureConfiguration) { // TODO(b/124030770): Please do not use this feature without contacting the C++ rules team at // bazel-team@google.com. The feature will be removed in a later Bazel release and it might // break you. Contact us so we can find alternatives for your build. if (featureConfiguration.getRequestedFeatures().contains("coptnopic")) { return false; } - return toolchain.getCppConfiguration().forcePic() - || (toolchain.usePicForDynamicLibraries(featureConfiguration) - && toolchain.getCppConfiguration().getCompilationMode() != CompilationMode.OPT); + return cppConfiguration.forcePic() + || (toolchain.usePicForDynamicLibraries(cppConfiguration, featureConfiguration) + && cppConfiguration.getCompilationMode() != CompilationMode.OPT); } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java index 77df428a21689e..ffd203de1fcd29 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java @@ -1044,7 +1044,8 @@ public CppLinkAction build() throws InterruptedException { featureConfiguration, cppConfiguration.forcePic() || (linkType.isDynamicLibrary() - && toolchain.usePicForDynamicLibraries(featureConfiguration)), + && toolchain.usePicForDynamicLibraries( + cppConfiguration, featureConfiguration)), Matcher.quoteReplacement( isNativeDeps && cppConfiguration.shareNativeDeps() ? output.getExecPathString() diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FeatureConfigurationForStarlark.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FeatureConfigurationForStarlark.java new file mode 100644 index 00000000000000..1d0c8fc8264781 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FeatureConfigurationForStarlark.java @@ -0,0 +1,58 @@ +// Copyright 2019 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.rules.cpp; + +import com.google.common.base.Preconditions; +import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; +import com.google.devtools.build.lib.skylarkbuildapi.cpp.FeatureConfigurationApi; + +/** + * Wrapper class for {@link FeatureConfiguration} and {@link CppConfiguration}. + * + *

Instances are created in Starlark by cc_common.configure_features(ctx, cc_toolchain), and + * passed around pretending to be {@link FeatureConfiguration}. Then when the need arises, we get + * the {@link CppConfiguration} from it and use it in times when configuration of cc_toolchain is + * different than configuration of the rule depending on it. + */ +// TODO(b/129045294): Remove once cc_toolchain has target configuration. +public class FeatureConfigurationForStarlark implements FeatureConfigurationApi { + + private final FeatureConfiguration featureConfiguration; + private final CppConfiguration cppConfiguration; + + public static FeatureConfigurationForStarlark from( + FeatureConfiguration featureConfiguration, CppConfiguration cppConfiguration) { + return new FeatureConfigurationForStarlark(featureConfiguration, cppConfiguration); + } + + private FeatureConfigurationForStarlark( + FeatureConfiguration featureConfiguration, CppConfiguration cppConfiguration) { + this.featureConfiguration = Preconditions.checkNotNull(featureConfiguration); + this.cppConfiguration = Preconditions.checkNotNull(cppConfiguration); + } + + public FeatureConfiguration getFeatureConfiguration() { + return featureConfiguration; + } + + /** + * Get {@link CppConfiguration} that is threaded along with {@link FeatureConfiguration}. Do this + * only when you're completely aware of why this method was added and hlopko@ allowed you to. + */ + CppConfiguration + getCppConfigurationFromFeatureConfigurationCreatedForStarlark_andIKnowWhatImDoing() { + return cppConfiguration; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java index b1d5c22fb345a0..202f1150837023 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java @@ -121,7 +121,7 @@ public static CcToolchainVariables setupVariables( new CcToolchainVariables.Builder(ccToolchainProvider.getBuildVariables()); // pic - if (ccToolchainProvider.getForcePic()) { + if (cppConfiguration.forcePic()) { buildVariables.addStringVariable(FORCE_PIC.getVariableName(), ""); } diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcToolchainProviderApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcToolchainProviderApi.java index 4d83410b3dfb8e..1acf63b09804cb 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcToolchainProviderApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/cpp/CcToolchainProviderApi.java @@ -41,7 +41,7 @@ public interface CcToolchainProviderApi