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

Author: hlopko

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<Artifact> sources,
       SkylarkList<Artifact> headers,
@@ -87,7 +87,7 @@ public CompilationInfo compile(
   @Override
   public LinkingInfo link(
       SkylarkRuleContext skylarkRuleContext,
-      FeatureConfiguration skylarkFeatureConfiguration,
+      FeatureConfigurationForStarlark skylarkFeatureConfiguration,
       CcToolchainProvider skylarkCcToolchainProvider,
       CcCompilationOutputs ccCompilationOutputs,
       Object skylarkLinkopts,

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<LibraryToLink> 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<Artifact> getDwpInputs(
       RuleContext context,
       CcToolchainProvider toolchain,
+      CppConfiguration cppConfiguration,
       FeatureConfiguration featureConfiguration,
       NestedSet<Artifact> picDwoArtifacts,
       NestedSet<Artifact> dwoArtifacts) {
-    return usePic(context, toolchain, featureConfiguration) ? picDwoArtifacts : dwoArtifacts;
+    return usePic(context, toolchain, cppConfiguration, featureConfiguration)
+        ? picDwoArtifacts
+        : dwoArtifacts;
   }
 
   /**
@@ -855,13 +860,15 @@ public static Iterable<Artifact> getDwpInputs(
   private static void createDebugPackagerActions(
       RuleContext context,
       CcToolchainProvider toolchain,
+      CppConfiguration cppConfiguration,
       FeatureConfiguration featureConfiguration,
       Artifact dwpOutput,
       DwoArtifactsCollector dwoArtifactsCollector) {
     Iterable<Artifact> allInputs =
         getDwpInputs(
             context,
             toolchain,
+            cppConfiguration,
             featureConfiguration,
             dwoArtifactsCollector.getPicDwoArtifacts(),
             dwoArtifactsCollector.getDwoArtifacts());
@@ -1053,7 +1060,7 @@ private static void addTransitiveInfoProviders(
     NestedSet<Artifact> filesToCompile =
         ccCompilationOutputs.getFilesToCompile(
             cppConfiguration.processHeadersInDependencies(),
-            toolchain.usePicForDynamicLibraries(featureConfiguration));
+            toolchain.usePicForDynamicLibraries(cppConfiguration, featureConfiguration));
 
     builder
         .setFilesToBuild(filesToBuild)
@@ -1094,11 +1101,13 @@ private static NestedSet<LibraryToLink> 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);
     }
   }
 }

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<String, NestedSet<Artifact>> buildOutputGroupsForEmittingCompi
     Map<String, NestedSet<Artifact>> 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));

src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java

@@ -481,7 +481,7 @@ private static NestedSet<Artifact> collectHiddenTopLevelArtifacts(
     NestedSetBuilder<Artifact> 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 :

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 =

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<String> 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<String> 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<String, String> 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<Artifact>, List<Artifact>> 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<String> linkopts =
@@ -694,7 +704,7 @@ protected static LinkingInfo link(
         new CcLinkingHelper(
                 ruleContext,
                 cppSemantics,
-                featureConfiguration,
+                featureConfiguration.getFeatureConfiguration(),
                 ccToolchainProvider,
                 fdoContext,
                 ruleContext.getConfiguration())

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<FeatureConfiguration> FEATURE_CONFIGURATION_INTERNER =
         BlazeInterners.newWeakInterner();