diff options
5 files changed, 283 insertions, 89 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java index 9d3a2f6719..17d4adcb60 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java @@ -32,6 +32,8 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Sets; import com.google.common.collect.Sets.SetView; import com.google.common.collect.Streams; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.collect.nestedset.NestedSet; @@ -76,7 +78,7 @@ import javax.annotation.Nullable; */ @Immutable public class CcToolchainFeatures implements Serializable { - + /** * Thrown when a flag value cannot be expanded under a set of build variables. * @@ -292,7 +294,7 @@ public class CcToolchainFeatures implements Serializable { * <p>The {@code variables} controls which variables are visible during the expansion and allows * to recursively expand nested flag groups. */ - void expand(Variables variables, List<String> commandLine); + void expand(Variables variables, @Nullable ArtifactExpander expander, List<String> commandLine); } /** @@ -308,10 +310,11 @@ public class CcToolchainFeatures implements Serializable { private Flag(ImmutableList<StringChunk> chunks) { this.chunks = chunks; } - + /** Expand this flag into a single new entry in {@code commandLine}. */ @Override - public void expand(Variables variables, List<String> commandLine) { + public void expand( + Variables variables, @Nullable ArtifactExpander expander, List<String> commandLine) { StringBuilder flag = new StringBuilder(); for (StringChunk chunk : chunks) { chunk.expand(variables, flag); @@ -406,53 +409,55 @@ public class CcToolchainFeatures implements Serializable { this.expandIfEqual = null; } } - + @Override - public void expand(Variables variables, final List<String> commandLine) { - if (!canBeExpanded(variables)) { + public void expand( + Variables variables, @Nullable ArtifactExpander expander, final List<String> commandLine) { + if (!canBeExpanded(variables, expander)) { return; } if (iterateOverVariable != null) { - for (VariableValue variableValue : variables.getSequenceVariable(iterateOverVariable)) { + for (VariableValue variableValue : + variables.getSequenceVariable(iterateOverVariable, expander)) { Variables nestedVariables = new Variables(variables, iterateOverVariable, variableValue); for (Expandable expandable : expandables) { - expandable.expand(nestedVariables, commandLine); + expandable.expand(nestedVariables, expander, commandLine); } } } else { for (Expandable expandable : expandables) { - expandable.expand(variables, commandLine); + expandable.expand(variables, expander, commandLine); } } } - private boolean canBeExpanded(Variables variables) { + private boolean canBeExpanded(Variables variables, @Nullable ArtifactExpander expander) { for (String variable : expandIfAllAvailable) { - if (!variables.isAvailable(variable)) { + if (!variables.isAvailable(variable, expander)) { return false; } } for (String variable : expandIfNoneAvailable) { - if (variables.isAvailable(variable)) { + if (variables.isAvailable(variable, expander)) { return false; } } if (expandIfTrue != null - && (!variables.isAvailable(expandIfTrue) + && (!variables.isAvailable(expandIfTrue, expander) || !variables.getVariable(expandIfTrue).isTruthy())) { return false; } if (expandIfFalse != null - && (!variables.isAvailable(expandIfFalse) + && (!variables.isAvailable(expandIfFalse, expander) || variables.getVariable(expandIfFalse).isTruthy())) { return false; } if (expandIfEqual != null - && (!variables.isAvailable(expandIfEqual.variable) + && (!variables.isAvailable(expandIfEqual.variable, expander) || !variables - .getVariable(expandIfEqual.variable) - .getStringValue(expandIfEqual.variable) - .equals(expandIfEqual.value))) { + .getVariable(expandIfEqual.variable) + .getStringValue(expandIfEqual.variable) + .equals(expandIfEqual.value))) { return false; } return true; @@ -473,8 +478,9 @@ public class CcToolchainFeatures implements Serializable { * explicit 'iterate_over' instead. * </ul> */ - private void expandCommandLine(Variables variables, final List<String> commandLine) { - expand(variables, commandLine); + private void expandCommandLine( + Variables variables, @Nullable ArtifactExpander expander, final List<String> commandLine) { + expand(variables, expander, commandLine); } } @@ -532,9 +538,10 @@ public class CcToolchainFeatures implements Serializable { String action, Variables variables, Set<String> enabledFeatureNames, + @Nullable ArtifactExpander expander, List<String> commandLine) { for (String variable : expandIfAllAvailable) { - if (!variables.isAvailable(variable)) { + if (!variables.isAvailable(variable, expander)) { return; } } @@ -545,7 +552,7 @@ public class CcToolchainFeatures implements Serializable { return; } for (FlagGroup flagGroup : flagGroups) { - flagGroup.expandCommandLine(variables, commandLine); + flagGroup.expandCommandLine(variables, expander, commandLine); } } } @@ -649,9 +656,10 @@ public class CcToolchainFeatures implements Serializable { String action, Variables variables, Set<String> enabledFeatureNames, + @Nullable ArtifactExpander expander, List<String> commandLine) { for (FlagSet flagSet : flagSets) { - flagSet.expandCommandLine(action, variables, enabledFeatureNames, commandLine); + flagSet.expandCommandLine(action, variables, enabledFeatureNames, expander, commandLine); } } } @@ -778,9 +786,13 @@ public class CcToolchainFeatures implements Serializable { /** Adds the flags that apply to this action to {@code commandLine}. */ private void expandCommandLine( - Variables variables, Set<String> enabledFeatureNames, List<String> commandLine) { + Variables variables, + Set<String> enabledFeatureNames, + @Nullable ArtifactExpander expander, + List<String> commandLine) { for (FlagSet flagSet : flagSets) { - flagSet.expandCommandLine(actionName, variables, enabledFeatureNames, commandLine); + flagSet.expandCommandLine( + actionName, variables, enabledFeatureNames, expander, commandLine); } } } @@ -899,6 +911,9 @@ public class CcToolchainFeatures implements Serializable { */ VariableValue getFieldValue(String variableName, String field); + VariableValue getFieldValue( + String variableName, String field, @Nullable ArtifactExpander expander); + /** Returns true if the variable is truthy */ boolean isTruthy(); } @@ -919,6 +934,12 @@ public class CcToolchainFeatures implements Serializable { @Override public VariableValue getFieldValue(String variableName, String field) { + return getFieldValue(variableName, field, null); + } + + @Override + public VariableValue getFieldValue( + String variableName, String field, @Nullable ArtifactExpander expander) { throw new ExpansionException( String.format( "Invalid toolchain configuration: Cannot expand variable '%s.%s': variable '%s' is " @@ -1097,58 +1118,87 @@ public class CcToolchainFeatures implements Serializable { } private final String name; + private final Artifact directory; private final ImmutableList<String> objectFiles; private final boolean isWholeArchive; private final Type type; public static LibraryToLinkValue forDynamicLibrary(String name) { return new LibraryToLinkValue( - Preconditions.checkNotNull(name), null, false, Type.DYNAMIC_LIBRARY); + Preconditions.checkNotNull(name), null, null, false, Type.DYNAMIC_LIBRARY); } public static LibraryToLinkValue forVersionedDynamicLibrary( String name) { return new LibraryToLinkValue( - Preconditions.checkNotNull(name), null, false, Type.VERSIONED_DYNAMIC_LIBRARY); + Preconditions.checkNotNull(name), null, null, false, Type.VERSIONED_DYNAMIC_LIBRARY); } public static LibraryToLinkValue forInterfaceLibrary(String name) { return new LibraryToLinkValue( - Preconditions.checkNotNull(name), null, false, Type.INTERFACE_LIBRARY); + Preconditions.checkNotNull(name), null, null, false, Type.INTERFACE_LIBRARY); } public static LibraryToLinkValue forStaticLibrary(String name, boolean isWholeArchive) { return new LibraryToLinkValue( - Preconditions.checkNotNull(name), null, isWholeArchive, Type.STATIC_LIBRARY); + Preconditions.checkNotNull(name), null, null, isWholeArchive, Type.STATIC_LIBRARY); } public static LibraryToLinkValue forObjectFile(String name, boolean isWholeArchive) { return new LibraryToLinkValue( - Preconditions.checkNotNull(name), null, isWholeArchive, Type.OBJECT_FILE); + Preconditions.checkNotNull(name), null, null, isWholeArchive, Type.OBJECT_FILE); } public static LibraryToLinkValue forObjectFileGroup( ImmutableList<String> objects, boolean isWholeArchive) { Preconditions.checkNotNull(objects); Preconditions.checkArgument(!objects.isEmpty()); - return new LibraryToLinkValue(null, objects, isWholeArchive, Type.OBJECT_FILE_GROUP); + return new LibraryToLinkValue(null, null, objects, isWholeArchive, Type.OBJECT_FILE_GROUP); + } + + public static LibraryToLinkValue forObjectDirectory( + Artifact directory, boolean isWholeArchive) { + Preconditions.checkNotNull(directory); + Preconditions.checkArgument(directory.isTreeArtifact()); + return new LibraryToLinkValue( + null, directory, null, isWholeArchive, Type.OBJECT_FILE_GROUP); } private LibraryToLinkValue( - String name, ImmutableList<String> objectFiles, boolean isWholeArchive, Type type) { + String name, + Artifact directory, + ImmutableList<String> objectFiles, + boolean isWholeArchive, + Type type) { this.name = name; + this.directory = directory; this.objectFiles = objectFiles; this.isWholeArchive = isWholeArchive; this.type = type; } @Override - public VariableValue getFieldValue(String variableName, String field) { + public VariableValue getFieldValue( + String variableName, String field, @Nullable ArtifactExpander expander) { Preconditions.checkNotNull(field); if (NAME_FIELD_NAME.equals(field) && !type.equals(Type.OBJECT_FILE_GROUP)) { return new StringValue(name); } else if (OBJECT_FILES_FIELD_NAME.equals(field) && type.equals(Type.OBJECT_FILE_GROUP)) { - return new StringSequence(objectFiles); + ImmutableList.Builder<String> expandedObjectFiles = ImmutableList.builder(); + if (objectFiles != null) { + expandedObjectFiles.addAll(objectFiles); + } else if (directory != null) { + if (expander != null) { + List<Artifact> artifacts = new ArrayList<>(); + expander.expand(directory, artifacts); + + expandedObjectFiles.addAll( + Iterables.transform(artifacts, artifact -> artifact.getExecPathString())); + } else { + expandedObjectFiles.add(directory.getExecPathString()); + } + } + return new StringSequence(expandedObjectFiles.build()); } else if (TYPE_FIELD_NAME.equals(field)) { return new StringValue(type.name); } else if (IS_WHOLE_ARCHIVE_FIELD_NAME.equals(field)) { @@ -1283,7 +1333,8 @@ public class CcToolchainFeatures implements Serializable { } @Override - public VariableValue getFieldValue(String variableName, String field) { + public VariableValue getFieldValue( + String variableName, String field, @Nullable ArtifactExpander expander) { if (value.containsKey(field)) { return value.get(field); } else { @@ -1570,7 +1621,11 @@ public class CcToolchainFeatures implements Serializable { * accessing a field of non-structured variable */ public VariableValue getVariable(String name) { - return lookupVariable(name, true); + return lookupVariable(name, true, null); + } + + public VariableValue getVariable(String name, @Nullable ArtifactExpander expander) { + return lookupVariable(name, true, expander); } /** @@ -1580,12 +1635,14 @@ public class CcToolchainFeatures implements Serializable { * @return Pair<VariableValue, String> returns either (variable value, null) or (null, string * reason why variable was not found) */ - private VariableValue lookupVariable(String name, boolean throwOnMissingVariable) { + private VariableValue lookupVariable( + String name, boolean throwOnMissingVariable, @Nullable ArtifactExpander expander) { VariableValue nonStructuredVariable = getNonStructuredVariable(name); if (nonStructuredVariable != null) { return nonStructuredVariable; } - VariableValue structuredVariable = getStructureVariable(name, throwOnMissingVariable); + VariableValue structuredVariable = + getStructureVariable(name, throwOnMissingVariable, expander); if (structuredVariable != null) { return structuredVariable; } else if (throwOnMissingVariable) { @@ -1612,7 +1669,8 @@ public class CcToolchainFeatures implements Serializable { return null; } - private VariableValue getStructureVariable(String name, boolean throwOnMissingVariable) { + private VariableValue getStructureVariable( + String name, boolean throwOnMissingVariable, @Nullable ArtifactExpander expander) { if (!name.contains(".")) { return null; } @@ -1633,7 +1691,7 @@ public class CcToolchainFeatures implements Serializable { while (!fieldsToAccess.empty()) { String field = fieldsToAccess.pop(); - variable = variable.getFieldValue(structPath, field); + variable = variable.getFieldValue(structPath, field, expander); if (variable == null) { if (throwOnMissingVariable) { throw new ExpansionException( @@ -1650,16 +1708,29 @@ public class CcToolchainFeatures implements Serializable { } public String getStringVariable(String variableName) { - return getVariable(variableName).getStringValue(variableName); + return getVariable(variableName, null).getStringValue(variableName); + } + + public String getStringVariable(String variableName, @Nullable ArtifactExpander expander) { + return getVariable(variableName, expander).getStringValue(variableName); } public Iterable<? extends VariableValue> getSequenceVariable(String variableName) { - return getVariable(variableName).getSequenceValue(variableName); + return getVariable(variableName, null).getSequenceValue(variableName); + } + + public Iterable<? extends VariableValue> getSequenceVariable( + String variableName, @Nullable ArtifactExpander expander) { + return getVariable(variableName, expander).getSequenceValue(variableName); } /** Returns whether {@code variable} is set. */ boolean isAvailable(String variable) { - return lookupVariable(variable, false) != null; + return isAvailable(variable, null); + } + + boolean isAvailable(String variable, @Nullable ArtifactExpander expander) { + return lookupVariable(variable, false, expander) != null; } } @@ -1725,19 +1796,22 @@ public class CcToolchainFeatures implements Serializable { return enabledActionConfigActionNames.contains(actionName); } - /** - * @return the command line for the given {@code action}. - */ + /** @return the command line for the given {@code action}. */ public List<String> getCommandLine(String action, Variables variables) { + return getCommandLine(action, variables, null); + } + + public List<String> getCommandLine( + String action, Variables variables, @Nullable ArtifactExpander expander) { List<String> commandLine = new ArrayList<>(); if (actionIsConfigured(action)) { actionConfigByActionName .get(action) - .expandCommandLine(variables, enabledFeatureNames, commandLine); + .expandCommandLine(variables, enabledFeatureNames, expander, commandLine); } for (Feature feature : enabledFeatures) { - feature.expandCommandLine(action, variables, enabledFeatureNames, commandLine); + feature.expandCommandLine(action, variables, enabledFeatureNames, expander, commandLine); } return commandLine; @@ -1746,18 +1820,23 @@ public class CcToolchainFeatures implements Serializable { /** @return the flags expanded for the given {@code action} in per-feature buckets. */ public ImmutableList<Pair<String, List<String>>> getPerFeatureExpansions( String action, Variables variables) { + return getPerFeatureExpansions(action, variables, null); + } + + public ImmutableList<Pair<String, List<String>>> getPerFeatureExpansions( + String action, Variables variables, @Nullable ArtifactExpander expander) { ImmutableList.Builder<Pair<String, List<String>>> perFeatureExpansions = ImmutableList.builder(); if (actionIsConfigured(action)) { List<String> commandLine = new ArrayList<>(); ActionConfig actionConfig = actionConfigByActionName.get(action); - actionConfig.expandCommandLine(variables, enabledFeatureNames, commandLine); + actionConfig.expandCommandLine(variables, enabledFeatureNames, expander, commandLine); perFeatureExpansions.add(Pair.of(actionConfig.getName(), commandLine)); } for (Feature feature : enabledFeatures) { List<String> commandLine = new ArrayList<>(); - feature.expandCommandLine(action, variables, enabledFeatureNames, commandLine); + feature.expandCommandLine(action, variables, enabledFeatureNames, expander, commandLine); perFeatureExpansions.add(Pair.of(feature.getName(), commandLine)); } 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 86aea4c801..932b5d8d93 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 @@ -29,6 +29,7 @@ import com.google.devtools.build.lib.actions.ActionKeyContext; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.ActionResult; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; import com.google.devtools.build.lib.actions.CommandAction; import com.google.devtools.build.lib.actions.CommandLineExpansionException; import com.google.devtools.build.lib.actions.ExecException; @@ -284,10 +285,11 @@ public final class CppLinkAction extends AbstractAction * Returns the command line specification for this link, included any required linkstamp * compilation steps. The command line may refer to a .params file. * + * @param expander ArtifactExpander for expanding TreeArtifacts. * @return a finalized command line suitable for execution */ - public final List<String> getCommandLine() { - return linkCommandLine.getCommandLine(); + public final List<String> getCommandLine(@Nullable ArtifactExpander expander) { + return linkCommandLine.getCommandLine(expander); } /** @@ -305,18 +307,19 @@ public final class CppLinkAction extends AbstractAction public ActionResult execute(ActionExecutionContext actionExecutionContext) throws ActionExecutionException, InterruptedException { if (fake) { - executeFake(); + executeFake(actionExecutionContext.getArtifactExpander()); return ActionResult.EMPTY; } else { try { - Spawn spawn = new SimpleSpawn( - this, - ImmutableList.copyOf(getCommandLine()), - getEnvironment(), - getExecutionInfo(), - ImmutableList.copyOf(getMandatoryInputs()), - getOutputs().asList(), - estimateResourceConsumptionLocal()); + Spawn spawn = + new SimpleSpawn( + this, + ImmutableList.copyOf(getCommandLine(actionExecutionContext.getArtifactExpander())), + getEnvironment(), + getExecutionInfo(), + ImmutableList.copyOf(getMandatoryInputs()), + getOutputs().asList(), + estimateResourceConsumptionLocal()); return ActionResult.create( actionExecutionContext .getSpawnActionContext(getMnemonic()) @@ -332,11 +335,11 @@ public final class CppLinkAction extends AbstractAction // Don't forget to update FAKE_LINK_GUID if you modify this method. @ThreadCompatible - private void executeFake() - throws ActionExecutionException { + private void executeFake(@Nullable ArtifactExpander expander) throws ActionExecutionException { // Prefix all fake output files in the command line with $TEST_TMPDIR/. final String outputPrefix = "$TEST_TMPDIR/"; - List<String> escapedLinkArgv = escapeLinkArgv(linkCommandLine.getRawLinkArgv(), outputPrefix); + List<String> escapedLinkArgv = + escapeLinkArgv(linkCommandLine.getRawLinkArgv(expander), outputPrefix); // Write the commands needed to build the real target to the fake target // file. StringBuilder s = new StringBuilder(); @@ -424,7 +427,7 @@ public final class CppLinkAction extends AbstractAction info.setLinkStaticness(getLinkCommandLine().getLinkStaticness().name()); info.addAllLinkStamp(Artifact.toExecPaths(getLinkstampObjects())); info.addAllBuildInfoHeaderArtifact(Artifact.toExecPaths(getBuildInfoHeaderArtifacts())); - info.addAllLinkOpt(getLinkCommandLine().getRawLinkArgv()); + info.addAllLinkOpt(getLinkCommandLine().getRawLinkArgv(null)); try { return super.getExtraActionInfo(actionKeyContext) 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 df76aad01a..f7646c6d25 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 @@ -969,7 +969,8 @@ public class CppLinkActionBuilder { .setLinkopts(ImmutableList.copyOf(linkopts)); } else { List<String> opts = new ArrayList<>(linkopts); - opts.addAll(featureConfiguration.getCommandLine("lto-indexing", buildVariables)); + opts.addAll( + featureConfiguration.getCommandLine("lto-indexing", buildVariables, null /* expander */)); opts.addAll(cppConfiguration.getLtoIndexOptions()); linkCommandLineBuilder.setLinkopts(ImmutableList.copyOf(opts)); } @@ -1065,9 +1066,13 @@ public class CppLinkActionBuilder { } if (linkCommandLine.getParamFile() != null) { inputsBuilder.add(ImmutableList.of(linkCommandLine.getParamFile())); + // Pass along tree artifacts, so they can be properly expanded. + Iterable<Artifact> expandedNonLibraryTreeArtifactInputs = + Iterables.filter(expandedNonLibraryInputs, a -> a.isTreeArtifact()); Action parameterFileWriteAction = new ParameterFileWriteAction( getOwner(), + expandedNonLibraryTreeArtifactInputs, paramFile, linkCommandLine.paramCmdLine(), ParameterFile.ParameterFileType.UNQUOTED, @@ -1260,15 +1265,6 @@ public class CppLinkActionBuilder { return this; } - private void addObjectFile(LinkerInput input) { - // We skip file extension checks for TreeArtifacts because they represent directory artifacts - // without a file extension. - String name = input.getArtifact().getFilename(); - Preconditions.checkArgument( - input.getArtifact().isTreeArtifact() || Link.OBJECT_FILETYPES.matches(name), name); - this.objectFiles.add(input); - } - public CppLinkActionBuilder addLtoBitcodeFiles(ImmutableMap<Artifact, Artifact> files) { Preconditions.checkState(ltoBitcodeFiles == null); ltoBitcodeFiles = files; @@ -1280,6 +1276,15 @@ public class CppLinkActionBuilder { return this; } + private void addObjectFile(LinkerInput input) { + // We skip file extension checks for TreeArtifacts because they represent directory artifacts + // without a file extension. + String name = input.getArtifact().getFilename(); + Preconditions.checkArgument( + input.getArtifact().isTreeArtifact() || Link.OBJECT_FILETYPES.matches(name), name); + this.objectFiles.add(input); + } + /** * Adds a single object file to the set of inputs. */ @@ -2058,10 +2063,16 @@ public class CppLinkActionBuilder { name = inputArtifact.getExecPathString(); } - librariesToLink.addValue( - artifactCategory.equals(ArtifactCategory.OBJECT_FILE) - ? LibraryToLinkValue.forObjectFile(name, inputIsWholeArchive) - : LibraryToLinkValue.forStaticLibrary(name, inputIsWholeArchive)); + if (artifactCategory.equals(ArtifactCategory.OBJECT_FILE)) { + if (inputArtifact.isTreeArtifact()) { + librariesToLink.addValue( + LibraryToLinkValue.forObjectDirectory(inputArtifact, inputIsWholeArchive)); + } else { + librariesToLink.addValue(LibraryToLinkValue.forObjectFile(name, inputIsWholeArchive)); + } + } else { + librariesToLink.addValue(LibraryToLinkValue.forStaticLibrary(name, inputIsWholeArchive)); + } } } } 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 83116f4dd5..f14fe175f7 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 @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; 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; @@ -188,7 +189,13 @@ public final class LinkCommandLine extends CommandLine { */ @VisibleForTesting final Pair<List<String>, List<String>> splitCommandline() { - List<String> args = getRawLinkArgv(); + return splitCommandline(null); + } + + @VisibleForTesting + final Pair<List<String>, List<String>> splitCommandline(@Nullable ArtifactExpander expander) { + Preconditions.checkNotNull(paramFile); + List<String> args = getRawLinkArgv(expander); if (linkTargetType.staticness() == Staticness.STATIC) { // Ar link commands can also generate huge command lines. List<String> paramFileArgs = new ArrayList<>(); @@ -215,7 +222,12 @@ public final class LinkCommandLine extends CommandLine { return new CommandLine() { @Override public Iterable<String> arguments() { - return splitCommandline().getSecond(); + return splitCommandline(null).getSecond(); + } + + @Override + public Iterable<String> arguments(ArtifactExpander expander) { + return splitCommandline(expander).getSecond(); } }; } @@ -342,11 +354,23 @@ public final class LinkCommandLine extends CommandLine { /** * Returns a raw link command for the given link invocation, including both command and arguments - * (argv). + * (argv). The version that uses the expander is preferred, but that one can't be used during + * analysis. * * @return raw link command line. */ public List<String> getRawLinkArgv() { + return getRawLinkArgv(null); + } + + /** + * Returns a raw link command for the given link invocation, including both command and arguments + * (argv). + * + * @param expander ArtifactExpander for expanding TreeArtifacts. + * @return raw link command line. + */ + public List<String> getRawLinkArgv(@Nullable ArtifactExpander expander) { List<String> argv = new ArrayList<>(); if (forcedToolPath != null) { argv.add(forcedToolPath); @@ -366,24 +390,30 @@ public final class LinkCommandLine extends CommandLine { new Variables.Builder(variables) .addStringSequenceVariable( CppLinkActionBuilder.LEGACY_LINK_FLAGS_VARIABLE, getToolchainFlags()) - .build())); + .build(), + expander)); return argv; } - List<String> getCommandLine() { + List<String> getCommandLine(@Nullable ArtifactExpander expander) { // 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(); + Pair<List<String>, List<String>> split = splitCommandline(expander); return split.first; } else { - return getRawLinkArgv(); + return getRawLinkArgv(expander); } } @Override public List<String> arguments() { - return getRawLinkArgv(); + return getRawLinkArgv(null); + } + + @Override + public Iterable<String> arguments(ArtifactExpander artifactExpander) { + return getRawLinkArgv(artifactExpander); } /** A builder for a {@link LinkCommandLine}. */ diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java index c31e623360..71fd9694a5 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java @@ -23,7 +23,13 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; +import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; +import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType; +import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; +import com.google.devtools.build.lib.actions.ArtifactOwner; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; @@ -44,11 +50,15 @@ 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.rules.cpp.LinkerInputs.LibraryToLink; +import com.google.devtools.build.lib.testutil.TestUtils; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.OsUtils; +import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; @@ -602,7 +612,7 @@ public class CppLinkActionTest extends BuildViewTestCase { .setLibraryIdentifier("foo") .setInterfaceOutput(scratchArtifact("FakeInterfaceOutput.ifso")); - List<String> commandLine = builder.build().getCommandLine(); + List<String> commandLine = builder.build().getCommandLine(null); assertThat(commandLine).hasSize(6); assertThat(commandLine.get(0)).endsWith("custom/crosstool/scripts/link_dynamic_library.sh"); assertThat(commandLine.get(1)).isEqualTo("yes"); @@ -654,4 +664,65 @@ public class CppLinkActionTest extends BuildViewTestCase { assertError("the need whole archive flag must be false for static links", builder); } + + private Artifact createTreeArtifact(String name) { + FileSystem fs = scratch.getFileSystem(); + Path execRoot = fs.getPath(TestUtils.tmpDir()); + PathFragment execPath = PathFragment.create("out").getRelative(name); + Path path = execRoot.getRelative(execPath); + return new SpecialArtifact( + path, + Root.asDerivedRoot(execRoot, execRoot.getRelative("out")), + execPath, + ArtifactOwner.NULL_OWNER, + SpecialArtifactType.TREE); + } + + private void verifyArguments( + Iterable<String> arguments, + Iterable<String> allowedArguments, + Iterable<String> disallowedArguments) { + assertThat(arguments).containsAllIn(allowedArguments); + assertThat(arguments).containsNoneIn(disallowedArguments); + } + + @Test + public void testLinksTreeArtifactLibraries() throws Exception { + Artifact testTreeArtifact = createTreeArtifact("library_directory"); + + TreeFileArtifact library0 = ActionInputHelper.treeFileArtifact(testTreeArtifact, "library0.o"); + TreeFileArtifact library1 = ActionInputHelper.treeFileArtifact(testTreeArtifact, "library1.o"); + + ArtifactExpander expander = + new ArtifactExpander() { + @Override + public void expand(Artifact artifact, Collection<? super Artifact> output) { + if (artifact.equals(testTreeArtifact)) { + output.add(library0); + output.add(library1); + } + }; + }; + + CppLinkActionBuilder builder = + createLinkBuilder(LinkTargetType.STATIC_LIBRARY) + .setLibraryIdentifier("foo") + .addObjectFiles(ImmutableList.of(testTreeArtifact)) + // Makes sure this doesn't use a params file. + .setFake(true); + + CppLinkAction linkAction = builder.build(); + + Iterable<String> treeArtifactsPaths = ImmutableList.of(testTreeArtifact.getExecPathString()); + Iterable<String> treeFileArtifactsPaths = + ImmutableList.of(library0.getExecPathString(), library1.getExecPathString()); + + // Should only reference the tree artifact. + verifyArguments(linkAction.getCommandLine(null), treeArtifactsPaths, treeFileArtifactsPaths); + verifyArguments(linkAction.getArguments(), treeArtifactsPaths, treeFileArtifactsPaths); + + // Should only reference tree file artifacts. + verifyArguments( + linkAction.getCommandLine(expander), treeFileArtifactsPaths, treeArtifactsPaths); + } } |