Migrate usage of CppConfiguration#getToolPathFragment and

CppConfiguration#getLdExecutable to CcToolchainProvider.

PiperOrigin-RevId: 171818406

6 files changed, 59 insertions, 15 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
index 15bc567281..a9b82e4982 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
@@ -382,10 +382,26 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
       toolchain = getToolchainFromAttributes(ruleContext, cppConfiguration);
     }
 
+    CppToolchainInfo toolchainInfo = null;
+    if (toolchain != null) {
+      try {
+        toolchainInfo =
+            new CppToolchainInfo(
+                toolchain,
+                cppConfiguration.getCrosstoolTopPathFragment(),
+                cppConfiguration.getCrosstoolTop());
+      } catch (InvalidConfigurationException e) {
+        ruleContext.throwWithRuleError(e.getMessage());
+      }
+    } else {
+      toolchainInfo = cppConfiguration.getCppToolchainInfo();
+    }
+
     CcToolchainProvider ccProvider =
         new CcToolchainProvider(
             cppConfiguration,
             toolchain,
+            toolchainInfo,
             crosstool,
             fullInputsForCrosstool(ruleContext, crosstoolMiddleman),
             compile,
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 77eabde0cc..7b1fa5e7f5 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
@@ -43,6 +43,7 @@ public final class CcToolchainProvider extends ToolchainInfo {
       new CcToolchainProvider(
           null,
           null,
+          null,
           NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER),
           NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER),
           NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER),
