diff options
author | 2015-06-18 13:24:49 +0000 | |
---|---|---|
committer | 2015-06-18 15:57:20 +0000 | |
commit | ad3582369b805372a34ab0fa56f73d33660af30a (patch) | |
tree | b19a79107a0d92766f4ede053c2a90740cecc106 /src/main/java/com/google/devtools/build | |
parent | 884eef0c4474ed5ee33396bd10e463e9f8353697 (diff) |
Move split/no-split command-line decision to CppLinkAction.
--
MOS_MIGRATED_REVID=96301836
Diffstat (limited to 'src/main/java/com/google/devtools/build')
3 files changed, 130 insertions, 96 deletions
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 1528eff604..d9af4f82e4 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.common.annotations.VisibleForTesting; 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 @@ public class ParameterFile { 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 4d6399457e..46988c91bb 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.LinkStaticness; 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.List; 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 class CppLinkAction extends AbstractAction { */ 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 @@ public final class CppLinkAction extends AbstractAction { 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 @@ public final class CppLinkAction extends AbstractAction { 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 @@ public final class CppLinkAction extends AbstractAction { 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 @@ public final class CppLinkAction extends AbstractAction { 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 @@ public final class CppLinkAction extends AbstractAction { 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 @@ public final class CppLinkAction extends AbstractAction { 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 PathFragment paramFileFragment = - supportsParamFiles - ? ParameterFile.derivePath(outputLibrary.getArtifact().getExecPath()) + final ImmutableList<Artifact> actionOutputs = + constructOutputs( + outputLibrary.getArtifact(), + linkstampMap.values(), + interfaceOutputLibrary == null ? null : interfaceOutputLibrary.getArtifact(), + symbolCountOutput); + + @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 @@ public final class CppLinkAction extends AbstractAction { .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 @@ public final class CppLinkAction extends AbstractAction { 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 f8fc37e0a1..fd30c25d82 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 @@ public final class LinkCommandLine extends CommandLine { 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 @@ public final class LinkCommandLine extends CommandLine { 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 @@ public final class LinkCommandLine extends CommandLine { 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 @@ public final class LinkCommandLine extends CommandLine { * 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 @@ public final class LinkCommandLine extends CommandLine { return symbolCountsOutput; } + @Nullable + public Artifact getParamFile() { + return paramFile; + } + /** * Returns the (ordered, immutable) list of header files that contain build info. */ @@ -250,14 +257,13 @@ public final class LinkCommandLine extends CommandLine { /** * 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 @@ public final class LinkCommandLine extends CommandLine { 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 @@ public final class LinkCommandLine extends CommandLine { 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 @@ public final class LinkCommandLine extends CommandLine { * @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 @@ public final class LinkCommandLine extends CommandLine { }; } - 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 @@ public final class LinkCommandLine extends CommandLine { 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 @@ public final class LinkCommandLine extends CommandLine { 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 @@ public final class LinkCommandLine extends CommandLine { 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 @@ public final class LinkCommandLine extends CommandLine { return this; } - public Builder setParamFileFragment(PathFragment paramFragment) { - this.paramFileFragment = paramFragment; + public Builder setParamFile(Artifact paramFile) { + this.paramFile = paramFile; return this; } } |