Move split/no-split command-line decision to CppLinkAction.
--
MOS_MIGRATED_REVID=96301836
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ParameterFile.java b/src/main/java/com/google/devtools/build/lib/actions/ParameterFile.java
index 1528eff..d9af4f8 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ParameterFile.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ParameterFile.java
@@ -17,7 +17,6 @@
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.vfs.PathFragment;
-
/**
* Support for parameter file generation (as used by gcc and other tools, e.g.
* {@code gcc @param_file}. Note that the parameter file needs to be explicitly
@@ -64,7 +63,7 @@
private ParameterFile() {
}
/**
- * Derives an exec path from a given exec path by appending <code>".params"</code>.
+ * Derives an path from a given path by appending <code>".params"</code>.
*/
public static PathFragment derivePath(PathFragment original) {
return original.replaceName(original.getBaseName() + "-2.params");
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 4d63994..46988c9 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
@@ -54,7 +54,6 @@
import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
import com.google.devtools.build.lib.util.Fingerprint;
-import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.ShellEscaper;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
@@ -68,6 +67,8 @@
import java.util.Map;
import java.util.Set;
+import javax.annotation.Nullable;
+
/**
* Action that represents an ELF linking step.
*/
@@ -204,16 +205,7 @@
*/
public final List<String> getCommandLine()
throws ExecException {
- 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 (linkCommandLine.canBeSplit()) {
- Pair<List<String>, List<String>> split = linkCommandLine.splitCommandline();
- commandlineArgs = split.first;
- } else {
- commandlineArgs = linkCommandLine.getRawLinkArgv();
- }
- return linkCommandLine.finalizeWithLinkstampCommands(commandlineArgs);
+ return linkCommandLine.getCommandLine();
}
@Override
@@ -456,7 +448,9 @@
private final RuleContext ruleContext;
private final AnalysisEnvironment analysisEnvironment;
private final PathFragment outputPath;
- private final CcToolchainProvider toolchain;
+
+ // can be null for CppLinkAction.createTestBuilder()
+ @Nullable private final CcToolchainProvider toolchain;
private PathFragment interfaceOutputPath;
private PathFragment runtimeSolibDir;
protected final BuildConfiguration configuration;
@@ -479,7 +473,6 @@
private boolean isNativeDeps;
private boolean useTestOnlyFlags;
private boolean wholeArchive;
- private boolean supportsParamFiles = true;
/**
* Creates a builder that builds {@link CppLinkAction} instances.
@@ -524,14 +517,9 @@
this.configuration = Preconditions.checkNotNull(configuration);
this.cppConfiguration = configuration.getFragment(CppConfiguration.class);
this.toolchain = toolchain;
-
- // The toolchain != null is here for CppLinkAction.createTestBuilder(). Meh.
if (cppConfiguration.supportsEmbeddedRuntimes() && toolchain != null) {
runtimeSolibDir = toolchain.getDynamicRuntimeSolibDir();
}
- if (toolchain != null) {
- supportsParamFiles = toolchain.supportsParamFiles();
- }
}
/**
@@ -566,6 +554,29 @@
this.useTestOnlyFlags = linkContext.useTestOnlyFlags;
}
+ @VisibleForTesting
+ boolean canSplitCommandLine() {
+ if (toolchain == null || !toolchain.supportsParamFiles()) {
+ return false;
+ }
+
+ switch (linkType) {
+ // We currently can't split dynamic library links if they have interface outputs. That was
+ // probably an unintended side effect of the change that introduced interface outputs.
+ case DYNAMIC_LIBRARY:
+ return (interfaceOutputPath == null);
+ case EXECUTABLE:
+ case STATIC_LIBRARY:
+ case PIC_STATIC_LIBRARY:
+ case ALWAYS_LINK_STATIC_LIBRARY:
+ case ALWAYS_LINK_PIC_STATIC_LIBRARY:
+ return true;
+
+ default:
+ return false;
+ }
+ }
+
/**
* Builds the Action as configured and returns it.
*
@@ -595,8 +606,8 @@
linkStaticness, linkType, linkopts, isNativeDeps, cppConfiguration);
NestedSet<LibraryToLink> uniqueLibraries = libraries.build();
- final Iterable<Artifact> filteredNonLibraryArtifacts = filterLinkerInputArtifacts(
- LinkerInputs.toLibraryArtifacts(nonLibraries));
+ final Iterable<Artifact> filteredNonLibraryArtifacts =
+ filterLinkerInputArtifacts(LinkerInputs.toLibraryArtifacts(nonLibraries));
final Iterable<LinkerInput> linkerInputs = IterablesChain.<LinkerInput>builder()
.add(ImmutableList.copyOf(filterLinkerInputs(nonLibraries)))
.add(ImmutableIterable.from(Link.mergeInputsCmdLine(
@@ -616,37 +627,42 @@
final ImmutableMap<Artifact, Artifact> linkstampMap =
mapLinkstampsToOutputs(linkstamps, ruleContext, output);
- final ImmutableList<Artifact> actionOutputs = constructOutputs(
- outputLibrary.getArtifact(),
- linkstampMap.values(),
- interfaceOutputLibrary == null ? null : interfaceOutputLibrary.getArtifact(),
- symbolCountOutput);
+ final ImmutableList<Artifact> actionOutputs =
+ constructOutputs(
+ outputLibrary.getArtifact(),
+ linkstampMap.values(),
+ interfaceOutputLibrary == null ? null : interfaceOutputLibrary.getArtifact(),
+ symbolCountOutput);
- final PathFragment paramFileFragment =
- supportsParamFiles
- ? ParameterFile.derivePath(outputLibrary.getArtifact().getExecPath())
+ @Nullable
+ final Artifact paramFile =
+ canSplitCommandLine()
+ ? createArtifact(
+ ParameterFile.derivePath(outputLibrary.getArtifact().getRootRelativePath()))
: null;
- LinkCommandLine linkCommandLine = new LinkCommandLine.Builder(configuration, getOwner())
- .setOutput(outputLibrary.getArtifact())
- .setInterfaceOutput(interfaceOutput)
- .setSymbolCountsOutput(symbolCountOutput)
- .setBuildInfoHeaderArtifacts(buildInfoHeaderArtifacts)
- .setLinkerInputs(linkerInputs)
- .setRuntimeInputs(ImmutableList.copyOf(LinkerInputs.simpleLinkerInputs(runtimeInputs)))
- .setLinkTargetType(linkType)
- .setLinkStaticness(linkStaticness)
- .setLinkopts(ImmutableList.copyOf(linkopts))
- .setFeatures(features)
- .setLinkstamps(linkstampMap)
- .addLinkstampCompileOptions(linkstampOptions)
- .setRuntimeSolibDir(linkType.isStaticLibraryLink() ? null : runtimeSolibDir)
- .setNativeDeps(isNativeDeps)
- .setUseTestOnlyFlags(useTestOnlyFlags)
- .setNeedWholeArchive(needWholeArchive)
- .setInterfaceSoBuilder(getInterfaceSoBuilder())
- .setParamFileFragment(paramFileFragment)
- .build();
+ LinkCommandLine linkCommandLine =
+ new LinkCommandLine.Builder(configuration, getOwner())
+ .setOutput(outputLibrary.getArtifact())
+ .setInterfaceOutput(interfaceOutput)
+ .setSymbolCountsOutput(symbolCountOutput)
+ .setBuildInfoHeaderArtifacts(buildInfoHeaderArtifacts)
+ .setLinkerInputs(linkerInputs)
+ .setRuntimeInputs(
+ ImmutableList.copyOf(LinkerInputs.simpleLinkerInputs(runtimeInputs)))
+ .setLinkTargetType(linkType)
+ .setLinkStaticness(linkStaticness)
+ .setLinkopts(ImmutableList.copyOf(linkopts))
+ .setFeatures(features)
+ .setLinkstamps(linkstampMap)
+ .addLinkstampCompileOptions(linkstampOptions)
+ .setRuntimeSolibDir(linkType.isStaticLibraryLink() ? null : runtimeSolibDir)
+ .setNativeDeps(isNativeDeps)
+ .setUseTestOnlyFlags(useTestOnlyFlags)
+ .setNeedWholeArchive(needWholeArchive)
+ .setInterfaceSoBuilder(getInterfaceSoBuilder())
+ .setParamFile(paramFile)
+ .build();
// Compute the set of inputs - we only need stable order here.
NestedSetBuilder<Artifact> dependencyInputsBuilder = NestedSetBuilder.stableOrder();
@@ -668,13 +684,15 @@
.add(dependencyInputsBuilder.build())
.add(ImmutableIterable.from(expandedInputs));
- if (linkCommandLine.canBeSplit()) {
- Artifact paramFile = createArtifact(
- ParameterFile.derivePath(outputLibrary.getArtifact().getRootRelativePath()));
- inputsBuilder.add(ImmutableList.of(paramFile));
- Action parameterFileWriteAction = new ParameterFileWriteAction(
- getOwner(), paramFile, linkCommandLine.paramCmdLine(),
- ParameterFile.ParameterFileType.UNQUOTED, ISO_8859_1);
+ if (linkCommandLine.getParamFile() != null) {
+ inputsBuilder.add(ImmutableList.of(linkCommandLine.getParamFile()));
+ Action parameterFileWriteAction =
+ new ParameterFileWriteAction(
+ getOwner(),
+ paramFile,
+ linkCommandLine.paramCmdLine(),
+ ParameterFile.ParameterFileType.UNQUOTED,
+ ISO_8859_1);
analysisEnvironment.registerAction(parameterFileWriteAction);
}
@@ -748,8 +766,9 @@
return ruleContext.getActionOwner();
}
- protected Artifact createArtifact(PathFragment path) {
- return analysisEnvironment.getDerivedArtifact(path, configuration.getBinDirectory());
+ protected Artifact createArtifact(PathFragment rootRelativePath) {
+ return analysisEnvironment.getDerivedArtifact(
+ rootRelativePath, configuration.getBinDirectory());
}
protected Artifact getInterfaceSoBuilder() {
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 f8fc37e..fd30c25 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
@@ -74,7 +74,7 @@
private final boolean nativeDeps;
private final boolean useTestOnlyFlags;
private final boolean needWholeArchive;
- private final PathFragment paramFileExecPath;
+ @Nullable private final Artifact paramFile;
@Nullable private final Artifact interfaceSoBuilder;
private LinkCommandLine(
@@ -96,7 +96,7 @@
boolean nativeDeps,
boolean useTestOnlyFlags,
boolean needWholeArchive,
- PathFragment paramFileFragment,
+ @Nullable Artifact paramFile,
Artifact interfaceSoBuilder) {
Preconditions.checkArgument(linkTargetType != LinkTargetType.INTERFACE_DYNAMIC_LIBRARY,
"you can't link an interface dynamic library directly");
@@ -143,7 +143,8 @@
this.nativeDeps = nativeDeps;
this.useTestOnlyFlags = useTestOnlyFlags;
this.needWholeArchive = needWholeArchive;
- this.paramFileExecPath = paramFileFragment;
+ this.paramFile = paramFile;
+
// For now, silently ignore interfaceSoBuilder if we don't build an interface dynamic library.
this.interfaceSoBuilder =
((linkTargetType == LinkTargetType.DYNAMIC_LIBRARY) && (interfaceOutput != null))
@@ -157,7 +158,8 @@
* non-null if {@link #getLinkTargetType} is {@code DYNAMIC_LIBRARY} and an interface shared
* object was requested.
*/
- @Nullable public Artifact getInterfaceOutput() {
+ @Nullable
+ public Artifact getInterfaceOutput() {
return interfaceOutput;
}
@@ -170,6 +172,11 @@
return symbolCountsOutput;
}
+ @Nullable
+ public Artifact getParamFile() {
+ return paramFile;
+ }
+
/**
* Returns the (ordered, immutable) list of header files that contain build info.
*/
@@ -250,14 +257,13 @@
/**
* Splits the link command-line into a part to be written to a parameter file, and the remaining
- * actual command line to be executed (which references the parameter file). Call {@link
- * #canBeSplit} first to check if the command-line can be split.
+ * actual command line to be executed (which references the parameter file). Should only be used
+ * if getParamFile() is not null.
*
* @throws IllegalStateException if the command-line cannot be split
*/
@VisibleForTesting
final Pair<List<String>, List<String>> splitCommandline() {
- Preconditions.checkState(canBeSplit());
List<String> args = getRawLinkArgv();
if (linkTargetType.isStaticLibraryLink()) {
// Ar link commands can also generate huge command lines.
@@ -265,7 +271,7 @@
List<String> commandlineArgs = new ArrayList<>();
commandlineArgs.add(args.get(0));
- commandlineArgs.add("@" + paramFileExecPath.getPathString());
+ commandlineArgs.add("@" + paramFile.getExecPath().getPathString());
return Pair.of(commandlineArgs, paramFileArgs);
} else {
// Gcc link commands tend to generate humongous commandlines for some targets, which may
@@ -275,7 +281,7 @@
List<String> commandlineArgs = new ArrayList<>();
extractArgumentsForParamFile(args, commandlineArgs, paramFileArgs);
- commandlineArgs.add("-Wl,@" + paramFileExecPath.getPathString());
+ commandlineArgs.add("-Wl,@" + paramFile.getExecPath().getPathString());
return Pair.of(commandlineArgs, paramFileArgs);
}
}
@@ -286,7 +292,7 @@
* @throws IllegalStateException if the command-line cannot be split
*/
CommandLine paramCmdLine() {
- Preconditions.checkState(canBeSplit());
+ Preconditions.checkNotNull(paramFile);
return new CommandLine() {
@Override
public Iterable<String> arguments() {
@@ -295,25 +301,6 @@
};
}
- boolean canBeSplit() {
- if (paramFileExecPath == null) {
- return false;
- }
- switch (linkTargetType) {
- // We currently can't split dynamic library links if they have interface outputs. That was
- // probably an unintended side effect of the change that introduced interface outputs.
- case DYNAMIC_LIBRARY:
- return interfaceOutput == null;
- case EXECUTABLE:
- case STATIC_LIBRARY:
- case PIC_STATIC_LIBRARY:
- case ALWAYS_LINK_STATIC_LIBRARY:
- case ALWAYS_LINK_PIC_STATIC_LIBRARY:
- return true;
- default:
- return false;
- }
- }
private static void extractArgumentsForParamFile(List<String> args, List<String> commandlineArgs,
List<String> paramFileArgs) {
@@ -421,6 +408,19 @@
return argv;
}
+ 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;
+ } else {
+ commandlineArgs = getRawLinkArgv();
+ }
+ return finalizeWithLinkstampCommands(commandlineArgs);
+ }
+
@Override
public List<String> arguments() {
return finalizeWithLinkstampCommands(getRawLinkArgv());
@@ -946,7 +946,7 @@
private boolean nativeDeps;
private boolean useTestOnlyFlags;
private boolean needWholeArchive;
- private PathFragment paramFileFragment;
+ @Nullable private Artifact paramFile;
@Nullable private Artifact interfaceSoBuilder;
public Builder(BuildConfiguration configuration, ActionOwner owner) {
@@ -966,10 +966,26 @@
actualLinkstampCompileOptions = ImmutableList.copyOf(
Iterables.concat(DEFAULT_LINKSTAMP_OPTIONS, linkstampCompileOptions));
}
- return new LinkCommandLine(configuration, owner, output, interfaceOutput,
- symbolCountsOutput, buildInfoHeaderArtifacts, linkerInputs, runtimeInputs, linkTargetType,
- linkStaticness, linkopts, features, linkstamps, actualLinkstampCompileOptions,
- runtimeSolibDir, nativeDeps, useTestOnlyFlags, needWholeArchive, paramFileFragment,
+ return new LinkCommandLine(
+ configuration,
+ owner,
+ output,
+ interfaceOutput,
+ symbolCountsOutput,
+ buildInfoHeaderArtifacts,
+ linkerInputs,
+ runtimeInputs,
+ linkTargetType,
+ linkStaticness,
+ linkopts,
+ features,
+ linkstamps,
+ actualLinkstampCompileOptions,
+ runtimeSolibDir,
+ nativeDeps,
+ useTestOnlyFlags,
+ needWholeArchive,
+ paramFile,
interfaceSoBuilder);
}
@@ -1131,8 +1147,8 @@
return this;
}
- public Builder setParamFileFragment(PathFragment paramFragment) {
- this.paramFileFragment = paramFragment;
+ public Builder setParamFile(Artifact paramFile) {
+ this.paramFile = paramFile;
return this;
}
}