@@ -72,6 +73,7 @@ public final class CcToolchainProvider extends ToolchainInfo {
 
   @Nullable private final CppConfiguration cppConfiguration;
   private final CToolchain toolchain;
+  private final CppToolchainInfo toolchainInfo;
   private final NestedSet<Artifact> crosstool;
   private final NestedSet<Artifact> crosstoolMiddleman;
   private final NestedSet<Artifact> compile;
@@ -102,6 +104,7 @@ public final class CcToolchainProvider extends ToolchainInfo {
   public CcToolchainProvider(
       @Nullable CppConfiguration cppConfiguration,
       CToolchain toolchain,
+      CppToolchainInfo toolchainInfo,
       NestedSet<Artifact> crosstool,
       NestedSet<Artifact> crosstoolMiddleman,
       NestedSet<Artifact> compile,
@@ -131,6 +134,7 @@ public final class CcToolchainProvider extends ToolchainInfo {
     super(ImmutableMap.of(), Location.BUILTIN);
     this.cppConfiguration = cppConfiguration;
     this.toolchain = toolchain;
+    this.toolchainInfo = toolchainInfo;
     this.crosstool = Preconditions.checkNotNull(crosstool);
     this.crosstoolMiddleman = Preconditions.checkNotNull(crosstoolMiddleman);
     this.compile = Preconditions.checkNotNull(compile);
@@ -368,6 +372,14 @@ public final class CcToolchainProvider extends ToolchainInfo {
     return sysroot;
   }
 
+  /**
+   * Returns the path fragment that is either absolute or relative to the execution root that can be
+   * used to execute the given tool.
+   */
+  public PathFragment getToolPathFragment(CppConfiguration.Tool tool) {
+    return toolchainInfo.getToolPathFragment(tool);
+  }
+
   @SkylarkCallable(
     name = "unfiltered_compiler_options_do_not_use",
     doc =
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 1ac5cf2ecc..5d6c3689d4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -246,6 +246,8 @@ public class CppCompileAction extends AbstractAction
 
   private ImmutableList<Artifact> resolvedInputs = ImmutableList.<Artifact>of();
 
+  private PathFragment gccToolPath;
+
   /**
    * Creates a new action to compile C/C++ source files.
    *
@@ -367,6 +369,7 @@ public class CppCompileAction extends AbstractAction
     this.additionalIncludeScannables = ImmutableList.copyOf(additionalIncludeScannables);
     this.builtInIncludeDirectories =
         ImmutableList.copyOf(cppProvider.getBuiltInIncludeDirectories());
+    this.gccToolPath = cppProvider.getToolPathFragment(Tool.GCC);
   }
 
   /**
@@ -761,7 +764,7 @@ public class CppCompileAction extends AbstractAction
   @Override
   public ExtraActionInfo.Builder getExtraActionInfo() {
     CppCompileInfo.Builder info = CppCompileInfo.newBuilder();
-    info.setTool(cppConfiguration.getToolPathFragment(Tool.GCC).getPathString());
+    info.setTool(gccToolPath.getPathString());
     for (String option : getCompilerOptions()) {
       info.addCompilerOption(option);
     }
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 c816424645..d8c606aca8 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
@@ -521,6 +521,11 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
         compilationMode, lipoMode, linkingMode, ldExecutable);
   }
 
+  /** Returns the {@link CppToolchainInfo} used by this configuration. */
+  public CppToolchainInfo getCppToolchainInfo() {
+    return cppToolchainInfo;
+  }
+
   /**
    * Returns the toolchain identifier, which uniquely identifies the compiler
    * version, target libc version, target cpu, and LIPO linkage.
@@ -534,6 +539,11 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     return crosstoolFile;
   }
 
+  /** Returns the label of the CROSSTOOL for this configuration. */
+  public Label getCrosstoolTop() {
+    return crosstoolTop;
+  }
+
   /** Returns the transformer that should be applied to cpu names in toolchain selection. */
   public Function<String, String> getCpuTransformer() {
     return cpuTransformer;
@@ -588,12 +598,12 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
   }
 
   /**
-   * Returns the path fragment that is either absolute or relative to the
-   * execution root that can be used to execute the given tool.
+   * Returns the path fragment that is either absolute or relative to the execution root that can be
+   * used to execute the given tool.
    *
-   * <p>Note that you must not use this method to get the linker location, but
-   * use {@link #getLdExecutable} instead!
+   * <p>Deprecated: Use {@link CcToolchainProvider#getToolPathFragment(Tool)}
    */
+  @Deprecated
   public PathFragment getToolPathFragment(CppConfiguration.Tool tool) {
     return cppToolchainInfo.getToolPathFragment(tool);
   }
@@ -1061,14 +1071,10 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
    */
   @SkylarkCallable(name = "ld_executable", structField = true, doc = "Path to the linker binary.")
   public String getLdExecutableForSkylark() {
-    PathFragment ldExecutable = getLdExecutable();
+    PathFragment ldExecutable = getToolPathFragment(CppConfiguration.Tool.LD);
     return ldExecutable != null ? ldExecutable.getPathString() : "";
   }
 
-  public PathFragment getLdExecutable() {
-    return cppToolchainInfo.getLdExecutable();
-  }
-
   /**
    * Returns the dynamic linking mode (full, off, or default).
    */
@@ -1594,7 +1600,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     // Make variables provided by crosstool/gcc compiler suite.
     globalMakeEnvBuilder.put("AR", getArExecutable().getPathString());
     globalMakeEnvBuilder.put("NM", getNmExecutable().getPathString());
-    globalMakeEnvBuilder.put("LD", getLdExecutable().getPathString());
+    globalMakeEnvBuilder.put("LD", getToolPathFragment(Tool.LD).getPathString());
     PathFragment objcopyTool = getObjCopyExecutable();
     if (objcopyTool != null) {
       // objcopy is optional in Crosstool
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
index 48d5f945e7..806ecfc96a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
@@ -45,6 +45,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkStaticness;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
 import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
@@ -114,6 +115,8 @@ public final class CppLinkAction extends AbstractAction
   private final boolean fake;
   private final boolean isLtoIndexing;
 
+  private final PathFragment ldExecutable;
+
   // This is set for both LTO indexing and LTO linking.
   @Nullable private final Iterable<LtoBackendArtifacts> allLtoBackendArtifacts;
   private final Iterable<Artifact> mandatoryInputs;
@@ -158,7 +161,8 @@ public final class CppLinkAction extends AbstractAction
       ImmutableSet<String> clientEnvironmentVariables,
       ImmutableMap<String, String> actionEnv,
       ImmutableMap<String, String> toolchainEnv,
-      ImmutableSet<String> executionRequirements) {
+      ImmutableSet<String> executionRequirements,
+      CcToolchainProvider toolchain) {
     super(owner, inputs, outputs);
     if (mnemonic == null) {
       this.mnemonic = (isLtoIndexing) ? "CppLTOIndexing" : "CppLink";
@@ -178,6 +182,7 @@ public final class CppLinkAction extends AbstractAction
     this.actionEnv = actionEnv;
     this.toolchainEnv = toolchainEnv;
     this.executionRequirements = executionRequirements;
+    this.ldExecutable = toolchain.getToolPathFragment(Tool.LD);
   }
 
   private CppConfiguration getCppConfiguration() {
@@ -445,7 +450,7 @@ public final class CppLinkAction extends AbstractAction
   protected String computeKey() {
     Fingerprint f = new Fingerprint();
     f.addString(fake ? FAKE_LINK_GUID : LINK_GUID);
-    f.addString(getCppConfiguration().getLdExecutable().getPathString());
+    f.addString(ldExecutable.getPathString());
     f.addStrings(linkCommandLine.arguments());
     f.addStrings(getExecutionInfo().keySet());
 
@@ -475,7 +480,8 @@ public final class CppLinkAction extends AbstractAction
     message.append('\n');
     message.append("  Command: ");
     message.append(
-        ShellEscaper.escapeString(getCppConfiguration().getLdExecutable().getPathString()));
+        ShellEscaper.escapeString(
+            getCppConfiguration().getToolPathFragment(Tool.LD).getPathString()));
     message.append('\n');
     // Outputting one argument per line makes it easier to diff the results.
     for (String argument : ShellEscaper.escapeAll(linkCommandLine.arguments())) {
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 6af0118e78..5c2c2f6063 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
@@ -935,7 +935,8 @@ public class CppLinkActionBuilder {
         configuration.getVariableShellEnvironment(),
         configuration.getLocalShellEnvironment(),
         toolchainEnv,
-        executionRequirements.build());
+        executionRequirements.build(),
+        toolchain);
   }
 
   private boolean shouldUseLinkDynamicLibraryTool() {