Abstract away artifact creation in CppLinkAction so that we can create most of the artifacts in a way that checks that they are under the package directory.

The exception is nativedeps, whose link actions are shared, and thus they cannot be at a package-specific location.

--
MOS_MIGRATED_REVID=101216949

4 files changed, 67 insertions, 23 deletions

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 45302a4794..0ad2a332dd 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
@@ -293,7 +293,7 @@ public class CppHelper {
   private static Artifact getIncludesOutput(RuleContext ruleContext, Artifact src) {
     Root root = ruleContext.getFragment(CppConfiguration.class).getGreppedIncludesDirectory();
     PathFragment relOut = IncludeScanningUtil.getRootRelativeOutputPath(src.getExecPath());
-    return ruleContext.getAnalysisEnvironment().getDerivedArtifact(relOut, root);
+    return ruleContext.getShareableArtifact(relOut, root);
   }
 
   /**
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 886455d5d5..24c2d5dc2d 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
@@ -76,6 +76,33 @@ import javax.annotation.Nullable;
  */
 @ThreadCompatible
 public final class CppLinkAction extends AbstractAction {
+  /**
+   * An abstraction for creating intermediate and output artifacts for C++ linking.
+   *
+   * <p>This is unfortunately necessary, because most of the time, these artifacts are well-behaved
+   * ones sitting under a package directory, but nativedeps link actions can be shared. In order to
+   * avoid creating every artifact here with {@code getShareableArtifact()}, we abstract the
+   * artifact creation away.
+   */
+  public interface LinkArtifactFactory {
+    /**
+     * Create an artifact at the specified root-relative path in the bin directory.
+     */
+    Artifact create(RuleContext ruleContext, PathFragment rootRelativePath);
+  }
+
+  /**
+   * An implementation of {@link LinkArtifactFactory} that can only create artifacts in the package
+   * directory.
+   */
+  public static final LinkArtifactFactory DEFAULT_ARTIFACT_FACTORY = new LinkArtifactFactory() {
+    @Override
+    public Artifact create(RuleContext ruleContext, PathFragment rootRelativePath) {
+      return ruleContext.getDerivedArtifact(rootRelativePath,
+          ruleContext.getConfiguration().getBinDirectory());
+    }
+  };
+
   private static final String LINK_GUID = "58ec78bd-1176-4e36-8143-439f656b181d";
   private static final String FAKE_LINK_GUID = "da36f819-5a15-43a9-8a45-e01b60e10c8b";
 
@@ -517,6 +544,7 @@ public final class CppLinkAction extends AbstractAction {
     private boolean isNativeDeps;
     private boolean useTestOnlyFlags;
     private boolean wholeArchive;
+    private LinkArtifactFactory linkArtifactFactory = DEFAULT_ARTIFACT_FACTORY;
 
     private boolean isLTOIndexing = false;
     private Iterable<LTOBackendArtifacts> allLTOArtifacts = null;
@@ -597,6 +625,11 @@ public final class CppLinkAction extends AbstractAction {
       this.useTestOnlyFlags = linkContext.useTestOnlyFlags;
     }
 
+    public CppLinkAction.Builder setLinkArtifactFactory(LinkArtifactFactory linkArtifactFactory) {
+      this.linkArtifactFactory = linkArtifactFactory;
+      return this;
+    }
+
     private Iterable<LTOBackendArtifacts> createLTOArtifacts(
         PathFragment ltoOutputRootPrefix, NestedSet<LibraryToLink> uniqueLibraries) {
       // This flattens the set of object files, so for M binaries and N .o files,
@@ -618,9 +651,8 @@ public final class CppLinkAction extends AbstractAction {
 
       ImmutableList.Builder<LTOBackendArtifacts> ltoOutputs = ImmutableList.builder();
       for (Artifact a : allBitcode) {
-        LTOBackendArtifacts ltoArtifacts =
-            new LTOBackendArtifacts(
-                ltoOutputRootPrefix, a, allBitcode, analysisEnvironment, configuration);
+        LTOBackendArtifacts ltoArtifacts = new LTOBackendArtifacts(
+            ltoOutputRootPrefix, a, allBitcode, configuration, ruleContext, linkArtifactFactory);
         ltoOutputs.add(ltoArtifacts);
       }
       return ltoOutputs.build();
@@ -688,7 +720,7 @@ public final class CppLinkAction extends AbstractAction {
               : LinkerInputs.newInputLibrary(interfaceOutput, filteredNonLibraryArtifacts);
 
       final ImmutableMap<Artifact, Artifact> linkstampMap =
-          mapLinkstampsToOutputs(linkstamps, ruleContext, output);
+          mapLinkstampsToOutputs(linkstamps, ruleContext, output, linkArtifactFactory);
 
       PathFragment ltoOutputRootPrefix = null;
       if (isLTOIndexing && allLTOArtifacts == null) {
@@ -721,8 +753,7 @@ public final class CppLinkAction extends AbstractAction {
       @Nullable
       final Artifact paramFile =
           canSplitCommandLine()
-              ? analysisEnvironment.getDerivedArtifact(
-                  paramRootPath, configuration.getBinDirectory())
+              ? linkArtifactFactory.create(ruleContext, paramRootPath)
               : null;
 
       LinkCommandLine.Builder linkCommandLineBuilder =
@@ -865,7 +896,8 @@ public final class CppLinkAction extends AbstractAction {
      *         corresponding object file that should be fed into the link
      */
     public static ImmutableMap<Artifact, Artifact> mapLinkstampsToOutputs(
-        Collection<Artifact> linkstamps, RuleContext ruleContext, Artifact outputBinary) {
+        Collection<Artifact> linkstamps, RuleContext ruleContext, Artifact outputBinary,
+        LinkArtifactFactory linkArtifactFactory) {
       ImmutableMap.Builder<Artifact, Artifact> mapBuilder = ImmutableMap.builder();
 
       PathFragment outputBinaryPath = outputBinary.getRootRelativePath();
@@ -878,8 +910,7 @@ public final class CppLinkAction extends AbstractAction {
         mapBuilder.put(linkstamp,
             // Note that link stamp actions can be shared between link actions that output shared
             // native dep libraries.
-            ruleContext.getAnalysisEnvironment().getDerivedArtifact(
-                stampOutputPath, outputBinary.getRoot()));
+            linkArtifactFactory.create(ruleContext, stampOutputPath));
       }
       return mapBuilder.build();    }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendArtifacts.java
index edeb70edf2..945bb1e9ef 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendArtifacts.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendArtifacts.java
@@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.Root;
-import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -79,22 +78,20 @@ public final class LTOBackendArtifacts {
       PathFragment ltoOutputRootPrefix,
       Artifact bitcodeFile,
       NestedSet<Artifact> allBitCodeFiles,
-      AnalysisEnvironment analysisEnvironment,
-      BuildConfiguration configuration) {
+      BuildConfiguration configuration,
+      RuleContext ruleContext,
+      CppLinkAction.LinkArtifactFactory linkArtifactFactory) {
     this.bitcodeFile = bitcodeFile;
     PathFragment obj = ltoOutputRootPrefix.getRelative(bitcodeFile.getRootRelativePath());
     Root binDir = configuration.getBinDirectory();
 
-    objectFile = analysisEnvironment.getDerivedArtifact(obj, binDir);
-    imports =
-        analysisEnvironment.getDerivedArtifact(
-            FileSystemUtils.replaceExtension(obj, ".imports"), binDir);
-    index =
-        analysisEnvironment.getDerivedArtifact(
-            FileSystemUtils.replaceExtension(obj, ".thinlto.index"), binDir);
-    beCommandline =
-        analysisEnvironment.getDerivedArtifact(
-            FileSystemUtils.replaceExtension(obj, ".thinlto_commandline.txt"), binDir);
+    objectFile = linkArtifactFactory.create(ruleContext, obj);
+    imports = linkArtifactFactory.create(
+        ruleContext, FileSystemUtils.replaceExtension(obj, ".imports"));
+    index = linkArtifactFactory.create(
+        ruleContext, FileSystemUtils.replaceExtension(obj, ".thinlto.index"));
+    beCommandline = linkArtifactFactory.create(
+        ruleContext, FileSystemUtils.replaceExtension(obj, ".thinlto_commandline.txt"));
 
     bitcodeFiles = allBitCodeFiles;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java b/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java
index e808eb538b..0a509cd722 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java
@@ -52,6 +52,21 @@ import java.util.Map;
  * that some rules are implicitly neverlink.
  */
 public abstract class NativeDepsHelper {
+  /**
+   * An implementation of {@link CppLinkAction.LinkArtifactFactory} that can create artifacts
+   * anywhere.
+   *
+   * <p>Necessary because the actions of nativedeps libraries should be shareable, and thus cannot
+   * be under the package directory.
+   */
+  private static final CppLinkAction.LinkArtifactFactory SHAREABLE_LINK_ARTIFACT_FACTORY =
+      new CppLinkAction.LinkArtifactFactory() {
+        @Override
+        public Artifact create(RuleContext ruleContext, PathFragment rootRelativePath) {
+          return ruleContext.getShareableArtifact(rootRelativePath,
+              ruleContext.getConfiguration().getBinDirectory());
+        }
+      };
 
   private NativeDepsHelper() {}
 
@@ -157,6 +172,7 @@ public abstract class NativeDepsHelper {
         linkerOutputPath, configuration.getBinDirectory());
     CppLinkAction.Builder builder = new CppLinkAction.Builder(
         ruleContext, sharedLibrary, configuration, toolchain);
+    builder.setLinkArtifactFactory(SHAREABLE_LINK_ARTIFACT_FACTORY);
     CppLinkAction linkAction = builder
         .setCrosstoolInputs(toolchain.getLink())
         .addLibraries(linkerInputs)