From d62d2940f6b6b8850da08d3184965d86321deedc Mon Sep 17 00:00:00 2001 From: hlopko Date: Tue, 12 Feb 2019 04:46:03 -0800 Subject: [PATCH] Flip --incompatible_linkopts_in_user_link_flags Closes #6826. This is a roll-forward of https://github.com/bazelbuild/bazel/commit/4866f3334b3bf0c8faac577d50bdb9df95f5460d after Bazel with the flag was cut. RELNOTES: Incompatible flag `--incompatible_linkopts_in_user_link_flags` has been flipped (https://github.com/bazelbuild/bazel/issues/6826) PiperOrigin-RevId: 233584729 --- .../lib/bazel/rules/BazelRulesModule.java | 13 ++++++++++ .../build/lib/rules/cpp/CppConfiguration.java | 4 --- .../lib/rules/cpp/CppLinkActionBuilder.java | 15 +++-------- .../build/lib/rules/cpp/CppOptions.java | 15 ----------- .../lib/rules/cpp/LinkBuildVariables.java | 4 --- .../lib/rules/cpp/LinkBuildVariablesTest.java | 26 +------------------ 6 files changed, 18 insertions(+), 59 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java index 8eafb0eae634c3..8209ab9d6403c7 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java @@ -40,6 +40,19 @@ public class BazelRulesModule extends BlazeModule { /** This is where deprecated options go to die. */ public static class GraveyardOptions extends OptionsBase { + @Option( + name = "incompatible_linkopts_in_user_link_flags", + defaultValue = "true", + documentationCategory = OptionDocumentationCategory.TOOLCHAIN, + effectTags = {OptionEffectTag.ACTION_COMMAND_LINES, OptionEffectTag.LOADING_AND_ANALYSIS}, + metadataTags = { + OptionMetadataTag.DEPRECATED, + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + help = "Deprecated no-op.") + public boolean enableLinkoptsInUserLinkFlags; + @Option( name = "incompatible_disable_runtimes_filegroups", defaultValue = "false", diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index a029c4d24460a6..97de9205e57334 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -556,10 +556,6 @@ public static String getLegacyCrosstoolFieldErrorMessage(String field) { + "migration instructions)."; } - public boolean enableLinkoptsInUserLinkFlags() { - return cppOptions.enableLinkoptsInUserLinkFlags; - } - public boolean disableEmittingStaticLibgcc() { return cppOptions.disableEmittingStaticLibgcc; } 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 b92412d3d39002..187e1c8884f352 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 @@ -882,16 +882,6 @@ public CppLinkAction build() throws InterruptedException { CcToolchainVariables variables; try { - ImmutableList userLinkFlags; - if (cppConfiguration.enableLinkoptsInUserLinkFlags()) { - userLinkFlags = - ImmutableList.builder() - .addAll(linkopts) - .addAll(cppConfiguration.getLinkopts()) - .build(); - } else { - userLinkFlags = ImmutableList.copyOf(linkopts); - } variables = LinkBuildVariables.setupVariables( getLinkType().linkerOrArchiver().equals(LinkerOrArchiver.LINKER), @@ -906,7 +896,10 @@ public CppLinkAction build() throws InterruptedException { featureConfiguration, useTestOnlyFlags, isLtoIndexing, - userLinkFlags, + ImmutableList.builder() + .addAll(linkopts) + .addAll(cppConfiguration.getLinkopts()) + .build(), toolchain.getInterfaceSoBuilder().getExecPathString(), interfaceOutput != null ? interfaceOutput.getExecPathString() : null, ltoOutputRootPrefix, diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java index 1127ecf1f0d4f1..a721c2994552af 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java @@ -727,21 +727,6 @@ public Label getFdoPrefetchHintsLabel() { + "(see https://github.com/bazelbuild/bazel/issues/7008 for migration instructions).") public boolean disableExpandIfAllAvailableInFlagSet; - @Option( - name = "incompatible_linkopts_in_user_link_flags", - oldName = "experimental_linkopts_in_user_link_flags", - defaultValue = "true", - documentationCategory = OptionDocumentationCategory.TOOLCHAIN, - effectTags = {OptionEffectTag.ACTION_COMMAND_LINES, OptionEffectTag.LOADING_AND_ANALYSIS}, - metadataTags = { - OptionMetadataTag.INCOMPATIBLE_CHANGE, - OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES - }, - help = - "If true, flags coming from --linkopt Bazel option will appear in user_link_flags " - + "crosstool variable, not in legacy_link_flags.") - public boolean enableLinkoptsInUserLinkFlags; - @Option( name = "incompatible_dont_emit_static_libgcc", oldName = "experimental_dont_emit_static_libgcc", 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 101fd9e44485f6..8d30de4b45d698 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 @@ -313,10 +313,6 @@ private static ImmutableList getToolchainFlags( result.addAll(ccToolchainProvider.getTestOnlyLinkOptions()); } - if (!cppConfiguration.enableLinkoptsInUserLinkFlags()) { - result.addAll(cppConfiguration.getLinkopts()); - } - // -pie is not compatible with shared and should be // removed when the latter is part of the link command. Should we need to further // distinguish between shared libraries and executables, we could add additional diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java index be3139341c5cf2..e41b96c62c89ff 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java @@ -460,33 +460,9 @@ private Action getPredecessorByInputName(Action action, String str) { return null; } - @Test - public void testUserLinkFlags() throws Exception { - useConfiguration("--linkopt=-bar", "--noexperimental_linkopts_in_user_link_flags"); - - scratch.file("x/BUILD", "cc_binary(name = 'foo', srcs = ['a.cc'], linkopts = ['-foo'])"); - scratch.file("x/a.cc"); - - ConfiguredTarget testTarget = getConfiguredTarget("//x:foo"); - CcToolchainVariables testVariables = - getLinkBuildVariables(testTarget, LinkTargetType.EXECUTABLE); - - ImmutableList userLinkFlags = - CcToolchainVariables.toStringList( - testVariables, LinkBuildVariables.USER_LINK_FLAGS.getVariableName()); - assertThat(userLinkFlags).contains("-foo"); - assertThat(userLinkFlags).doesNotContain("-bar"); - - ImmutableList legacyLinkFlags = - CcToolchainVariables.toStringList( - testVariables, LinkBuildVariables.LEGACY_LINK_FLAGS.getVariableName()); - assertThat(legacyLinkFlags).doesNotContain("-foo"); - assertThat(legacyLinkFlags).contains("-bar"); - } - @Test public void testUserLinkFlagsWithLinkoptOption() throws Exception { - useConfiguration("--linkopt=-bar", "--experimental_linkopts_in_user_link_flags"); + useConfiguration("--linkopt=-bar"); scratch.file("x/BUILD", "cc_binary(name = 'foo', srcs = ['a.cc'], linkopts = ['-foo'])"); scratch.file("x/a.cc");