From 99de0d07574f808fee36826289cb1f5c83e3b3e0 Mon Sep 17 00:00:00 2001 From: Lukacs Berki Date: Tue, 9 Aug 2016 13:44:27 +0000 Subject: Cleanup: store non-code inputs to link actions separately. -- MOS_MIGRATED_REVID=129743936 --- .../devtools/build/lib/rules/cpp/CcBinary.java | 8 +- .../build/lib/rules/cpp/CcCompilationOutputs.java | 6 +- .../build/lib/rules/cpp/CcLibraryHelper.java | 27 +++---- .../build/lib/rules/cpp/CppCompilationContext.java | 20 ++--- .../build/lib/rules/cpp/CppLinkAction.java | 31 ++----- .../build/lib/rules/cpp/CppLinkActionBuilder.java | 94 +++++++++++++--------- .../devtools/build/lib/rules/cpp/CppModel.java | 19 ++--- .../google/devtools/build/lib/rules/cpp/Link.java | 8 -- .../rules/cpp/CcLibraryConfiguredTargetTest.java | 4 +- .../build/lib/rules/cpp/CppLinkActionTest.java | 2 +- 10 files changed, 109 insertions(+), 110 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java index 35a3294d3a..a4ceda45a3 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java @@ -204,7 +204,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { linkStaticness, linkopts); linkActionBuilder.setUseTestOnlyFlags(isTest); - linkActionBuilder.addNonLibraryInputs(ccCompilationOutputs.getHeaderTokenFiles()); + linkActionBuilder.addNonCodeInputs(ccCompilationOutputs.getHeaderTokenFiles()); CcToolchainProvider ccToolchain = CppHelper.getToolchain(ruleContext); if (linkStaticness == LinkStaticness.DYNAMIC) { @@ -398,7 +398,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { CppLinkActionBuilder builder = new CppLinkActionBuilder(context, binary) .setCrosstoolInputs(CppHelper.getToolchain(context).getLink()) - .addNonLibraryInputs(compilationPrerequisites); + .addNonCodeInputs(compilationPrerequisites); // Determine the object files to link in. boolean usePic = CppHelper.usePic(context, !isLinkShared(context)); @@ -407,11 +407,11 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { if (fake) { builder.addFakeNonLibraryInputs(objectFiles); } else { - builder.addNonLibraryInputs(objectFiles); + builder.addObjectFiles(objectFiles); } builder.addLTOBitcodeFiles(compilationOutputs.getLtoBitcodeFiles()); - builder.addNonLibraryInputs(common.getLinkerScripts()); + builder.addNonCodeInputs(common.getLinkerScripts()); // Determine the libraries to link in. // First libraries from srcs. Shared library artifacts here are substituted with mangled symlink diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java index b7f0300adb..d0cd8dfece 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java @@ -20,7 +20,7 @@ import com.google.devtools.build.lib.actions.Artifact; 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.util.Preconditions; import java.util.ArrayList; import java.util.LinkedHashSet; import java.util.List; @@ -200,11 +200,15 @@ public class CcCompilationOutputs { * Adds an .o file. */ public Builder addObjectFile(Artifact artifact) { + Preconditions.checkArgument(Link.OBJECT_FILETYPES.matches(artifact.getFilename())); objectFiles.add(artifact); return this; } public Builder addObjectFiles(Iterable artifacts) { + for (Artifact artifact : artifacts) { + Preconditions.checkArgument(Link.OBJECT_FILETYPES.matches(artifact.getFilename())); + } Iterables.addAll(objectFiles, artifacts); return this; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java index f5c579d1b8..230d7d12a1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java @@ -51,7 +51,6 @@ import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; - import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -62,7 +61,6 @@ import java.util.Map; import java.util.Set; import java.util.TreeMap; import java.util.regex.Pattern; - import javax.annotation.Nullable; /** @@ -249,6 +247,7 @@ public final class CcLibraryHelper { private final Set compilationUnitSources = new LinkedHashSet<>(); private final List objectFiles = new ArrayList<>(); private final List picObjectFiles = new ArrayList<>(); + private final List nonCodeLinkerInputs = new ArrayList<>(); private final List copts = new ArrayList<>(); private final List linkopts = new ArrayList<>(); @Nullable private Pattern nocopts; @@ -334,7 +333,7 @@ public final class CcLibraryHelper { .addDefines(common.getDefines()) .addDeps(ruleContext.getPrerequisites("deps", Mode.TARGET)) .addLooseIncludeDirs(common.getLooseIncludeDirs()) - .addPicIndependentObjectFiles(common.getLinkerScripts()) + .addNonCodeLinkerInputs(common.getLinkerScripts()) .addSystemIncludeDirs(common.getSystemIncludeDirs()) .setNoCopts(common.getNoCopts()) .setHeadersCheckingMode(semantics.determineHeadersCheckingMode(ruleContext)); @@ -513,18 +512,18 @@ public final class CcLibraryHelper { } /** - * Add the corresponding files as linker inputs for both PIC and non-PIC links. + * Adds the corresponding non-code files as linker inputs. */ - public CcLibraryHelper addPicIndependentObjectFiles(Iterable objectFiles) { - addPicObjectFiles(objectFiles); - return addObjectFiles(objectFiles); - } + public CcLibraryHelper addNonCodeLinkerInputs(Iterable nonCodeLinkerInputs) { + for (Artifact nonCodeLinkerInput : nonCodeLinkerInputs) { + String basename = nonCodeLinkerInput.getFilename(); + Preconditions.checkArgument(!Link.OBJECT_FILETYPES.matches(basename)); + Preconditions.checkArgument(!Link.ARCHIVE_LIBRARY_FILETYPES.matches(basename)); + Preconditions.checkArgument(!Link.SHARED_LIBRARY_FILETYPES.matches(basename)); + this.nonCodeLinkerInputs.add(nonCodeLinkerInput); + } - /** - * Add the corresponding files as linker inputs for both PIC and non-PIC links. - */ - public CcLibraryHelper addPicIndependentObjectFiles(Artifact... objectFiles) { - return addPicIndependentObjectFiles(Arrays.asList(objectFiles)); + return this; } /** @@ -913,7 +912,7 @@ public final class CcLibraryHelper { // generate any link actions, effectively disabling header checking in some cases. if (linkType.isStaticLibraryLink()) { // TODO(bazel-team): This can't create the link action for a cc_binary yet. - ccLinkingOutputs = model.createCcLinkActions(ccOutputs); + ccLinkingOutputs = model.createCcLinkActions(ccOutputs, nonCodeLinkerInputs); } } CcLinkingOutputs originalLinkingOutputs = ccLinkingOutputs; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java index b3b293bfb1..4b0980f5d6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompilationContext.java @@ -23,7 +23,6 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.MiddlemanFactory; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration; 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; @@ -31,7 +30,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; - import java.util.LinkedHashSet; import java.util.Map; import java.util.Objects; @@ -663,13 +661,17 @@ public final class CppCompilationContext implements TransitiveInfoProvider { public Builder addCompilationPrerequisites(Iterable prerequisites) { // LIPO collector must not add compilation prerequisites in order to avoid // the creation of a middleman action. + for (Artifact prerequisite : prerequisites) { + Preconditions.checkArgument(!Link.OBJECT_FILETYPES.matches(prerequisite.getFilename())); + } Iterables.addAll(compilationPrerequisites, prerequisites); return this; } /** * Add a single include directory to be added with "-I". It can be either - * relative to the exec root (see {@link BuildConfiguration#getExecRoot}) or + * relative to the exec root (see + * {@link com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot}) or * absolute. Before it is stored, the include directory is normalized. */ public Builder addIncludeDir(PathFragment includeDir) { @@ -680,8 +682,8 @@ public final class CppCompilationContext implements TransitiveInfoProvider { /** * Add multiple include directories to be added with "-I". These can be * either relative to the exec root (see {@link - * BuildConfiguration#getExecRoot}) or absolute. The entries are normalized - * before they are stored. + * com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot}) or absolute. The + * entries are normalized before they are stored. */ public Builder addIncludeDirs(Iterable includeDirs) { for (PathFragment includeDir : includeDirs) { @@ -693,8 +695,8 @@ public final class CppCompilationContext implements TransitiveInfoProvider { /** * Add a single include directory to be added with "-iquote". It can be * either relative to the exec root (see {@link - * BuildConfiguration#getExecRoot}) or absolute. Before it is stored, the - * include directory is normalized. + * com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot}) or absolute. Before it + * is stored, the include directory is normalized. */ public Builder addQuoteIncludeDir(PathFragment quoteIncludeDir) { quoteIncludeDirs.add(quoteIncludeDir.normalize()); @@ -704,8 +706,8 @@ public final class CppCompilationContext implements TransitiveInfoProvider { /** * Add a single include directory to be added with "-isystem". It can be * either relative to the exec root (see {@link - * BuildConfiguration#getExecRoot}) or absolute. Before it is stored, the - * include directory is normalized. + * com.google.devtools.build.lib.analysis.BlazeDirectories#getExecRoot}) or absolute. Before it + * is stored, the include directory is normalized. */ public Builder addSystemIncludeDir(PathFragment systemIncludeDir) { systemIncludeDirs.add(systemIncludeDir.normalize()); 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 5a7d0717eb..32782a70d8 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 @@ -18,7 +18,6 @@ import static java.nio.charset.StandardCharsets.ISO_8859_1; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; -import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -51,12 +50,10 @@ import com.google.devtools.build.lib.util.ShellEscaper; 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.List; import java.util.Map; - import javax.annotation.Nullable; /** @@ -161,24 +158,6 @@ public final class CppLinkAction extends AbstractAction implements ExecutionInfo this.executionRequirements = executionRequirements; } - static Iterable filterLinkerInputs(Iterable inputs) { - return Iterables.filter(inputs, new Predicate() { - @Override - public boolean apply(LinkerInput input) { - return Link.VALID_LINKER_INPUTS.matches(input.getArtifact().getFilename()); - } - }); - } - - static Iterable filterLinkerInputArtifacts(Iterable inputs) { - return Iterables.filter(inputs, new Predicate() { - @Override - public boolean apply(Artifact input) { - return Link.VALID_LINKER_INPUTS.matches(input.getFilename()); - } - }); - } - private CppConfiguration getCppConfiguration() { return cppConfiguration; } @@ -519,7 +498,8 @@ public final class CppLinkAction extends AbstractAction implements ExecutionInfo public static final class Context implements TransitiveInfoProvider { // Morally equivalent with {@link Builder}, except these are immutable. // Keep these in sync with {@link Builder}. - final ImmutableSet nonLibraries; + final ImmutableSet objectFiles; + final ImmutableSet nonCodeInputs; final NestedSet libraries; final NestedSet crosstoolInputs; final Artifact runtimeMiddleman; @@ -540,7 +520,8 @@ public final class CppLinkAction extends AbstractAction implements ExecutionInfo * @param builder a mutable {@link CppLinkActionBuilder} to clone from */ public Context(CppLinkActionBuilder builder) { - this.nonLibraries = ImmutableSet.copyOf(builder.getNonLibraries()); + this.objectFiles = ImmutableSet.copyOf(builder.getObjectFiles()); + this.nonCodeInputs = ImmutableSet.copyOf(builder.getNonCodeInputs()); this.libraries = NestedSetBuilder.linkOrder() .addTransitive(builder.getLibraries().build()).build(); this.crosstoolInputs = @@ -562,8 +543,8 @@ public final class CppLinkAction extends AbstractAction implements ExecutionInfo /** * Returns linker inputs that are not libraries. */ - public ImmutableSet getNonLibraries() { - return this.nonLibraries; + public ImmutableSet getObjectFiles() { + return this.objectFiles; } /** 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 1e5e418a85..881eec63de 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 @@ -123,7 +123,8 @@ public class CppLinkActionBuilder { // Morally equivalent with {@link Context}, except these are mutable. // Keep these in sync with {@link Context}. - private final Set nonLibraries = new LinkedHashSet<>(); + private final Set objectFiles = new LinkedHashSet<>(); + private final Set nonCodeInputs = new LinkedHashSet<>(); private final NestedSetBuilder libraries = NestedSetBuilder.linkOrder(); private NestedSet crosstoolInputs = NestedSetBuilder.emptySet(Order.STABLE_ORDER); private Artifact runtimeMiddleman; @@ -224,7 +225,8 @@ public class CppLinkActionBuilder { Preconditions.checkNotNull(linkContext); // All linkContext fields should be transferred to this Builder. - this.nonLibraries.addAll(linkContext.nonLibraries); + this.objectFiles.addAll(linkContext.objectFiles); + this.nonCodeInputs.addAll(linkContext.nonCodeInputs); this.libraries.addTransitive(linkContext.libraries); this.crosstoolInputs = linkContext.crosstoolInputs; this.runtimeMiddleman = linkContext.runtimeMiddleman; @@ -245,10 +247,14 @@ public class CppLinkActionBuilder { } /** Returns linker inputs that are not libraries. */ - public Set getNonLibraries() { - return nonLibraries; + public Set getObjectFiles() { + return objectFiles; } - + + public Set getNonCodeInputs() { + return nonCodeInputs; + } + /** * Returns linker inputs that are libraries. */ @@ -369,14 +375,9 @@ public class CppLinkActionBuilder { } } } - for (LinkerInput input : nonLibraries) { - // This relies on file naming conventions. It would be less fragile to have a dedicated - // field for non-library .o files. - if (CppFileTypes.OBJECT_FILE.matches(input.getArtifact().getExecPath()) - || CppFileTypes.PIC_OBJECT_FILE.matches(input.getArtifact().getExecPath())) { - if (this.ltoBitcodeFiles.contains(input.getArtifact())) { - allBitcode.put(input.getArtifact().getExecPath(), input.getArtifact()); - } + for (LinkerInput input : objectFiles) { + if (this.ltoBitcodeFiles.contains(input.getArtifact())) { + allBitcode.put(input.getArtifact().getExecPath(), input.getArtifact()); } } @@ -432,12 +433,11 @@ public class CppLinkActionBuilder { || needWholeArchive(linkStaticness, linkType, linkopts, isNativeDeps, cppConfiguration); NestedSet uniqueLibraries = libraries.build(); - final Iterable filteredNonLibraryArtifacts = - CppLinkAction.filterLinkerInputArtifacts(LinkerInputs.toLibraryArtifacts(nonLibraries)); + final Iterable objectArtifacts = LinkerInputs.toLibraryArtifacts(objectFiles); final Iterable linkerInputs = IterablesChain.builder() - .add(ImmutableList.copyOf(CppLinkAction.filterLinkerInputs(nonLibraries))) + .add(ImmutableList.copyOf(objectFiles)) .add( ImmutableIterable.from( Link.mergeInputsCmdLine( @@ -460,12 +460,12 @@ public class CppLinkActionBuilder { } final LibraryToLink outputLibrary = LinkerInputs.newInputLibrary( - output, libraryIdentifier, filteredNonLibraryArtifacts, this.ltoBitcodeFiles); + output, libraryIdentifier, objectArtifacts, this.ltoBitcodeFiles); final LibraryToLink interfaceOutputLibrary = (interfaceOutput == null) ? null : LinkerInputs.newInputLibrary(interfaceOutput, libraryIdentifier, - filteredNonLibraryArtifacts, this.ltoBitcodeFiles); + objectArtifacts, this.ltoBitcodeFiles); final ImmutableMap linkstampMap = mapLinkstampsToOutputs(linkstamps, ruleContext, configuration, output, linkArtifactFactory); @@ -610,7 +610,8 @@ public class CppLinkActionBuilder { LinkerInputs.toLibraryArtifacts( Link.mergeInputsDependencies( uniqueLibraries, needWholeArchive, cppConfiguration.archiveType())); - Iterable expandedNonLibraryInputs = LinkerInputs.toLibraryArtifacts(nonLibraries); + Iterable expandedNonLibraryInputs = LinkerInputs.toLibraryArtifacts(objectFiles); + if (!isLTOIndexing && allLTOArtifacts != null) { // We are doing LTO, and this is the real link, so substitute // the LTO bitcode files with the real object files they were translated into. @@ -649,6 +650,7 @@ public class CppLinkActionBuilder { IterablesChain.Builder inputsBuilder = IterablesChain.builder() .add(ImmutableList.copyOf(expandedNonLibraryInputs)) + .add(ImmutableList.copyOf(nonCodeInputs)) .add(dependencyInputsBuilder.build()) .add(ImmutableIterable.from(expandedInputs)); @@ -824,14 +826,10 @@ public class CppLinkActionBuilder { return this; } - private void addNonLibraryInput(LinkerInput input) { + private void addObjectFile(LinkerInput input) { String name = input.getArtifact().getFilename(); - Preconditions.checkArgument( - !Link.ARCHIVE_LIBRARY_FILETYPES.matches(name) - && !Link.SHARED_LIBRARY_FILETYPES.matches(name), - "'%s' is a library file", - input); - this.nonLibraries.add(input); + Preconditions.checkArgument(Link.OBJECT_FILETYPES.matches(name), name); + this.objectFiles.add(input); } public CppLinkActionBuilder addLTOBitcodeFiles(Iterable files) { @@ -842,30 +840,52 @@ public class CppLinkActionBuilder { } /** - * Adds a single artifact to the set of inputs (C++ source files, header files, etc). Artifacts - * that are not of recognized types will be used for dependency checking but will not be passed to - * the linker. The artifact must not be an archive or a shared library. + * Adds a single object file to the set of inputs. + */ + public CppLinkActionBuilder addObjectFile(Artifact input) { + addObjectFile(LinkerInputs.simpleLinkerInput(input)); + return this; + } + + /** + * Adds object files to the linker action. */ - public CppLinkActionBuilder addNonLibraryInput(Artifact input) { - addNonLibraryInput(LinkerInputs.simpleLinkerInput(input)); + public CppLinkActionBuilder addObjectFiles(Iterable inputs) { + for (Artifact input : inputs) { + addObjectFile(LinkerInputs.simpleLinkerInput(input)); + } return this; } /** - * Adds multiple artifacts to the set of inputs (C++ source files, header files, etc). Artifacts - * that are not of recognized types will be used for dependency checking but will not be passed to - * the linker. The artifacts must not be archives or shared libraries. + * Adds non-code files to the set of inputs. They will not be passed to the linker command line + * unless that is explicitly modified, too. */ - public CppLinkActionBuilder addNonLibraryInputs(Iterable inputs) { + public CppLinkActionBuilder addNonCodeInputs(Iterable inputs) { for (Artifact input : inputs) { - addNonLibraryInput(LinkerInputs.simpleLinkerInput(input)); + addNonCodeInput(input); } + + return this; + } + + /** + * Adds a single non-code file to the set of inputs. It will not be passed to the linker command + * line unless that is explicitly modified, too. + */ + public CppLinkActionBuilder addNonCodeInput(Artifact input) { + String basename = input.getFilename(); + Preconditions.checkArgument(!Link.ARCHIVE_LIBRARY_FILETYPES.matches(basename), basename); + Preconditions.checkArgument(!Link.SHARED_LIBRARY_FILETYPES.matches(basename), basename); + Preconditions.checkArgument(!Link.OBJECT_FILETYPES.matches(basename), basename); + + this.nonCodeInputs.add(input); return this; } public CppLinkActionBuilder addFakeNonLibraryInputs(Iterable inputs) { for (Artifact input : inputs) { - addNonLibraryInput(LinkerInputs.fakeLinkerInput(input)); + addObjectFile(LinkerInputs.fakeLinkerInput(input)); } return this; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java index d7429af765..1c08ea1582 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java @@ -809,8 +809,8 @@ public final class CppModel { * * @throws RuleErrorException */ - public CcLinkingOutputs createCcLinkActions(CcCompilationOutputs ccOutputs) - throws RuleErrorException { + public CcLinkingOutputs createCcLinkActions(CcCompilationOutputs ccOutputs, + Iterable nonCodeLinkerInputs) throws RuleErrorException { // For now only handle static links. Note that the dynamic library link below ignores linkType. // TODO(bazel-team): Either support non-static links or move this check to setLinkType(). Preconditions.checkState(linkType.isStaticLibraryLink(), "can only handle static links"); @@ -848,8 +848,9 @@ public final class CppModel { labelName.replaceName("lib" + labelName.getBaseName())).getPathString(); CppLinkAction maybePicAction = newLinkActionBuilder(linkedArtifact) - .addNonLibraryInputs(ccOutputs.getObjectFiles(usePicForBinaries)) - .addNonLibraryInputs(ccOutputs.getHeaderTokenFiles()) + .addObjectFiles(ccOutputs.getObjectFiles(usePicForBinaries)) + .addNonCodeInputs(ccOutputs.getHeaderTokenFiles()) + .addNonCodeInputs(nonCodeLinkerInputs) .addLTOBitcodeFiles(ccOutputs.getLtoBitcodeFiles()) .setLinkType(linkType) .setLinkStaticness(LinkStaticness.FULLY_STATIC) @@ -873,8 +874,8 @@ public final class CppModel { CppLinkAction picAction = newLinkActionBuilder(picArtifact) - .addNonLibraryInputs(ccOutputs.getObjectFiles(true)) - .addNonLibraryInputs(ccOutputs.getHeaderTokenFiles()) + .addObjectFiles(ccOutputs.getObjectFiles(true)) + .addObjectFiles(ccOutputs.getHeaderTokenFiles()) .addLTOBitcodeFiles(ccOutputs.getLtoBitcodeFiles()) .setLinkType(picLinkType) .setLinkStaticness(LinkStaticness.FULLY_STATIC) @@ -922,8 +923,8 @@ public final class CppModel { CppLinkActionBuilder linkActionBuilder = newLinkActionBuilder(soImpl) .setInterfaceOutput(soInterface) - .addNonLibraryInputs(ccOutputs.getObjectFiles(usePicForSharedLibs)) - .addNonLibraryInputs(ccOutputs.getHeaderTokenFiles()) + .addObjectFiles(ccOutputs.getObjectFiles(usePicForSharedLibs)) + .addNonCodeInputs(ccOutputs.getHeaderTokenFiles()) .addLTOBitcodeFiles(ccOutputs.getLtoBitcodeFiles()) .setLinkType(LinkTargetType.DYNAMIC_LIBRARY) .setLinkStaticness(LinkStaticness.DYNAMIC) @@ -985,7 +986,7 @@ public final class CppModel { private CppLinkActionBuilder newLinkActionBuilder(Artifact outputArtifact) { return new CppLinkActionBuilder(ruleContext, outputArtifact) .setCrosstoolInputs(CppHelper.getToolchain(ruleContext).getLink()) - .addNonLibraryInputs(context.getTransitiveCompilationPrerequisites()); + .addNonCodeInputs(context.getTransitiveCompilationPrerequisites()); } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java index 1e3e313ab0..f75721d36f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java @@ -37,14 +37,6 @@ public abstract class Link { private Link() {} // uninstantiable - /** The set of valid linker input files. */ - public static final FileTypeSet VALID_LINKER_INPUTS = FileTypeSet.of( - CppFileTypes.ARCHIVE, CppFileTypes.PIC_ARCHIVE, - CppFileTypes.ALWAYS_LINK_LIBRARY, CppFileTypes.ALWAYS_LINK_PIC_LIBRARY, - CppFileTypes.OBJECT_FILE, CppFileTypes.PIC_OBJECT_FILE, - CppFileTypes.SHARED_LIBRARY, CppFileTypes.VERSIONED_SHARED_LIBRARY, - CppFileTypes.INTERFACE_SHARED_LIBRARY); - /** * These file are supposed to be added using {@code addLibrary()} calls to {@link CppLinkAction} * but will never be expanded to their constituent {@code .o} files. {@link CppLinkAction} checks diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java index 1c6f7548b3..4462417b04 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java @@ -299,12 +299,12 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase { .setupCrosstool(mockToolsConfig, "artifact_name_pattern {" + " category_name: 'object_file'" - + " pattern: '%{base_name}.obj'" + + " pattern: 'object_%{base_name}.o'" + "}"); useConfiguration(); ConfiguredTarget hello = getConfiguredTarget("//hello:hello"); - assertThat(artifactByPath(getFilesToBuild(hello), ".a", "hello.obj")).isNotNull(); + assertThat(artifactByPath(getFilesToBuild(hello), ".a", "object_hello.o")).isNotNull(); } @Test 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 863e7c1bf1..963d831af6 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 @@ -319,7 +319,7 @@ public class CppLinkActionTest extends BuildViewTestCase { new PathFragment(outputPath), getTargetConfiguration().getBinDirectory()), ruleContext.getConfiguration(), shouldIncludeToolchain ? CppHelper.getToolchain(ruleContext) : null) - .addNonLibraryInputs(nonLibraryInputs) + .addObjectFiles(nonLibraryInputs) .addLibraries(NestedSetBuilder.wrap(Order.LINK_ORDER, libraryInputs)) .setLinkType(type) .setCrosstoolInputs(NestedSetBuilder.emptySet(Order.STABLE_ORDER)) -- cgit v1.2.3