RELNOTES: Effectively remove sysroot from CppConfiguration and allow …

…it to use select statements.

PiperOrigin-RevId: 155480011

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

@@ -523,8 +523,7 @@ private static final boolean isLinkShared(RuleContext context) {
 
   private static final boolean dashStaticInLinkopts(List<String> linkopts,
       CppConfiguration cppConfiguration) {
-    return linkopts.contains("-static")
-        || cppConfiguration.getLinkOptions().contains("-static");
+    return linkopts.contains("-static") || cppConfiguration.hasStaticLinkOption();
   }
 
   private static final LinkStaticness getLinkStaticness(RuleContext context,

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

@@ -36,6 +36,7 @@
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.actions.SymlinkAction;
 import com.google.devtools.build.lib.analysis.config.CompilationMode;
+import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -284,6 +285,15 @@ public ConfiguredTarget create(RuleContext ruleContext)
       coverage = crosstool;
     }
 
+    PathFragment sysroot = calculateSysroot(ruleContext);
+
+    ImmutableList<PathFragment> builtInIncludeDirectories = null;
+    try {
+      builtInIncludeDirectories = cppConfiguration.getBuiltInIncludeDirectories(sysroot);
+    } catch (InvalidConfigurationException e) {
+      ruleContext.ruleError(e.getMessage());
+    }
+
     CcToolchainProvider ccProvider =
         new CcToolchainProvider(
             cppConfiguration,
@@ -310,9 +320,8 @@ public ConfiguredTarget create(RuleContext ruleContext)
             coverageEnvironment.build(),
             ruleContext.getPrerequisiteArtifact("$link_dynamic_library_tool", Mode.HOST),
             getEnvironment(ruleContext),
-            cppConfiguration.getBuiltInIncludeDirectories());
-
-    PathFragment sysroot = cppConfiguration.getSysroot();
+            builtInIncludeDirectories,
+            sysroot);
 
     MakeVariableProvider makeVariableProvider =
         createMakeVariableProvider(cppConfiguration, sysroot);
@@ -481,4 +490,16 @@ protected Map<String, String> getBuildVariables(RuleContext ruleContext)
   protected ImmutableMap<String, String> getEnvironment(RuleContext ruleContext) {
     return ImmutableMap.<String, String>of();
   }
+
+  private PathFragment calculateSysroot(RuleContext ruleContext) {
+
+    TransitiveInfoCollection sysrootTarget = ruleContext.getPrerequisite(":sysroot", Mode.TARGET);
+    if (sysrootTarget == null) {
+      CppConfiguration cppConfiguration =
+          Preconditions.checkNotNull(ruleContext.getFragment(CppConfiguration.class));
+      return cppConfiguration.getDefaultSysroot();
+    }
+
+    return sysrootTarget.getLabel().getPackageFragment();
+  }
 }

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

@@ -75,7 +75,8 @@ public final class CcToolchainProvider
           NestedSetBuilder.<Pair<String, String>>emptySet(Order.COMPILE_ORDER),
           null,
           ImmutableMap.<String, String>of(),
-          ImmutableList.<PathFragment>of());
+          ImmutableList.<PathFragment>of(),
+          null);
 
   @Nullable private final CppConfiguration cppConfiguration;
   private final NestedSet<Artifact> crosstool;
@@ -102,6 +103,7 @@ public final class CcToolchainProvider
   @Nullable private final Artifact linkDynamicLibraryTool;
   private final ImmutableMap<String, String> environment;
   private final ImmutableList<PathFragment> builtInIncludeDirectories;
