From cfa35f3750d1ae37935bdf402a0cee306672795b Mon Sep 17 00:00:00 2001
From: hlopko <hlopko@google.com>
Date: Thu, 19 Jul 2018 11:34:51 -0700
Subject: Move --linkopt flags into user_link_flags

Previous behavior was to put flags coming from Bazel option --linkopt into
legacy_link_flags. They should be in user_link_flags instead (together with
flags coming from linkopts rule attribute). This cl introduces
--experimental_linkopts_in_user_link_flags option that flips the behavior.

There is another incompatible change. Previously cc_common.create_link_variables() included flags from --linkopt, with the flag flipped it doesn't anymore. I believe
--linkopt flags shouldn't be there by default because:
* We don't tie the API with the specifics of C++ rules/options, enabling theoretical use with other languages (objc)
* Users are free to use ctx.fragments.cpp to access C++ options and add them explicitly (https://github.com/bazelbuild/bazel/issues/5602)
* New behavior maintains the symmetry with --copt and user_compile_flags

RELNOTES: None.
PiperOrigin-RevId: 205274272
---
 .../devtools/build/lib/rules/cpp/CppConfiguration.java       |  4 ++++
 .../devtools/build/lib/rules/cpp/CppLinkActionBuilder.java   | 12 +++++++++++-
 .../com/google/devtools/build/lib/rules/cpp/CppOptions.java  | 11 +++++++++++
 .../devtools/build/lib/rules/cpp/LinkBuildVariables.java     |  4 +++-
 4 files changed, 29 insertions(+), 2 deletions(-)

(limited to 'src/main')

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 8a9fc665d4..82a4ae7176 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
@@ -104,6 +104,10 @@ public final class CppConfiguration extends BuildConfiguration.Fragment
     return cppOptions.disableLinkingModeFlags;
   }
 
+  public boolean enableLinkoptsInUserLinkFlags() {
+    return cppOptions.enableLinkoptsInUserLinkFlags;
+  }
+
   /**
    * An enumeration of all the tools that comprise a toolchain.
    */
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 56bca06241..8a99e731a3 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
@@ -926,6 +926,16 @@ public class CppLinkActionBuilder {
 
     CcToolchainVariables variables;
     try {
+      ImmutableList<String> userLinkFlags;
+      if (cppConfiguration.enableLinkoptsInUserLinkFlags()) {
+        userLinkFlags =
+            ImmutableList.<String>builder()
+                .addAll(linkopts)
+                .addAll(toolchain.getLinkOptions())
+                .build();
+      } else {
+        userLinkFlags = ImmutableList.copyOf(linkopts);
+      }
       variables =
           LinkBuildVariables.setupVariables(
               getLinkType().linkerOrArchiver().equals(LinkerOrArchiver.LINKER),
@@ -941,7 +951,7 @@ public class CppLinkActionBuilder {
               featureConfiguration,
               useTestOnlyFlags,
               isLtoIndexing,
-              ImmutableList.copyOf(linkopts),
+              userLinkFlags,
               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 5992b22f5c..b5616f0890 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
@@ -860,6 +860,17 @@ public class CppOptions extends FragmentOptions {
               + "field of CcLinkingInfo. See b/111289526.")
   public boolean enableCcDynamicLibrariesForRuntime;
 
+  @Option(
+      name = "experimental_linkopts_in_user_link_flags",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.LOADING_AND_ANALYSIS},
+      metadataTags = {OptionMetadataTag.EXPERIMENTAL},
+      help =
+          "If true, flags coming from --linkopt Bazel option will appear in user_link_flags "
+              + "crosstool variable.")
+  public boolean enableLinkoptsInUserLinkFlags;
+
   @Override
   public FragmentOptions getHost() {
     CppOptions host = (CppOptions) getDefault();
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 b57dc237be..a26b8b76cc 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
@@ -329,7 +329,9 @@ public enum LinkBuildVariables {
       result.addAll(ccToolchainProvider.getTestOnlyLinkOptions());
     }
 
-    result.addAll(ccToolchainProvider.getLinkOptions());
+    if (!cppConfiguration.enableLinkoptsInUserLinkFlags()) {
+      result.addAll(ccToolchainProvider.getLinkOptions());
+    }
 
     // -pie is not compatible with shared and should be
     // removed when the latter is part of the link command. Should we need to further
-- 
cgit v1.2.3