diff options
Diffstat (limited to 'src/main/java')
10 files changed, 332 insertions, 474 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java index cdd388342a..3f82509a59 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java @@ -57,7 +57,7 @@ public final class CompileCommandLine { this.cppConfiguration = Preconditions.checkNotNull(cppConfiguration); this.variables = variables; this.actionName = actionName; - this.dotdFile = isGenerateDotdFile(sourceFile) ? Preconditions.checkNotNull(dotdFile) : null; + this.dotdFile = isGenerateDotdFile(sourceFile) ? dotdFile : null; } /** Returns true if Dotd file should be generated. */ 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 667ef4d10f..4a97d11181 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 @@ -266,6 +266,7 @@ public class CppCompileAction extends AbstractAction boolean usePic, boolean useHeaderModules, NestedSet<Artifact> mandatoryInputs, + ImmutableList<Artifact> builtinIncludeFiles, NestedSet<Artifact> prunableInputs, Artifact outputFile, DotdFile dotdFile, @@ -334,7 +335,7 @@ public class CppCompileAction extends AbstractAction // artifact and will definitely exist prior to this action execution. this.mandatoryInputs = mandatoryInputs; this.prunableInputs = prunableInputs; - this.builtinIncludeFiles = ImmutableList.copyOf(cppProvider.getBuiltinIncludeFiles()); + this.builtinIncludeFiles = builtinIncludeFiles; this.cppSemantics = cppSemantics; this.additionalIncludeScannables = ImmutableList.copyOf(additionalIncludeScannables); this.builtInIncludeDirectories = diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java index c5fe8c2466..a079119ab5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java @@ -42,6 +42,7 @@ import java.util.List; import java.util.Map; import java.util.UUID; import java.util.function.Consumer; +import javax.annotation.Nullable; /** * Builder class to construct C++ compile actions. @@ -81,6 +82,8 @@ public class CppCompileActionBuilder { private CcToolchainProvider ccToolchain; private final ImmutableMap<String, String> localShellEnvironment; private final boolean codeCoverageEnabled; + @Nullable private String actionName; + private ImmutableList<Artifact> builtinIncludeFiles; // New fields need to be added to the copy constructor. /** @@ -173,6 +176,7 @@ public class CppCompileActionBuilder { this.codeCoverageEnabled = other.codeCoverageEnabled; this.cppSemantics = other.cppSemantics; this.ccToolchain = other.ccToolchain; + this.actionName = other.actionName; } public PathFragment getTempOutputFile() { @@ -215,6 +219,9 @@ public class CppCompileActionBuilder { } private String getActionName() { + if (actionName != null) { + return actionName; + } PathFragment sourcePath = sourceFile.getExecPath(); if (CppFileTypes.CPP_MODULE_MAP.matches(sourcePath)) { return CppCompileAction.CPP_MODULE_COMPILE; @@ -356,6 +363,7 @@ public class CppCompileActionBuilder { usePic, useHeaderModules, realMandatoryInputs, + getBuiltinIncludeFiles(), prunableInputs, outputFile, tempOutputFile, @@ -382,6 +390,7 @@ public class CppCompileActionBuilder { usePic, useHeaderModules, realMandatoryInputs, + getBuiltinIncludeFiles(), prunableInputs, outputFile, dotdFile, @@ -410,13 +419,22 @@ public class CppCompileActionBuilder { return action; } + private ImmutableList<Artifact> getBuiltinIncludeFiles() { + ImmutableList.Builder<Artifact> result = ImmutableList.builder(); + result.addAll(ccToolchain.getBuiltinIncludeFiles()); + if (builtinIncludeFiles != null) { + result.addAll(builtinIncludeFiles); + } + return result.build(); + } + /** * Returns the list of mandatory inputs for the {@link CppCompileAction} as configured. */ NestedSet<Artifact> buildMandatoryInputs() { NestedSetBuilder<Artifact> realMandatoryInputsBuilder = NestedSetBuilder.compileOrder(); realMandatoryInputsBuilder.addTransitive(mandatoryInputsBuilder.build()); - realMandatoryInputsBuilder.addAll(ccToolchain.getBuiltinIncludeFiles()); + realMandatoryInputsBuilder.addAll(getBuiltinIncludeFiles()); realMandatoryInputsBuilder.addAll(context.getTransitiveCompilationPrerequisites()); if (useHeaderModules() && !shouldPruneModules()) { realMandatoryInputsBuilder.addTransitive(context.getTransitiveModules(usePic)); @@ -476,6 +494,15 @@ public class CppCompileActionBuilder { } /** + * Set action name that is used to pick the right action_config and features from {@link + * FeatureConfiguration}. By default the action name is decided from the source filetype. + */ + public CppCompileActionBuilder setActionName(String actionName) { + this.actionName = actionName; + return this; + } + + /** * Sets the feature configuration to be used for the action. */ public CppCompileActionBuilder setFeatureConfiguration( @@ -664,4 +691,10 @@ public class CppCompileActionBuilder { this.coptsFilter = Preconditions.checkNotNull(coptsFilter); return this; } + + public CppCompileActionBuilder setBuiltinIncludeFiles( + ImmutableList<Artifact> builtinIncludeFiles) { + this.builtinIncludeFiles = builtinIncludeFiles; + return this; + } } 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 7fca555b30..9392ffafe6 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 @@ -590,13 +590,10 @@ public class CppHelper { } /** - * Resolves the linkstamp collection from the {@code CcLinkParams} into a map. - * - * <p>Emits a warning on the rule if there are identical linkstamp artifacts with different + * Emits a warning on the rule if there are identical linkstamp artifacts with different * compilation contexts. */ - public static Map<Artifact, NestedSet<Artifact>> resolveLinkstamps( - RuleErrorConsumer listener, CcLinkParams linkParams) { + public static void checkLinkstampsUnique(RuleErrorConsumer listener, CcLinkParams linkParams) { Map<Artifact, NestedSet<Artifact>> result = new LinkedHashMap<>(); for (Linkstamp pair : linkParams.getLinkstamps()) { Artifact artifact = pair.getArtifact(); @@ -606,7 +603,6 @@ public class CppHelper { } result.put(artifact, pair.getDeclaredIncludeSrcs()); } - return result; } public static void addTransitiveLipoInfoForCommonAttributes( 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 644a1b3298..3cc3791e02 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 @@ -47,6 +47,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.CcLinkParams.Linkstamp; 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; @@ -57,7 +58,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; -import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import javax.annotation.Nullable; @@ -110,6 +110,7 @@ public final class CppLinkAction extends AbstractAction private final ImmutableMap<String, String> actionEnv; private final ImmutableMap<String, String> toolchainEnv; private final ImmutableSet<String> executionRequirements; + private final ImmutableList<Artifact> linkstampObjects; private final LinkCommandLine linkCommandLine; @@ -158,6 +159,7 @@ public final class CppLinkAction extends AbstractAction LibraryToLink interfaceOutputLibrary, boolean fake, boolean isLtoIndexing, + ImmutableList<Artifact> linkstampObjects, LinkCommandLine linkCommandLine, ImmutableSet<String> clientEnvironmentVariables, ImmutableMap<String, String> actionEnv, @@ -177,6 +179,7 @@ public final class CppLinkAction extends AbstractAction this.interfaceOutputLibrary = interfaceOutputLibrary; this.fake = fake; this.isLtoIndexing = isLtoIndexing; + this.linkstampObjects = linkstampObjects; this.linkCommandLine = linkCommandLine; this.clientEnvironmentVariables = clientEnvironmentVariables; this.actionEnv = actionEnv; @@ -288,14 +291,13 @@ public final class CppLinkAction extends AbstractAction } /** - * Returns a (possibly empty) mapping of (C++ source file, .o output file) pairs for source files - * that need to be compiled at link time. + * Returns a (possibly empty) list of linkstamp object files. * * <p>This is used to embed various values from the build system into binaries to identify their * provenance. */ - public ImmutableMap<Artifact, Artifact> getLinkstamps() { - return linkCommandLine.getLinkstamps(); + public ImmutableList<Artifact> getLinkstampObjects() { + return linkstampObjects; } @Override @@ -332,14 +334,6 @@ public final class CppLinkAction extends AbstractAction @ThreadCompatible private void executeFake() throws ActionExecutionException { - // The uses of getLinkConfiguration in this method may not be consistent with the computed key. - // I.e., this may be incrementally incorrect. - final Collection<Artifact> linkstampOutputs = getLinkstamps().values(); - - // Prefix all fake output files in the command line with $TEST_TMPDIR/. - final String outputPrefix = "$TEST_TMPDIR/"; - List<String> escapedLinkArgv = escapeLinkArgv(linkCommandLine.getRawLinkArgv(), - linkstampOutputs, outputPrefix); // Write the commands needed to build the real target to the fake target // file. StringBuilder s = new StringBuilder(); @@ -365,22 +359,13 @@ public final class CppLinkAction extends AbstractAction } s.append(getOutputFile().getBaseName()).append(": "); - for (Artifact linkstamp : linkstampOutputs) { - s.append("mkdir -p " + outputPrefix - + linkstamp.getExecPath().getParentDirectory() + " && "); - } - Joiner.on(' ').appendTo(s, - ShellEscaper.escapeAll(linkCommandLine.finalizeAlreadyEscapedWithLinkstampCommands( - escapedLinkArgv, outputPrefix))); + Joiner.on(' ').appendTo(s, ShellEscaper.escapeAll(linkCommandLine.getRawLinkArgv())); s.append('\n'); if (getOutputFile().exists()) { getOutputFile().setWritable(true); // (IOException) } FileSystemUtils.writeContent(getOutputFile(), ISO_8859_1, s.toString()); getOutputFile().setExecutable(true); // (IOException) - for (Artifact linkstamp : linkstampOutputs) { - FileSystemUtils.touchFile(linkstamp.getPath()); - } } catch (IOException e) { throw new ActionExecutionException("failed to create fake link command for rule '" + getOwner().getLabel() + ": " + e.getMessage(), @@ -388,34 +373,6 @@ public final class CppLinkAction extends AbstractAction } } - /** - * Shell-escapes the raw link command line. - * - * @param rawLinkArgv raw link command line - * @param linkstampOutputs linkstamp artifacts - * @param outputPrefix to be prepended to any outputs - * @return escaped link command line - */ - private List<String> escapeLinkArgv(List<String> rawLinkArgv, - final Collection<Artifact> linkstampOutputs, final String outputPrefix) { - final List<String> linkstampExecPaths = Artifact.asExecPaths(linkstampOutputs); - ImmutableList.Builder<String> escapedArgs = ImmutableList.builder(); - for (String rawArg : rawLinkArgv) { - String escapedArg; - if (rawArg.equals(getPrimaryOutput().getExecPathString()) - || linkstampExecPaths.contains(rawArg)) { - escapedArg = outputPrefix + ShellEscaper.escapeString(rawArg); - } else if (rawArg.startsWith(Link.FAKE_OBJECT_PREFIX)) { - escapedArg = outputPrefix + ShellEscaper.escapeString( - rawArg.substring(Link.FAKE_OBJECT_PREFIX.length())); - } else { - escapedArg = ShellEscaper.escapeString(rawArg); - } - escapedArgs.add(escapedArg); - } - return escapedArgs.build(); - } - @Override public ExtraActionInfo.Builder getExtraActionInfo(ActionKeyContext actionKeyContext) { // The uses of getLinkConfiguration in this method may not be consistent with the computed key. @@ -431,7 +388,7 @@ public final class CppLinkAction extends AbstractAction } info.setLinkTargetType(getLinkCommandLine().getLinkTargetType().name()); info.setLinkStaticness(getLinkCommandLine().getLinkStaticness().name()); - info.addAllLinkStamp(Artifact.toExecPaths(getLinkstamps().values())); + info.addAllLinkStamp(Artifact.toExecPaths(getLinkstampObjects())); info.addAllBuildInfoHeaderArtifact(Artifact.toExecPaths(getBuildInfoHeaderArtifacts())); info.addAllLinkOpt(getLinkCommandLine().getRawLinkArgv()); @@ -563,8 +520,7 @@ public final class CppLinkAction extends AbstractAction final Artifact runtimeMiddleman; final NestedSet<Artifact> runtimeInputs; final ArtifactCategory runtimeType; - final NestedSet<Artifact> compilationInputs; - final ImmutableSet<Artifact> linkstamps; + final ImmutableSet<Linkstamp> linkstamps; final ImmutableList<String> linkopts; final LinkTargetType linkType; final LinkStaticness linkStaticness; @@ -590,9 +546,7 @@ public final class CppLinkAction extends AbstractAction this.runtimeInputs = NestedSetBuilder.<Artifact>stableOrder().addTransitive(builder.getRuntimeInputs()).build(); this.runtimeType = builder.getRuntimeType(); - this.compilationInputs = NestedSetBuilder.<Artifact>stableOrder() - .addTransitive(builder.getCompilationInputs().build()).build(); - this.linkstamps = ImmutableSet.copyOf(builder.getLinkstamps()); + this.linkstamps = builder.getLinkstamps(); this.linkopts = ImmutableList.copyOf(builder.getLinkopts()); this.linkType = builder.getLinkType(); this.linkStaticness = builder.getLinkStaticness(); @@ -635,21 +589,12 @@ public final class CppLinkAction extends AbstractAction public NestedSet<Artifact> getRuntimeInputs() { return this.runtimeInputs; } - - /** - * Returns compilation inputs for compilations arising from the linking of this target. - */ - public NestedSet<Artifact> getCompilationInputs() { - return this.compilationInputs; - } - /** - * Returns linkstamp artifacts. - */ - public ImmutableSet<Artifact> getLinkstamps() { + /** Returns linkstamp artifacts. */ + public ImmutableSet<Linkstamp> getLinkstamps() { return this.linkstamps; } - + /** * Returns linkopts for the linking of this target. */ 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 a546292c2d..52fdab4b83 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 @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.analysis.AnalysisEnvironment; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.CollectionUtils; import com.google.devtools.build.lib.collect.ImmutableIterable; import com.google.devtools.build.lib.collect.IterablesChain; @@ -39,6 +40,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.RuleErrorConsumer; +import com.google.devtools.build.lib.rules.cpp.CcLinkParams.Linkstamp; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.LibraryToLinkValue; @@ -58,7 +60,9 @@ import java.util.HashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; +import java.util.regex.Matcher; import javax.annotation.Nullable; /** Builder class to construct {@link CppLinkAction}s. */ @@ -160,7 +164,7 @@ public class CppLinkActionBuilder { @Nullable private final RuleContext ruleContext; private final AnalysisEnvironment analysisEnvironment; private final Artifact output; - private final CppSemantics unusedCppSemantics; + private final CppSemantics cppSemantics; @Nullable private String mnemonic; // can be null for CppLinkAction.createTestBuilder() @@ -182,8 +186,7 @@ public class CppLinkActionBuilder { private Artifact runtimeMiddleman; private ArtifactCategory runtimeType = null; private NestedSet<Artifact> runtimeInputs = NestedSetBuilder.emptySet(Order.STABLE_ORDER); - private final NestedSetBuilder<Artifact> compilationInputs = NestedSetBuilder.stableOrder(); - private final Set<Artifact> linkstamps = new LinkedHashSet<>(); + private final ImmutableSet.Builder<Linkstamp> linkstampsBuilder = ImmutableSet.builder(); private ImmutableList<String> additionalLinkstampDefines = ImmutableList.of(); private final List<String> linkopts = new ArrayList<>(); private LinkTargetType linkType = LinkTargetType.STATIC_LIBRARY; @@ -293,7 +296,7 @@ public class CppLinkActionBuilder { runtimeSolibDir = toolchain.getDynamicRuntimeSolibDir(); } this.featureConfiguration = featureConfiguration; - this.unusedCppSemantics = Preconditions.checkNotNull(cppSemantics); + this.cppSemantics = Preconditions.checkNotNull(cppSemantics); } /** @@ -339,8 +342,7 @@ public class CppLinkActionBuilder { this.runtimeMiddleman = linkContext.runtimeMiddleman; this.runtimeInputs = linkContext.runtimeInputs; this.runtimeType = linkContext.runtimeType; - this.compilationInputs.addTransitive(linkContext.compilationInputs); - this.linkstamps.addAll(linkContext.linkstamps); + this.linkstampsBuilder.addAll(linkContext.linkstamps); this.linkopts.addAll(linkContext.linkopts); this.linkType = linkContext.linkType; this.linkStaticness = linkContext.linkStaticness; @@ -395,18 +397,9 @@ public class CppLinkActionBuilder { return runtimeType; } - /** - * Returns compilation inputs for this link action. - */ - public final NestedSetBuilder<Artifact> getCompilationInputs() { - return this.compilationInputs; - } - - /** - * Returns linkstamps for this link action. - */ - public final Set<Artifact> getLinkstamps() { - return this.linkstamps; + /** Returns linkstamps for this link action. */ + public final ImmutableSet<Linkstamp> getLinkstamps() { + return linkstampsBuilder.build(); } /** @@ -733,6 +726,9 @@ public class CppLinkActionBuilder { boolean isExecutable = linkType.isExecutable(); Preconditions.checkState(hasIdentifier != isExecutable); Preconditions.checkNotNull(featureConfiguration); + ImmutableSet<Linkstamp> linkstamps = linkstampsBuilder.build(); + final ImmutableMap<Linkstamp, Artifact> linkstampMap = + mapLinkstampsToOutputs(linkstamps, ruleContext, configuration, output, linkArtifactFactory); if (interfaceOutput != null && (fake || linkType != LinkTargetType.DYNAMIC_LIBRARY)) { throw new RuntimeException( @@ -800,7 +796,12 @@ public class CppLinkActionBuilder { objectFileInputs = computeLtoIndexingObjectFileInputs(); uniqueLibraries = computeLtoIndexingUniqueLibraries(originalUniqueLibraries); } else { - objectFileInputs = ImmutableSet.copyOf(objectFiles); + ImmutableSet.Builder<LinkerInput> builder = + ImmutableSet.<LinkerInput>builder().addAll(objectFiles); + builder.addAll( + LinkerInputs.simpleLinkerInputs(linkstampMap.values(), ArtifactCategory.OBJECT_FILE)); + + objectFileInputs = builder.build(); uniqueLibraries = originalUniqueLibraries; } final Iterable<Artifact> objectArtifacts = LinkerInputs.toLibraryArtifacts(objectFileInputs); @@ -837,9 +838,6 @@ public class CppLinkActionBuilder { ltoBitcodeFiles, /* sharedNonLtoBackends= */ null); - final ImmutableMap<Artifact, Artifact> linkstampMap = - mapLinkstampsToOutputs(linkstamps, ruleContext, configuration, output, linkArtifactFactory); - @Nullable Artifact thinltoParamFile = null; if (allowLtoIndexing && allLtoArtifacts != null) { // Create artifact for the file that the LTO indexing step will emit @@ -869,7 +867,7 @@ public class CppLinkActionBuilder { actionOutputs = constructOutputs( output, - Iterables.concat(linkstampMap.values(), linkActionOutputs.build()), + linkActionOutputs.build(), interfaceOutputLibrary == null ? null : interfaceOutputLibrary.getArtifact(), symbolCounts); } @@ -893,7 +891,6 @@ public class CppLinkActionBuilder { isLtoIndexing ? new CppLinkVariablesExtension( configuration, - /* linkstampMap= */ ImmutableMap.of(), needWholeArchive, linkerInputs, runtimeLinkerInputs, @@ -907,7 +904,6 @@ public class CppLinkActionBuilder { /* interfaceLibraryOutput= */ null) : new CppLinkVariablesExtension( configuration, - linkstampMap, needWholeArchive, linkerInputs, runtimeLinkerInputs, @@ -947,7 +943,7 @@ public class CppLinkActionBuilder { } LinkCommandLine.Builder linkCommandLineBuilder = - new LinkCommandLine.Builder(configuration, getOwner(), ruleContext) + new LinkCommandLine.Builder(configuration, ruleContext) .setLinkerInputs(linkerInputs) .setRuntimeInputs(runtimeLinkerInputs) .setLinkTargetType(linkType) @@ -958,7 +954,6 @@ public class CppLinkActionBuilder { .setUseTestOnlyFlags(useTestOnlyFlags) .setParamFile(paramFile) .setToolchain(toolchain) - .setFdoSupport(fdoSupport.getFdoSupport()) .setBuildVariables(buildVariables) .setFeatureConfiguration(featureConfiguration); @@ -970,11 +965,8 @@ public class CppLinkActionBuilder { if (!isLtoIndexing) { linkCommandLineBuilder - .setOutput(output) .setBuildInfoHeaderArtifacts(buildInfoHeaderArtifacts) - .setLinkstamps(linkstampMap) - .setLinkopts(ImmutableList.copyOf(linkopts)) - .setAdditionalLinkstampDefines(additionalLinkstampDefines); + .setLinkopts(ImmutableList.copyOf(linkopts)); } else { List<String> opts = new ArrayList<>(linkopts); opts.addAll(featureConfiguration.getCommandLine("lto-indexing", buildVariables)); @@ -984,6 +976,30 @@ public class CppLinkActionBuilder { LinkCommandLine linkCommandLine = linkCommandLineBuilder.build(); + for (Entry<Linkstamp, Artifact> linkstampEntry : linkstampMap.entrySet()) { + analysisEnvironment.registerAction( + CppLinkstampCompileHelper.createLinkstampCompileAction( + ruleContext, + linkstampEntry.getKey().getArtifact(), + linkstampEntry.getValue(), + linkstampEntry.getKey().getDeclaredIncludeSrcs(), + ImmutableSet.copyOf(nonCodeInputs), + buildInfoHeaderArtifacts, + additionalLinkstampDefines, + toolchain, + configuration.isCodeCoverageEnabled(), + cppConfiguration, + CppHelper.getFdoBuildStamp(ruleContext, fdoSupport.getFdoSupport()), + featureConfiguration, + linkType == LinkTargetType.DYNAMIC_LIBRARY && toolchain.toolchainNeedsPic(), + Matcher.quoteReplacement( + isNativeDeps && cppConfiguration.shareNativeDeps() + ? output.getExecPathString() + : Label.print(getOwner().getLabel())), + Matcher.quoteReplacement(output.getExecPathString()), + cppSemantics)); + } + // Compute the set of inputs - we only need stable order here. NestedSetBuilder<Artifact> dependencyInputsBuilder = NestedSetBuilder.stableOrder(); dependencyInputsBuilder.addTransitive(crosstoolInputs); @@ -996,9 +1012,7 @@ public class CppLinkActionBuilder { dependencyInputsBuilder.add(runtimeMiddleman); } if (!isLtoIndexing) { - dependencyInputsBuilder.addAll(buildInfoHeaderArtifacts); - dependencyInputsBuilder.addAll(linkstamps); - dependencyInputsBuilder.addTransitive(compilationInputs.build()); + dependencyInputsBuilder.addAll(linkstampMap.values()); } if (defFile != null) { dependencyInputsBuilder.add(defFile); @@ -1084,6 +1098,11 @@ public class CppLinkActionBuilder { interfaceOutputLibrary, fake, isLtoIndexing, + linkstampMap + .keySet() + .stream() + .map(Linkstamp::getArtifact) + .collect(ImmutableList.toImmutableList()), linkCommandLine, configuration.getVariableShellEnvironment(), configuration.getLocalShellEnvironment(), @@ -1126,23 +1145,23 @@ public class CppLinkActionBuilder { } /** - * Translates a collection of linkstamp source files to an immutable mapping from source files to - * object files. In other words, given a set of source files, this method determines the output + * Translates a collection of {@link Linkstamp} instances to an immutable mapping from linkstamp + * to object files. In other words, given a set of source files, this method determines the output * path to which each file should be compiled. * - * @param linkstamps collection of linkstamp source files + * @param linkstamps set of {@link Linkstamp}s * @param ruleContext the rule for which this link is being performed * @param outputBinary the binary output path for this link * @return an immutable map that pairs each source file with the corresponding object file that * should be fed into the link */ - public static ImmutableMap<Artifact, Artifact> mapLinkstampsToOutputs( - Collection<Artifact> linkstamps, + public static ImmutableMap<Linkstamp, Artifact> mapLinkstampsToOutputs( + ImmutableSet<Linkstamp> linkstamps, RuleContext ruleContext, BuildConfiguration configuration, Artifact outputBinary, LinkArtifactFactory linkArtifactFactory) { - ImmutableMap.Builder<Artifact, Artifact> mapBuilder = ImmutableMap.builder(); + ImmutableMap.Builder<Linkstamp, Artifact> mapBuilder = ImmutableMap.builder(); PathFragment outputBinaryPath = outputBinary.getRootRelativePath(); PathFragment stampOutputDirectory = @@ -1151,10 +1170,11 @@ public class CppLinkActionBuilder { .getRelative(CppHelper.OBJS) .getRelative(outputBinaryPath.getBaseName()); - for (Artifact linkstamp : linkstamps) { + for (Linkstamp linkstamp : linkstamps) { PathFragment stampOutputPath = stampOutputDirectory.getRelative( - FileSystemUtils.replaceExtension(linkstamp.getRootRelativePath(), ".o")); + FileSystemUtils.replaceExtension( + linkstamp.getArtifact().getRootRelativePath(), ".o")); mapBuilder.put( linkstamp, // Note that link stamp actions can be shared between link actions that output shared @@ -1240,20 +1260,6 @@ public class CppLinkActionBuilder { return this; } - /** - * Add additional inputs needed for the linkstamp compilation that is being done as part of the - * link. - */ - public CppLinkActionBuilder addCompilationInputs(Iterable<Artifact> inputs) { - this.compilationInputs.addAll(inputs); - return this; - } - - public CppLinkActionBuilder addTransitiveCompilationInputs(NestedSet<Artifact> inputs) { - this.compilationInputs.addTransitive(inputs); - return this; - } - private void addObjectFile(LinkerInput input) { // We skip file extension checks for TreeArtifacts because they represent directory artifacts // without a file extension. @@ -1385,19 +1391,15 @@ public class CppLinkActionBuilder { } /** - * Adds a C++ source file which will be compiled at link time. This is used to embed various - * values from the build system into binaries to identify their provenance. + * Adds {@link Linkstamp}s. + * + * <p>This is used to embed various values from the build system into binaries to identify their + * provenance. * - * <p>Link stamps are also automatically added to the inputs. + * <p>Linkstamp object files are also automatically added to the inputs of the link action. */ - public CppLinkActionBuilder addLinkstamps(Map<Artifact, NestedSet<Artifact>> linkstamps) { - this.linkstamps.addAll(linkstamps.keySet()); - // Add inputs for linkstamping. - if (!linkstamps.isEmpty()) { - for (Map.Entry<Artifact, NestedSet<Artifact>> entry : linkstamps.entrySet()) { - addCompilationInputs(entry.getValue()); - } - } + public CppLinkActionBuilder addLinkstamps(Iterable<Linkstamp> linkstamps) { + this.linkstampsBuilder.addAll(linkstamps); return this; } @@ -1437,7 +1439,8 @@ public class CppLinkActionBuilder { addLibraries(extraLibrary.buildLibraries(ruleContext)); } } - addLinkstamps(CppHelper.resolveLinkstamps(errorListener, linkParams)); + CppHelper.checkLinkstampsUnique(errorListener, linkParams); + addLinkstamps(linkParams.getLinkstamps()); return this; } @@ -1553,7 +1556,6 @@ public class CppLinkActionBuilder { private class CppLinkVariablesExtension implements VariablesExtension { private final BuildConfiguration configuration; - private final ImmutableMap<Artifact, Artifact> linkstampMap; private final boolean needWholeArchive; private final Iterable<LinkerInput> linkerInputs; private final ImmutableList<LinkerInput> runtimeLinkerInputs; @@ -1569,7 +1571,6 @@ public class CppLinkActionBuilder { public CppLinkVariablesExtension( BuildConfiguration configuration, - ImmutableMap<Artifact, Artifact> linkstampMap, boolean needWholeArchive, Iterable<LinkerInput> linkerInputs, ImmutableList<LinkerInput> runtimeLinkerInputs, @@ -1581,7 +1582,6 @@ public class CppLinkActionBuilder { Artifact interfaceLibraryBuilder, Artifact interfaceLibraryOutput) { this.configuration = configuration; - this.linkstampMap = linkstampMap; this.needWholeArchive = needWholeArchive; this.linkerInputs = linkerInputs; this.runtimeLinkerInputs = runtimeLinkerInputs; @@ -1605,15 +1605,6 @@ public class CppLinkActionBuilder { SYMBOL_COUNTS_OUTPUT_VARIABLE, symbolCounts.getExecPathString()); } - // linkstamp - ImmutableSet.Builder<String> linkstampPaths = ImmutableSet.builder(); - for (Artifact linkstampOutput : linkstampMap.values()) { - linkstampPaths.add(linkstampOutput.getExecPathString()); - } - - buildVariables.addStringSequenceVariable( - LINKSTAMP_PATHS_VARIABLE, linkstampPaths.build()); - // pic if (cppConfiguration.forcePic()) { buildVariables.addStringVariable(FORCE_PIC_VARIABLE, ""); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java new file mode 100644 index 0000000000..dac1517c4b --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java @@ -0,0 +1,186 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.rules.cpp; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableList.Builder; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; +import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables; +import java.util.regex.Pattern; + +/** Handles creation of CppCompileAction used to compile linkstamp sources. */ +public class CppLinkstampCompileHelper { + + /** Creates {@link CppCompileAction} to compile linkstamp source */ + public static CppCompileAction createLinkstampCompileAction( + RuleContext ruleContext, + Artifact sourceFile, + Artifact outputFile, + Iterable<Artifact> compilationInputs, + ImmutableSet<Artifact> nonCodeInputs, + ImmutableList<Artifact> buildInfoHeaderArtifacts, + Iterable<String> additionalLinkstampDefines, + CcToolchainProvider ccToolchainProvider, + boolean codeCoverageEnabled, + CppConfiguration cppConfiguration, + String fdoBuildStamp, + FeatureConfiguration featureConfiguration, + boolean needsPic, + String labelReplacement, + String outputReplacement, + CppSemantics semantics) { + CppCompileActionBuilder builder = + new CppCompileActionBuilder(ruleContext, ccToolchainProvider) + .addMandatoryInputs(compilationInputs) + .setVariables( + getVariables( + sourceFile, + outputFile, + labelReplacement, + outputReplacement, + additionalLinkstampDefines, + buildInfoHeaderArtifacts, + featureConfiguration, + cppConfiguration, + ccToolchainProvider, + needsPic, + fdoBuildStamp, + codeCoverageEnabled)) + .setFeatureConfiguration(featureConfiguration) + .setSourceFile(sourceFile) + .setSemantics(semantics) + .setOutputs(outputFile, null) + .setBuiltinIncludeFiles(buildInfoHeaderArtifacts) + .addMandatoryInputs(nonCodeInputs) + .setCppConfiguration(cppConfiguration) + .setActionName(CppCompileAction.LINKSTAMP_COMPILE); + semantics.finalizeCompileActionBuilder(ruleContext, builder); + return builder.buildOrThrowIllegalStateException(); + } + + private static ImmutableList<String> computeAllLinkstampDefines( + String labelReplacement, + String outputReplacement, + Iterable<String> additionalLinkstampDefines, + CppConfiguration cppConfiguration, + String fdoBuildStamp, + boolean codeCoverageEnabled) { + String labelPattern = Pattern.quote("${LABEL}"); + String outputPathPattern = Pattern.quote("${OUTPUT_PATH}"); + Builder<String> defines = + ImmutableList.<String>builder() + .add("GPLATFORM=\"" + cppConfiguration + "\"") + .add("BUILD_COVERAGE_ENABLED=" + (codeCoverageEnabled ? "1" : "0")) + .add(CppConfiguration.FDO_STAMP_MACRO + "=\"" + fdoBuildStamp + "\"") + // G3_VERSION_INFO and G3_TARGET_NAME are C string literals that normally + // contain the label of the target being linked. However, they are set + // differently when using shared native deps. In that case, a single .so file + // is shared by multiple targets, and its contents cannot depend on which + // target(s) were specified on the command line. So in that case we have + // to use the (obscure) name of the .so file instead, or more precisely + // the path of the .so file relative to the workspace root. + .add("G3_VERSION_INFO=\"${LABEL}\"") + .add("G3_TARGET_NAME=\"${LABEL}\"") + // G3_BUILD_TARGET is a C string literal containing the output of this + // link. (An undocumented and untested invariant is that G3_BUILD_TARGET is the + // location of the executable, either absolutely, or relative to the directory part of + // BUILD_INFO.) + .add("G3_BUILD_TARGET=\"${OUTPUT_PATH}\"") + .addAll(additionalLinkstampDefines); + + if (fdoBuildStamp != null) { + defines.add(CppConfiguration.FDO_STAMP_MACRO + "=\"" + fdoBuildStamp + "\""); + } + + return defines + .build() + .stream() + .map( + define -> + define + .replaceAll(labelPattern, labelReplacement) + .replaceAll(outputPathPattern, outputReplacement)) + .collect(ImmutableList.toImmutableList()); + } + + private static Variables getVariables( + Artifact sourceFile, + Artifact outputFile, + String labelReplacement, + String outputReplacement, + Iterable<String> additionalLinkstampDefines, + ImmutableList<Artifact> buildInfoHeaderArtifacts, + FeatureConfiguration featureConfiguration, + CppConfiguration cppConfiguration, + CcToolchainProvider ccToolchainProvider, + boolean needsPic, + String fdoBuildStamp, + boolean codeCoverageEnabled) { + // TODO(b/34761650): Remove all this hardcoding by separating a full blown compile action. + Preconditions.checkArgument( + featureConfiguration.actionIsConfigured(CppCompileAction.LINKSTAMP_COMPILE)); + + Variables.Builder variables = new Variables.Builder(ccToolchainProvider.getBuildVariables()); + // We need to force inclusion of build_info headers + variables.addStringSequenceVariable( + CppModel.INCLUDES_VARIABLE_NAME, + buildInfoHeaderArtifacts + .stream() + .map(Artifact::getExecPathString) + .collect(ImmutableList.toImmutableList())); + // Input/Output files. + variables.addStringVariable(CppModel.SOURCE_FILE_VARIABLE_NAME, sourceFile.getExecPathString()); + variables.addStringVariable(CppModel.OUTPUT_FILE_VARIABLE_NAME, outputFile.getExecPathString()); + variables.addStringVariable( + CppModel.OUTPUT_OBJECT_FILE_VARIABLE_NAME, outputFile.getExecPathString()); + // Include directories for (normal includes with ".", empty quote- and system- includes). + variables.addStringSequenceVariable( + CppModel.INCLUDE_PATHS_VARIABLE_NAME, ImmutableList.of(".")); + variables.addStringSequenceVariable( + CppModel.QUOTE_INCLUDE_PATHS_VARIABLE_NAME, ImmutableList.of()); + variables.addStringSequenceVariable( + CppModel.SYSTEM_INCLUDE_PATHS_VARIABLE_NAME, ImmutableList.of()); + // Legacy flags coming from fields such as compiler_flag + variables.addLazyStringSequenceVariable( + CppModel.LEGACY_COMPILE_FLAGS_VARIABLE_NAME, + CppModel.getLegacyCompileFlagsSupplier( + cppConfiguration, sourceFile.getExecPathString(), ImmutableSet.of())); + // Unfilterable flags coming from unfiltered_cxx_flag fields + variables.addLazyStringSequenceVariable( + CppModel.UNFILTERED_COMPILE_FLAGS_VARIABLE_NAME, + CppModel.getUnfilteredCompileFlagsSupplier(ccToolchainProvider, ImmutableSet.of())); + // Collect all preprocessor defines, and in each replace ${LABEL} by labelReplacements, and + // ${OUTPUT_PATH} with outputPathReplacement. + variables.addStringSequenceVariable( + CppModel.PREPROCESSOR_DEFINES_VARIABLE_NAME, + computeAllLinkstampDefines( + labelReplacement, + outputReplacement, + additionalLinkstampDefines, + cppConfiguration, + fdoBuildStamp, + codeCoverageEnabled)); + // For dynamic libraries, produce position independent code. + if (needsPic) { + variables.addStringVariable(CppModel.PIC_VARIABLE_NAME, ""); + } + + return variables.build(); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java index 3fa6091dc2..46ef0c6c4a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java @@ -66,6 +66,7 @@ public class FakeCppCompileAction extends CppCompileAction { boolean usePic, boolean useHeaderModules, NestedSet<Artifact> mandatoryInputs, + ImmutableList<Artifact> builtinIncludeFiles, NestedSet<Artifact> prunableInputs, Artifact outputFile, PathFragment tempOutputFile, @@ -90,6 +91,7 @@ public class FakeCppCompileAction extends CppCompileAction { usePic, useHeaderModules, mandatoryInputs, + builtinIncludeFiles, prunableInputs, outputFile, dotdFile, diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java index 4556a08772..e1cde74d3d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java @@ -15,20 +15,15 @@ package com.google.devtools.build.lib.rules.cpp; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; -import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.actions.CommandLine; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; -import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.CollectionUtils; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; @@ -37,13 +32,9 @@ 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.Link.Staticness; import com.google.devtools.build.lib.util.Pair; -import com.google.devtools.build.lib.util.ShellEscaper; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.List; -import java.util.Map; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import javax.annotation.Nullable; /** @@ -54,13 +45,10 @@ import javax.annotation.Nullable; public final class LinkCommandLine extends CommandLine { private final String actionName; private final String forcedToolPath; - private final boolean codeCoverageEnabled; private final CppConfiguration cppConfiguration; - private final ActionOwner owner; private final CcToolchainFeatures.Variables variables; // The feature config can be null for tests. @Nullable private final FeatureConfiguration featureConfiguration; - @Nullable private final Artifact output; private final ImmutableList<Artifact> buildInfoHeaderArtifacts; private final Iterable<? extends LinkerInput> linkerInputs; private final Iterable<? extends LinkerInput> runtimeInputs; @@ -68,9 +56,6 @@ public final class LinkCommandLine extends CommandLine { private final LinkStaticness linkStaticness; private final ImmutableList<String> linkopts; private final ImmutableSet<String> features; - private final ImmutableMap<Artifact, Artifact> linkstamps; - private final ImmutableList<String> additionalLinkstampDefines; - @Nullable private final String fdoBuildStamp; @Nullable private final PathFragment runtimeSolibDir; private final boolean nativeDeps; private final boolean useTestOnlyFlags; @@ -82,8 +67,6 @@ public final class LinkCommandLine extends CommandLine { String actionName, String forcedToolPath, BuildConfiguration configuration, - ActionOwner owner, - Artifact output, ImmutableList<Artifact> buildInfoHeaderArtifacts, Iterable<? extends LinkerInput> linkerInputs, Iterable<? extends LinkerInput> runtimeInputs, @@ -91,9 +74,6 @@ public final class LinkCommandLine extends CommandLine { LinkStaticness linkStaticness, ImmutableList<String> linkopts, ImmutableSet<String> features, - ImmutableMap<Artifact, Artifact> linkstamps, - ImmutableList<String> additionalLinkstampDefines, - @Nullable String fdoBuildStamp, @Nullable PathFragment runtimeSolibDir, boolean nativeDeps, boolean useTestOnlyFlags, @@ -104,12 +84,9 @@ public final class LinkCommandLine extends CommandLine { this.actionName = actionName; this.forcedToolPath = forcedToolPath; - this.codeCoverageEnabled = configuration.isCodeCoverageEnabled(); this.cppConfiguration = configuration.getFragment(CppConfiguration.class); this.variables = variables; this.featureConfiguration = featureConfiguration; - this.owner = Preconditions.checkNotNull(owner); - this.output = output; this.buildInfoHeaderArtifacts = Preconditions.checkNotNull(buildInfoHeaderArtifacts); this.linkerInputs = Preconditions.checkNotNull(linkerInputs); this.runtimeInputs = Preconditions.checkNotNull(runtimeInputs); @@ -121,9 +98,6 @@ public final class LinkCommandLine extends CommandLine { ? ImmutableList.of() : Preconditions.checkNotNull(linkopts); this.features = Preconditions.checkNotNull(features); - this.linkstamps = Preconditions.checkNotNull(linkstamps); - this.additionalLinkstampDefines = additionalLinkstampDefines; - this.fdoBuildStamp = fdoBuildStamp; this.runtimeSolibDir = runtimeSolibDir; this.nativeDeps = nativeDeps; this.useTestOnlyFlags = useTestOnlyFlags; @@ -176,11 +150,6 @@ public final class LinkCommandLine extends CommandLine { return linkopts; } - /** See {@link CppLinkAction#getLinkstamps()} */ - protected ImmutableMap<Artifact, Artifact> getLinkstamps() { - return linkstamps; - } - /** * Returns the location of the C++ runtime solib symlinks. If null, the C++ dynamic runtime * libraries either do not exist (because they do not come from the depot) or they are in the @@ -373,8 +342,7 @@ public final class LinkCommandLine extends CommandLine { /** * Returns a raw link command for the given link invocation, including both command and arguments - * (argv). After any further usage-specific processing, this can be passed to {@link - * #finalizeWithLinkstampCommands} to give the final command line. + * (argv). * * @return raw link command line. */ @@ -403,252 +371,27 @@ public final class LinkCommandLine extends CommandLine { } List<String> getCommandLine() { - List<String> commandlineArgs; // Try to shorten the command line by use of a parameter file. // This makes the output with --subcommands (et al) more readable. if (paramFile != null) { Pair<List<String>, List<String>> split = splitCommandline(); - commandlineArgs = split.first; + return split.first; } else { - commandlineArgs = getRawLinkArgv(); + return getRawLinkArgv(); } - return finalizeWithLinkstampCommands(commandlineArgs); } @Override public List<String> arguments() { - return finalizeWithLinkstampCommands(getRawLinkArgv()); - } - - /** - * Takes a raw link command line and gives the final link command that will - * also first compile any linkstamps necessary. Elements of rawLinkArgv are - * shell-escaped. - * - * @param rawLinkArgv raw link command line - * - * @return final link command line suitable for execution - */ - public List<String> finalizeWithLinkstampCommands(List<String> rawLinkArgv) { - return addLinkstampingToCommand(getLinkstampCompileCommands(""), rawLinkArgv, true); - } - - /** - * Takes a raw link command line and gives the final link command that will also first compile any - * linkstamps necessary. Elements of rawLinkArgv are not shell-escaped. - * - * @param rawLinkArgv raw link command line - * @param outputPrefix prefix to add before the linkstamp outputs' exec paths - * - * @return final link command line suitable for execution - */ - public List<String> finalizeAlreadyEscapedWithLinkstampCommands( - List<String> rawLinkArgv, String outputPrefix) { - return addLinkstampingToCommand(getLinkstampCompileCommands(outputPrefix), rawLinkArgv, false); - } - - /** - * Adds linkstamp compilation to the (otherwise) fully specified link - * command if {@link #getLinkstamps} is non-empty. - * - * <p>Linkstamps were historically compiled implicitly as part of the link - * command, but implicit compilation doesn't guarantee consistent outputs. - * For example, the command "gcc input.o input.o foo/linkstamp.cc -o myapp" - * causes gcc to implicitly run "gcc foo/linkstamp.cc -o /tmp/ccEtJHDB.o", - * for some internally decided output path /tmp/ccEtJHDB.o, then add that path - * to the linker's command line options. The name of this path can change - * even between equivalently specified gcc invocations. - * - * <p>So now we explicitly compile these files in their own command - * invocations before running the link command, thus giving us direct - * control over the naming of their outputs. This method adds those extra - * steps as necessary. - * @param linkstampCommands individual linkstamp compilation commands - * @param linkCommand the complete list of link command arguments (after - * .params file compacting) for an invocation - * @param escapeArgs if true, linkCommand arguments are shell escaped. if - * false, arguments are returned as-is - * - * @return The original argument list if no linkstamps compilation commands - * are given, otherwise an expanded list that adds the linkstamp - * compilation commands and funnels their outputs into the link step. - * Note that these outputs only need to persist for the duration of - * the link step. - */ - private static List<String> addLinkstampingToCommand( - List<String> linkstampCommands, - List<String> linkCommand, - boolean escapeArgs) { - if (linkstampCommands.isEmpty()) { - return linkCommand; - } else { - List<String> batchCommand = Lists.newArrayListWithCapacity(3); - batchCommand.add("/bin/bash"); - batchCommand.add("-c"); - batchCommand.add( - Joiner.on(" && ").join(linkstampCommands) + " && " - + (escapeArgs - ? ShellEscaper.escapeJoinAll(linkCommand) - : Joiner.on(" ").join(linkCommand))); - return ImmutableList.copyOf(batchCommand); - } - } - - private boolean isSharedNativeLibrary() { - return nativeDeps && cppConfiguration.shareNativeDeps(); - } - - /** - * Computes, for each C++ source file in {@link #getLinkstamps}, the command necessary to compile - * that file such that the output is correctly fed into the link command. - * - * <p>As these options (as well as all others) are taken into account when computing the action - * key, they do not directly contain volatile build information to avoid unnecessary relinking. - * Instead this information is passed as an additional header generated by {@link - * com.google.devtools.build.lib.rules.cpp.WriteBuildInfoHeaderAction}. - * - * @param outputPrefix prefix to add before the linkstamp outputs' exec paths - * @return a list of shell-escaped compiler commmands, one for each entry in {@link - * #getLinkstamps} - */ - public List<String> getLinkstampCompileCommands(String outputPrefix) { - if (linkstamps.isEmpty()) { - return ImmutableList.of(); - } - - List<String> commands = Lists.newArrayListWithCapacity(linkstamps.size()); - - for (Map.Entry<Artifact, Artifact> linkstamp : linkstamps.entrySet()) { - Artifact sourceFile = linkstamp.getKey(); - Artifact outputFile = linkstamp.getValue(); - Variables linkstampVariables = collectLinkstampVariables(sourceFile, outputFile); - - ImmutableList.Builder<String> linkstampCompileCommandLine = ImmutableList.builder(); - linkstampCompileCommandLine.add( - featureConfiguration - .getToolForAction(CppCompileAction.LINKSTAMP_COMPILE) - .getToolPath(cppConfiguration.getCrosstoolTopPathFragment()) - .getPathString()); - linkstampCompileCommandLine.addAll( - featureConfiguration.getCommandLine( - CppCompileAction.LINKSTAMP_COMPILE, linkstampVariables)); - // TODO(b/28946988): Remove -c/-o hardcoded flags from bazel - linkstampCompileCommandLine.add("-c"); - linkstampCompileCommandLine.add(sourceFile.getExecPathString()); - linkstampCompileCommandLine.add("-o"); - // outputPrefix is there only for cc_fake_binary, and it can contain env var expansions - // (such as $TEST_TMPDIR) and cannot be escaped. When we move linkstamping to a separate - // action, there will no longer be bash around the invocation and therefore no need to do - // shell escaping. - String escapedCommandWithoutOutput = - ShellEscaper.escapeJoinAll(linkstampCompileCommandLine.build()); - commands.add( - escapedCommandWithoutOutput - + " " - + outputPrefix - + ShellEscaper.escapeString(outputFile.getExecPathString())); - } - - return commands; - } - - private Variables collectLinkstampVariables(Artifact sourceFile, Artifact outputFile) { - // TODO(b/34761650): Remove all this hardcoding by separating a full blown compile action. - Preconditions.checkArgument( - featureConfiguration.actionIsConfigured(CppCompileAction.LINKSTAMP_COMPILE)); - - Variables.Builder linkstampVariables = new Variables.Builder(ccProvider.getBuildVariables()); - // We need to force inclusion of build_info headers - linkstampVariables.addStringSequenceVariable( - CppModel.INCLUDES_VARIABLE_NAME, - buildInfoHeaderArtifacts - .stream() - .map(Artifact::getExecPathString) - .collect(ImmutableList.toImmutableList())); - // Input/Output files. - linkstampVariables.addStringVariable( - CppModel.SOURCE_FILE_VARIABLE_NAME, sourceFile.getExecPathString()); - linkstampVariables.addStringVariable( - CppModel.OUTPUT_OBJECT_FILE_VARIABLE_NAME, outputFile.getExecPathString()); - // Include directories for (normal includes with ".", empty quote- and system- includes). - linkstampVariables.addStringSequenceVariable( - CppModel.INCLUDE_PATHS_VARIABLE_NAME, ImmutableList.of(".")); - linkstampVariables.addStringSequenceVariable( - CppModel.QUOTE_INCLUDE_PATHS_VARIABLE_NAME, ImmutableList.of()); - linkstampVariables.addStringSequenceVariable( - CppModel.SYSTEM_INCLUDE_PATHS_VARIABLE_NAME, ImmutableList.of()); - // Legacy flags coming from fields such as compiler_flag - linkstampVariables.addLazyStringSequenceVariable( - CppModel.LEGACY_COMPILE_FLAGS_VARIABLE_NAME, - CppModel.getLegacyCompileFlagsSupplier( - cppConfiguration, sourceFile.getExecPathString(), features)); - // Unfilterable flags coming from unfiltered_cxx_flag fields - linkstampVariables.addLazyStringSequenceVariable( - CppModel.UNFILTERED_COMPILE_FLAGS_VARIABLE_NAME, - CppModel.getUnfilteredCompileFlagsSupplier(ccProvider, features)); - // Collect all preprocessor defines, and in each replace ${LABEL} by labelReplacements, and - // ${OUTPUT_PATH} with outputPathReplacement. - linkstampVariables.addStringSequenceVariable( - CppModel.PREPROCESSOR_DEFINES_VARIABLE_NAME, - computeAllLinkstampDefines()); - // For dynamic libraries, produce position independent code. - if (cppConfiguration.forcePic() - || (linkTargetType == LinkTargetType.DYNAMIC_LIBRARY && ccProvider.toolchainNeedsPic())) { - linkstampVariables.addStringVariable(CppModel.PIC_VARIABLE_NAME, ""); - } - return linkstampVariables.build(); - } - - private ImmutableList<String> computeAllLinkstampDefines() { - String labelReplacement = - Matcher.quoteReplacement( - isSharedNativeLibrary() ? output.getExecPathString() : Label.print(owner.getLabel())); - String outputPathReplacement = Matcher.quoteReplacement(output.getExecPathString()); - String labelPattern = Pattern.quote("${LABEL}"); - String outputPathPattern = Pattern.quote("${OUTPUT_PATH}"); - ImmutableList.Builder<String> defines = ImmutableList.builder(); - defines - .add("GPLATFORM=\"" + cppConfiguration + "\"") - .add("BUILD_COVERAGE_ENABLED=" + (codeCoverageEnabled ? "1" : "0")) - // G3_VERSION_INFO and G3_TARGET_NAME are C string literals that normally - // contain the label of the target being linked. However, they are set - // differently when using shared native deps. In that case, a single .so file - // is shared by multiple targets, and its contents cannot depend on which - // target(s) were specified on the command line. So in that case we have - // to use the (obscure) name of the .so file instead, or more precisely - // the path of the .so file relative to the workspace root. - .add("G3_VERSION_INFO=\"${LABEL}\"") - .add("G3_TARGET_NAME=\"${LABEL}\"") - // G3_BUILD_TARGET is a C string literal containing the output of this - // link. (An undocumented and untested invariant is that G3_BUILD_TARGET is the location of - // the executable, either absolutely, or relative to the directory part of BUILD_INFO.) - .add("G3_BUILD_TARGET=\"${OUTPUT_PATH}\"") - .addAll(additionalLinkstampDefines); - - if (fdoBuildStamp != null) { - defines.add(CppConfiguration.FDO_STAMP_MACRO + "=\"" + fdoBuildStamp + "\""); - } - - return defines - .build() - .stream() - .map( - define -> - define - .replaceAll(labelPattern, labelReplacement) - .replaceAll(outputPathPattern, outputPathReplacement)) - .collect(ImmutableList.toImmutableList()); + return (getRawLinkArgv()); } /** A builder for a {@link LinkCommandLine}. */ public static final class Builder { private final BuildConfiguration configuration; - private final ActionOwner owner; private final RuleContext ruleContext; - private String forcedToolPath; - @Nullable private Artifact output; private ImmutableList<Artifact> buildInfoHeaderArtifacts = ImmutableList.of(); private Iterable<? extends LinkerInput> linkerInputs = ImmutableList.of(); private Iterable<? extends LinkerInput> runtimeInputs = ImmutableList.of(); @@ -656,37 +399,30 @@ public final class LinkCommandLine extends CommandLine { private LinkStaticness linkStaticness = LinkStaticness.FULLY_STATIC; private ImmutableList<String> linkopts = ImmutableList.of(); private ImmutableSet<String> features = ImmutableSet.of(); - private ImmutableMap<Artifact, Artifact> linkstamps = ImmutableMap.of(); - private ImmutableList<String> additionalLinkstampDefines = ImmutableList.of(); @Nullable private PathFragment runtimeSolibDir; private boolean nativeDeps; private boolean useTestOnlyFlags; @Nullable private Artifact paramFile; private CcToolchainProvider toolchain; - private FdoSupport fdoSupport; private Variables variables; private FeatureConfiguration featureConfiguration; // This interface is needed to support tests that don't create a // ruleContext, in which case the configuration and action owner // cannot be accessed off of the give ruleContext. - public Builder(BuildConfiguration configuration, ActionOwner owner, RuleContext ruleContext) { + public Builder(BuildConfiguration configuration, RuleContext ruleContext) { this.configuration = configuration; - this.owner = owner; this.ruleContext = ruleContext; } public Builder(RuleContext ruleContext) { - this(ruleContext.getConfiguration(), ruleContext.getActionOwner(), ruleContext); + this(ruleContext.getConfiguration(), ruleContext); } public LinkCommandLine build() { if (linkTargetType.staticness() == Staticness.STATIC) { Preconditions.checkArgument( - linkstamps.isEmpty(), - "linkstamps may only be present on dynamic library or executable links"); - Preconditions.checkArgument( buildInfoHeaderArtifacts.isEmpty(), "build info headers may only be present on dynamic library or executable links"); } @@ -700,11 +436,6 @@ public final class LinkCommandLine extends CommandLine { // The ruleContext can be null for some tests. if (ruleContext != null) { Preconditions.checkNotNull(featureConfiguration); - - if (fdoSupport == null) { - fdoSupport = - CppHelper.getFdoSupportUsingDefaultCcToolchainAttribute(ruleContext).getFdoSupport(); - } } if (variables == null) { @@ -717,8 +448,6 @@ public final class LinkCommandLine extends CommandLine { actionName, forcedToolPath, configuration, - owner, - output, buildInfoHeaderArtifacts, linkerInputs, runtimeInputs, @@ -726,9 +455,6 @@ public final class LinkCommandLine extends CommandLine { linkStaticness, linkopts, features, - linkstamps, - additionalLinkstampDefines, - CppHelper.getFdoBuildStamp(ruleContext, fdoSupport), runtimeSolibDir, nativeDeps, useTestOnlyFlags, @@ -747,11 +473,6 @@ public final class LinkCommandLine extends CommandLine { return this; } - public Builder setFdoSupport(FdoSupport fdoSupport) { - this.fdoSupport = fdoSupport; - return this; - } - /** Use given tool path instead of the one from feature configuration */ public Builder forceToolPath(String forcedToolPath) { this.forcedToolPath = forcedToolPath; @@ -777,14 +498,6 @@ public final class LinkCommandLine extends CommandLine { } /** - * Sets the primary output artifact. This must be called before calling {@link #build}. - */ - public Builder setOutput(Artifact output) { - this.output = output; - return this; - } - - /** * Sets a list of linker inputs. These get turned into linker options depending on the * staticness and the target type. This call makes an immutable copy of the inputs, if the * provided Iterable isn't already immutable (see {@link CollectionUtils#makeImmutable}). @@ -822,22 +535,6 @@ public final class LinkCommandLine extends CommandLine { } /** - * Sets the linkstamps. Linkstamps are additional C++ source files that are compiled as part of - * the link command. The {@link #build} method throws an exception if the linkstamps are - * non-empty for a static link (see {@link LinkTargetType#staticness()}}). - */ - public Builder setLinkstamps(ImmutableMap<Artifact, Artifact> linkstamps) { - this.linkstamps = linkstamps; - return this; - } - - /** Adds the given list of preprocessor defines to the linkstamp compilation. */ - public Builder setAdditionalLinkstampDefines(ImmutableList<String> additionalLinkstampDefines) { - this.additionalLinkstampDefines = Preconditions.checkNotNull(additionalLinkstampDefines); - return this; - } - - /** * The build info header artifacts are generated header files that are used for link stamping. * The {@link #build} method throws an exception if the build info header artifacts are * non-empty for a static link (see {@link LinkTargetType#staticness()}}). 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 b95b91da04..652f993625 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 @@ -17,6 +17,7 @@ package com.google.devtools.build.lib.rules.nativedeps; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.analysis.RuleContext; @@ -26,6 +27,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.rules.cpp.ArtifactCategory; import com.google.devtools.build.lib.rules.cpp.CcCommon; import com.google.devtools.build.lib.rules.cpp.CcLinkParams; +import com.google.devtools.build.lib.rules.cpp.CcLinkParams.Linkstamp; 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.CppBuildInfo; @@ -47,7 +49,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.LinkedList; import java.util.List; -import java.util.Map; /** * Helper class to create a dynamic library for rules which support integration with native code. @@ -185,8 +186,8 @@ public abstract class NativeDepsHelper { List<String> linkopts = new ArrayList<>(extraLinkOpts); linkopts.addAll(linkParams.flattenedLinkopts()); - Map<Artifact, NestedSet<Artifact>> linkstamps = - CppHelper.resolveLinkstamps(ruleContext, linkParams); + CppHelper.checkLinkstampsUnique(ruleContext, linkParams); + ImmutableSet<Linkstamp> linkstamps = ImmutableSet.copyOf(linkParams.getLinkstamps()); List<Artifact> buildInfoArtifacts = linkstamps.isEmpty() ? ImmutableList.<Artifact>of() : ruleContext.getAnalysisEnvironment().getBuildInfo( @@ -196,10 +197,16 @@ public abstract class NativeDepsHelper { NestedSet<LibraryToLink> linkerInputs = linkParams.getLibraries(); Artifact sharedLibrary; if (shareNativeDeps) { - PathFragment sharedPath = getSharedNativeDepsPath( - LinkerInputs.toLibraryArtifacts(linkerInputs), - linkopts, linkstamps.keySet(), buildInfoArtifacts, - ruleContext.getFeatures()); + PathFragment sharedPath = + getSharedNativeDepsPath( + LinkerInputs.toLibraryArtifacts(linkerInputs), + linkopts, + linkstamps + .stream() + .map(Linkstamp::getArtifact) + .collect(ImmutableList.toImmutableList()), + buildInfoArtifacts, + ruleContext.getFeatures()); libraryIdentifier = sharedPath.getPathString(); sharedLibrary = ruleContext.getShareableArtifact( sharedPath.replaceName(sharedPath.getBaseName() + ".so"), |