+  @Nullable private final PathFragment sysroot;
 
   public CcToolchainProvider(
       @Nullable CppConfiguration cppConfiguration,
@@ -128,7 +130,8 @@ public CcToolchainProvider(
       NestedSet<Pair<String, String>> coverageEnvironment,
       Artifact linkDynamicLibraryTool,
       ImmutableMap<String, String> environment,
-      ImmutableList<PathFragment> builtInIncludeDirectories) {
+      ImmutableList<PathFragment> builtInIncludeDirectories,
+      @Nullable PathFragment sysroot) {
     super(SKYLARK_CONSTRUCTOR, ImmutableMap.<String, Object>of());
     this.cppConfiguration = cppConfiguration;
     this.crosstool = Preconditions.checkNotNull(crosstool);
@@ -155,6 +158,7 @@ public CcToolchainProvider(
     this.linkDynamicLibraryTool = linkDynamicLibraryTool;
     this.environment = environment;
     this.builtInIncludeDirectories = builtInIncludeDirectories;
+    this.sysroot = sysroot;
   }
 
   @SkylarkCallable(
@@ -352,7 +356,7 @@ public Artifact getInterfaceSoBuilder() {
             + "this method returns <code>None</code>."
   )
   public PathFragment getSysroot() {
-    return cppConfiguration.getSysroot();
+    return sysroot;
   }
 
   @SkylarkCallable(
@@ -362,7 +366,7 @@ public PathFragment getSysroot() {
             + "rules. These should be appended to the command line after filtering."
   )
   public ImmutableList<String> getUnfilteredCompilerOptions(Iterable<String> features) {
-    return cppConfiguration.getUnfilteredCompilerOptions(features);
+    return cppConfiguration.getUnfilteredCompilerOptions(features, sysroot);
   }
 
   @SkylarkCallable(
@@ -373,6 +377,6 @@ public ImmutableList<String> getUnfilteredCompilerOptions(Iterable<String> featu
             + "inferred from the command-line options."
   )
   public ImmutableList<String> getLinkOptions() {
-    return cppConfiguration.getLinkOptions();
+    return cppConfiguration.getLinkOptions(sysroot);
   }
 }

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

@@ -51,9 +51,8 @@ static boolean isCcToolchain(Target target) {
   private static final LateBoundLabel<BuildConfiguration> LIBC_TOP =
       new LateBoundLabel<BuildConfiguration>(CppConfiguration.class) {
         @Override
-        public Label resolve(
-            Rule rule, AttributeMap attributes, BuildConfiguration configuration) {
-          return configuration.getFragment(CppConfiguration.class).getLibcLabel();
+        public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) {
+          return configuration.getFragment(CppConfiguration.class).getSysrootLabel();
         }
       };
 
@@ -88,6 +87,7 @@ public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
                 .value(env.getToolsLabel("//tools/cpp:link_dynamic_library")))
         // TODO(bazel-team): Should be using the TARGET configuration.
         .add(attr(":libc_top", LABEL).cfg(HOST).value(LIBC_TOP))
+        .add(attr(":sysroot", LABEL).value(LIBC_TOP))
         .add(
             attr(":lipo_context_collector", LABEL)
                 .cfg(LipoTransition.LIPO_COLLECTOR)

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

@@ -45,6 +45,7 @@ public final class CompileCommandLine {
   @VisibleForTesting public final CcToolchainFeatures.Variables variables;
   private final String actionName;
   private final CppConfiguration cppConfiguration;
+  private final ImmutableList<String> unfilteredCompilerOptions;
 
   public CompileCommandLine(
       Artifact sourceFile,
@@ -57,7 +58,8 @@ public CompileCommandLine(
       CppConfiguration cppConfiguration,
       CcToolchainFeatures.Variables variables,
       String actionName,
-      DotdFile dotdFile) {
+      DotdFile dotdFile,
+      ImmutableList<String> unfilteredCompilerOptions) {
     this.sourceFile = Preconditions.checkNotNull(sourceFile);
     this.outputFile = Preconditions.checkNotNull(outputFile);
     this.sourceLabel = Preconditions.checkNotNull(sourceLabel);
@@ -69,6 +71,7 @@ public CompileCommandLine(
     this.variables = variables;
     this.actionName = actionName;
     this.dotdFile = isGenerateDotdFile(sourceFile) ? Preconditions.checkNotNull(dotdFile) : null;
+    this.unfilteredCompilerOptions = unfilteredCompilerOptions;
   }
 
   /** Returns true if Dotd file should be generated. */
@@ -161,7 +164,7 @@ public List<String> getCompilerOptions(
     // the user provided options, otherwise users adding include paths will not pick up their
     // own include paths first.
     if (!isObjcCompile(actionName)) {
-      options.addAll(toolchain.getUnfilteredCompilerOptions(features));
+      options.addAll(unfilteredCompilerOptions);
     }
 
     // Add the options of --per_file_copt, if the label or the base name of the source file

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

@@ -217,6 +217,8 @@ public Collection<Artifact> getInputsForIncludedFile(
   /** Whether this action needs to discover inputs. */
   private final boolean discoversInputs;
 
+  private final ImmutableList<PathFragment> builtInIncludeDirectories;
+
   /**
    * Set when the action prepares for execution. Used to preserve state between preparation and
    * execution.
@@ -271,6 +273,8 @@ public Collection<Artifact> getInputsForIncludedFile(
    * @param actionName a string giving the name of this action for the purpose of toolchain
    *     evaluation
    * @param cppSemantics C++ compilation semantics
+   * @param unfilteredCompilerOptions - Compile options that will always be added.
+   * @param builtInIncludeDirectories - Directories to search for system includes from.
    */
   protected CppCompileAction(
       ActionOwner owner,
@@ -308,7 +312,9 @@ protected CppCompileAction(
       ImmutableMap<String, String> environment,
       String actionName,
       Iterable<Artifact> builtinIncludeFiles,
-      CppSemantics cppSemantics) {
+      CppSemantics cppSemantics,
+      ImmutableList<String> unfilteredCompilerOptions,
+      ImmutableList<PathFragment> builtInIncludeDirectories) {
     super(
         owner,
         allInputs,
@@ -349,7 +355,8 @@ protected CppCompileAction(
             cppConfiguration,
             variables,
             actionName,
-            dotdFile);
+            dotdFile,
+            unfilteredCompilerOptions);
     this.actionContext = actionContext;
     this.lipoScannables = lipoScannables;
     this.actionClassId = actionClassId;
@@ -363,6 +370,7 @@ protected CppCompileAction(
     this.builtinIncludeFiles = ImmutableList.copyOf(builtinIncludeFiles);
     this.cppSemantics = cppSemantics;
     this.additionalIncludeScannables = ImmutableList.copyOf(additionalIncludeScannables);
+    this.builtInIncludeDirectories = builtInIncludeDirectories;
   }
 
   /**
@@ -381,7 +389,7 @@ public boolean shouldScanIncludes() {
 
   @Override
   public List<PathFragment> getBuiltInIncludeDirectories() {
-    return cppConfiguration.getBuiltInIncludeDirectories();
+    return builtInIncludeDirectories;
   }
 
   @Nullable
@@ -835,9 +843,10 @@ public void validateInclusions(
     if (optionalSourceFile != null) {
       allowedIncludes.add(optionalSourceFile);
     }
-    Iterable<PathFragment> ignoreDirs = cppConfiguration.isStrictSystemIncludes()
-        ? cppConfiguration.getBuiltInIncludeDirectories()
-        : getValidationIgnoredDirs();
+    Iterable<PathFragment> ignoreDirs =
+        cppConfiguration.isStrictSystemIncludes()
+            ? getBuiltInIncludeDirectories()
+            : getValidationIgnoredDirs();
 
     // Copy the sets to hash sets for fast contains checking.
     // Avoid immutable sets here to limit memory churn.
@@ -906,7 +915,7 @@ public void validateInclusions(
   }
 
   Iterable<PathFragment> getValidationIgnoredDirs() {
-    List<PathFragment> cxxSystemIncludeDirs = cppConfiguration.getBuiltInIncludeDirectories();
+    List<PathFragment> cxxSystemIncludeDirs = getBuiltInIncludeDirectories();
     return Iterables.concat(
         cxxSystemIncludeDirs, context.getSystemIncludeDirs());
   }
@@ -1259,9 +1268,8 @@ public DependencySet processDepset(Path execRoot, Reply reply) throws ActionExec
   }
 
   public List<Path> getPermittedSystemIncludePrefixes(Path execRoot) {
-    CppConfiguration toolchain = cppConfiguration;
     List<Path> systemIncludePrefixes = new ArrayList<>();
-    for (PathFragment includePath : toolchain.getBuiltInIncludeDirectories()) {
+    for (PathFragment includePath : getBuiltInIncludeDirectories()) {
       if (includePath.isAbsolute()) {
         systemIncludePrefixes.add(execRoot.getFileSystem().getPath(includePath));
       }

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

@@ -386,6 +386,8 @@ public CppCompileAction build() {
           getLipoScannables(realMandatoryInputs),
           ccToolchain.getBuiltinIncludeFiles(),
           cppSemantics,
+          ccToolchain.getUnfilteredCompilerOptions(features),
+          ccToolchain.getBuiltInIncludeDirectories(),
           ImmutableMap.copyOf(executionInfo));
     } else {
       return new CppCompileAction(
@@ -422,7 +424,9 @@ public CppCompileAction build() {
           ImmutableMap.copyOf(environment),
           getActionName(),
           ccToolchain.getBuiltinIncludeFiles(),
-          cppSemantics);
+          cppSemantics,
+          ccToolchain.getUnfilteredCompilerOptions(features),
+          ccToolchain.getBuiltInIncludeDirectories());
     }
   }