From 2661abb96b1fe51fb726a63eb08698564a82eb20 Mon Sep 17 00:00:00 2001 From: hlopko Date: Wed, 16 May 2018 22:45:06 -0700 Subject: Split user_link_flags from legacy_link_flags The difference between them is that user_link_flags will stay after we remove legacy fields from the crosstool. RELNOTES: None. PiperOrigin-RevId: 196940832 --- .../devtools/build/lib/rules/cpp/CcCommon.java | 4 +-- .../build/lib/rules/cpp/CppActionConfigs.java | 18 ++++++++++ .../build/lib/rules/cpp/CppCompileAction.java | 1 + .../build/lib/rules/cpp/CppLinkActionBuilder.java | 27 ++++++-------- .../google/devtools/build/lib/rules/cpp/Link.java | 42 +++++++++++++++------- .../build/lib/rules/cpp/LinkBuildVariables.java | 2 ++ .../build/lib/rules/cpp/LinkCommandLine.java | 27 +++++--------- 7 files changed, 71 insertions(+), 50 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/rules/cpp') diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java index 21ccb4a116..d5f1fba1f8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java @@ -116,9 +116,9 @@ public final class CcCommon { public static final ImmutableSet ALL_LINK_ACTIONS = ImmutableSet.of( + LinkTargetType.EXECUTABLE.getActionName(), Link.LinkTargetType.DYNAMIC_LIBRARY.getActionName(), - Link.LinkTargetType.NODEPS_DYNAMIC_LIBRARY.getActionName(), - LinkTargetType.EXECUTABLE.getActionName()); + Link.LinkTargetType.NODEPS_DYNAMIC_LIBRARY.getActionName()); public static final ImmutableSet ALL_ARCHIVE_ACTIONS = ImmutableSet.of(Link.LinkTargetType.STATIC_LIBRARY.getActionName()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java index efd741a6c3..36d754e61e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java @@ -462,6 +462,7 @@ public class CppActionConfigs { " implies: 'library_search_directories'", " implies: 'libraries_to_link'", " implies: 'force_pic_flags'", + " implies: 'user_link_flags'", " implies: 'legacy_link_flags'", " implies: 'linker_param_file'", " implies: 'fission_support'", @@ -485,6 +486,7 @@ public class CppActionConfigs { " implies: 'runtime_library_search_directories'", " implies: 'library_search_directories'", " implies: 'libraries_to_link'", + " implies: 'user_link_flags'", " implies: 'legacy_link_flags'", " implies: 'linker_param_file'", " implies: 'fission_support'", @@ -508,6 +510,7 @@ public class CppActionConfigs { " implies: 'runtime_library_search_directories'", " implies: 'library_search_directories'", " implies: 'libraries_to_link'", + " implies: 'user_link_flags'", " implies: 'legacy_link_flags'", " implies: 'linker_param_file'", " implies: 'fission_support'", @@ -853,6 +856,21 @@ public class CppActionConfigs { " }", " }", "}"), + ifTrue( + !existingFeatureNames.contains("user_link_flags"), + "feature {", + " name: 'user_link_flags'", + " flag_set {", + " expand_if_all_available: 'user_link_flags'", + " action: 'c++-link-executable'", + " action: 'c++-link-dynamic-library'", + " action: 'c++-link-nodeps-dynamic-library'", + " flag_group {", + " iterate_over: 'user_link_flags'", + " flag: '%{user_link_flags}'", + " }", + " }", + "}"), ifTrue( !existingFeatureNames.contains("legacy_link_flags"), "feature {", diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java index d5e692f75f..b338d5b173 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java @@ -86,6 +86,7 @@ import javax.annotation.Nullable; @ThreadCompatible public class CppCompileAction extends AbstractAction implements IncludeScannable, ExecutionInfoSpecifier, CommandAction { + private static final PathFragment BUILD_PATH_FRAGMENT = PathFragment.create("BUILD"); private static final int VALIDATION_DEBUG = 0; // 0==none, 1==warns/errors, 2==all 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 9ec07a3177..69294f04ef 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 @@ -686,19 +686,6 @@ public class CppLinkActionBuilder { List result = new ArrayList<>(); - /* - * For backwards compatibility, linkopts come _after_ inputFiles. - * This is needed to allow linkopts to contain libraries and - * positional library-related options such as - * -Wl,--begin-group -lfoo -lbar -Wl,--end-group - * or - * -Wl,--as-needed -lfoo -Wl,--no-as-needed - * - * As for the relative order of the three different flavours of linkopts - * (global defaults, per-target linkopts, and command-line linkopts), - * we have no idea what the right order should be, or if anyone cares. - */ - result.addAll(linkopts); // Extra toolchain link options based on the output's link staticness. if (fullyStatic) { result.addAll( @@ -734,11 +721,15 @@ public class CppLinkActionBuilder { // distinguish between shared libraries and executables, we could add additional // command line / CROSSTOOL flags that distinguish them. But as long as this is // the only relevant use case we're just special-casing it here. + return ImmutableList.copyOf(removePieIfCreatingSharedLibrary(result)); + } + + private Iterable removePieIfCreatingSharedLibrary(List flags) { if (linkType == LinkTargetType.DYNAMIC_LIBRARY) { - Iterables.removeIf(result, Predicates.equalTo("-pie")); + return Iterables.filter(flags, Predicates.not(Predicates.equalTo("-pie"))); + } else { + return flags; } - - return ImmutableList.copyOf(result); } /** Builds the Action as configured and returns it. */ @@ -1084,10 +1075,12 @@ public class CppLinkActionBuilder { linkType.linkerOrArchiver() == LinkerOrArchiver.ARCHIVER ? ImmutableList.of() : linkoptsForVariables; - linkCommandLineBuilder.setLinkopts(linkoptsForVariables); CcToolchainVariables patchedVariables = new CcToolchainVariables.Builder(buildVariables) + .addStringSequenceVariable( + LinkBuildVariables.USER_LINK_FLAGS.getVariableName(), + removePieIfCreatingSharedLibrary(linkoptsForVariables)) .addStringSequenceVariable( LinkBuildVariables.LEGACY_LINK_FLAGS.getVariableName(), getToolchainFlags(linkoptsForVariables)) 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 76018356ea..ffbfd76b8d 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 @@ -28,6 +28,24 @@ import com.google.devtools.build.lib.util.FileTypeSet; */ public abstract class Link { + /** Name of the action producing static library. */ + public static final String CPP_LINK_STATIC_LIBRARY_ACTION_NAME = "c++-link-static-library"; + /** Name of the action producing dynamic library from cc_library. */ + public static final String CPP_LINK_NODEPS_DYNAMIC_LIBRARY_ACTION_NAME = + "c++-link-nodeps-dynamic-library"; + /** Name of the action producing dynamic library from cc_binary. */ + public static final String CPP_LINK_DYNAMIC_LIBRARY_ACTION_NAME = "c++-link-dynamic-library"; + /** Name of the action producing executable binary. */ + public static final String CPP_LINK_EXECUTABLE_ACTION_NAME = "c++-link-executable"; + /** Name of the objc action producing static library */ + public static final String OBJC_ARCHIVE_ACTION_NAME = "objc-archive"; + /** Name of the objc action producing dynamic library */ + public static final String OBJC_FULLY_LINK_ACTION_NAME = "objc-fully-link"; + /** Name of the objc action producing objc executable binary */ + public static final String OBJC_EXECUTABLE_ACTION_NAME = "objc-executable"; + /** Name of the objc action producing objc++ executable binary */ + public static final String OBJCPP_EXECUTABLE_ACTION_NAME = "objc++-executable"; + private Link() {} // uninstantiable /** @@ -98,7 +116,7 @@ public abstract class Link { STATIC_LIBRARY( ".a", LinkerOrArchiver.ARCHIVER, - "c++-link-static-library", + CPP_LINK_STATIC_LIBRARY_ACTION_NAME, Picness.NOPIC, ArtifactCategory.STATIC_LIBRARY, Executable.NOT_EXECUTABLE), @@ -107,7 +125,7 @@ public abstract class Link { OBJC_ARCHIVE( ".a", LinkerOrArchiver.ARCHIVER, - "objc-archive", + OBJC_ARCHIVE_ACTION_NAME, Picness.NOPIC, ArtifactCategory.STATIC_LIBRARY, Executable.NOT_EXECUTABLE), @@ -116,7 +134,7 @@ public abstract class Link { OBJC_FULLY_LINKED_ARCHIVE( ".a", LinkerOrArchiver.ARCHIVER, - "objc-fully-link", + OBJC_FULLY_LINK_ACTION_NAME, Picness.NOPIC, ArtifactCategory.STATIC_LIBRARY, Executable.NOT_EXECUTABLE), @@ -125,7 +143,7 @@ public abstract class Link { OBJC_EXECUTABLE( "", LinkerOrArchiver.LINKER, - "objc-executable", + OBJC_EXECUTABLE_ACTION_NAME, Picness.NOPIC, ArtifactCategory.EXECUTABLE, Executable.EXECUTABLE), @@ -134,7 +152,7 @@ public abstract class Link { OBJCPP_EXECUTABLE( "", LinkerOrArchiver.LINKER, - "objc++-executable", + OBJCPP_EXECUTABLE_ACTION_NAME, Picness.NOPIC, ArtifactCategory.EXECUTABLE, Executable.EXECUTABLE), @@ -143,7 +161,7 @@ public abstract class Link { PIC_STATIC_LIBRARY( ".pic.a", LinkerOrArchiver.ARCHIVER, - "c++-link-static-library", + CPP_LINK_STATIC_LIBRARY_ACTION_NAME, Picness.PIC, ArtifactCategory.STATIC_LIBRARY, Executable.NOT_EXECUTABLE), @@ -152,7 +170,7 @@ public abstract class Link { INTERFACE_DYNAMIC_LIBRARY( ".ifso", LinkerOrArchiver.LINKER, - "c++-link-dynamic-library", + CPP_LINK_DYNAMIC_LIBRARY_ACTION_NAME, Picness.NOPIC, // Actually PIC but it's not indicated in the file name ArtifactCategory.INTERFACE_LIBRARY, Executable.NOT_EXECUTABLE), @@ -161,7 +179,7 @@ public abstract class Link { NODEPS_DYNAMIC_LIBRARY( ".so", LinkerOrArchiver.LINKER, - "c++-link-nodeps-dynamic-library", + CPP_LINK_NODEPS_DYNAMIC_LIBRARY_ACTION_NAME, Picness.NOPIC, // Actually PIC but it's not indicated in the file name ArtifactCategory.DYNAMIC_LIBRARY, Executable.NOT_EXECUTABLE), @@ -169,7 +187,7 @@ public abstract class Link { DYNAMIC_LIBRARY( ".so", LinkerOrArchiver.LINKER, - "c++-link-dynamic-library", + CPP_LINK_DYNAMIC_LIBRARY_ACTION_NAME, Picness.NOPIC, // Actually PIC but it's not indicated in the file name ArtifactCategory.DYNAMIC_LIBRARY, Executable.NOT_EXECUTABLE), @@ -178,7 +196,7 @@ public abstract class Link { ALWAYS_LINK_STATIC_LIBRARY( ".lo", LinkerOrArchiver.ARCHIVER, - "c++-link-static-library", + CPP_LINK_STATIC_LIBRARY_ACTION_NAME, Picness.NOPIC, ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY, Executable.NOT_EXECUTABLE), @@ -187,7 +205,7 @@ public abstract class Link { ALWAYS_LINK_PIC_STATIC_LIBRARY( ".pic.lo", LinkerOrArchiver.ARCHIVER, - "c++-link-static-library", + CPP_LINK_STATIC_LIBRARY_ACTION_NAME, Picness.PIC, ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY, Executable.NOT_EXECUTABLE), @@ -196,7 +214,7 @@ public abstract class Link { EXECUTABLE( "", LinkerOrArchiver.LINKER, - "c++-link-executable", + CPP_LINK_EXECUTABLE_ACTION_NAME, Picness.NOPIC, // Picness is not indicate in the file name ArtifactCategory.EXECUTABLE, Executable.EXECUTABLE); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java index 34d39c3690..248c1b4a7c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java @@ -61,6 +61,8 @@ public enum LinkBuildVariables { INTERFACE_LIBRARY_OUTPUT("interface_library_output_path"), /** Linker flags coming from the legacy crosstool fields. */ LEGACY_LINK_FLAGS("legacy_link_flags"), + /** Linker flags coming from the --linkopt or linkopts attribute. */ + USER_LINK_FLAGS("user_link_flags"), /** Path to which to write symbol counts. */ SYMBOL_COUNTS_OUTPUT("symbol_counts_output"), /** A build variable giving linkstamp paths. */ 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 a1c037f96e..19a646506d 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 @@ -50,7 +50,6 @@ public final class LinkCommandLine extends CommandLine { private final Iterable linkerInputArtifacts; private final LinkTargetType linkTargetType; private final Link.LinkingMode linkingMode; - private final ImmutableList linkopts; @Nullable private final PathFragment toolchainLibrariesSolibDir; private final boolean nativeDeps; private final boolean useTestOnlyFlags; @@ -65,7 +64,6 @@ public final class LinkCommandLine extends CommandLine { Iterable linkerInputArtifacts, LinkTargetType linkTargetType, Link.LinkingMode linkingMode, - ImmutableList linkopts, @Nullable PathFragment toolchainLibrariesSolibDir, boolean nativeDeps, boolean useTestOnlyFlags, @@ -81,7 +79,6 @@ public final class LinkCommandLine extends CommandLine { this.linkerInputArtifacts = Preconditions.checkNotNull(linkerInputArtifacts); this.linkTargetType = Preconditions.checkNotNull(linkTargetType); this.linkingMode = Preconditions.checkNotNull(linkingMode); - this.linkopts = linkopts; this.toolchainLibrariesSolibDir = toolchainLibrariesSolibDir; this.nativeDeps = nativeDeps; this.useTestOnlyFlags = useTestOnlyFlags; @@ -119,7 +116,12 @@ public final class LinkCommandLine extends CommandLine { * Returns the additional linker options for this link. */ public ImmutableList getLinkopts() { - return linkopts; + if (variables.isAvailable(LinkBuildVariables.USER_LINK_FLAGS.getVariableName())) { + return CcToolchainVariables.toStringList( + variables, LinkBuildVariables.USER_LINK_FLAGS.getVariableName()); + } else { + return ImmutableList.of(); + } } /** Returns the path to the linker. */ @@ -413,7 +415,6 @@ public final class LinkCommandLine extends CommandLine { private Iterable linkerInputArtifacts = ImmutableList.of(); @Nullable private LinkTargetType linkTargetType; private Link.LinkingMode linkingMode = Link.LinkingMode.LEGACY_FULLY_STATIC; - private ImmutableList linkopts = ImmutableList.of(); @Nullable private PathFragment toolchainLibrariesSolibDir; private boolean nativeDeps; private boolean useTestOnlyFlags; @@ -437,10 +438,10 @@ public final class LinkCommandLine extends CommandLine { if (ruleContext != null) { Preconditions.checkNotNull(featureConfiguration); } - + if (variables == null) { variables = CcToolchainVariables.EMPTY; - } + } String actionName = linkTargetType.getActionName(); @@ -451,7 +452,6 @@ public final class LinkCommandLine extends CommandLine { linkerInputArtifacts, linkTargetType, linkingMode, - linkopts, toolchainLibrariesSolibDir, nativeDeps, useTestOnlyFlags, @@ -494,17 +494,6 @@ public final class LinkCommandLine extends CommandLine { return this; } - /** - * Sets the linker options. These are passed to the linker in addition to the other linker - * options like linker inputs, symbol count options, etc. The {@link #build} method throws an - * exception if the linker options are non-empty for a static link (see {@link - * LinkTargetType#linkerOrArchiver()}). - */ - public Builder setLinkopts(ImmutableList linkopts) { - this.linkopts = linkopts; - return this; - } - /** * Sets how static the link is supposed to be. For static target types (see {@link * LinkTargetType#linkerOrArchiver()}}), the {@link #build} method throws an exception if this -- cgit v1.2.3