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
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 45302a4..0ad2a33 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 @@ 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 886455d..24c2d5d 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 @@ */ @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 @@ 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 @@ 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 @@ 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 @@ : 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 @@ @Nullable final Artifact paramFile = canSplitCommandLine() - ? analysisEnvironment.getDerivedArtifact( - paramRootPath, configuration.getBinDirectory()) + ? linkArtifactFactory.create(ruleContext, paramRootPath) : null; LinkCommandLine.Builder linkCommandLineBuilder = @@ -865,7 +896,8 @@ * 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 @@ 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 edeb70e..945bb1e 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.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 @@ 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 e808eb5..0a509cd 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 @@ * 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 @@ 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)