Change handling of LD_LIBRARY_PATH in link actions: instead of passing the directories using a linker option, pass it in the environment instead.

Fixes #2099.

--
Change-Id: I9bff1888fbb211bc0853a68065b3098c4908c3d9
Reviewed-on: https://cr.bazel.build/7530
MOS_MIGRATED_REVID=140133329

3 files changed, 22 insertions, 9 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 83bcf05325..a22b2e015f 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
@@ -1455,10 +1455,8 @@ public class CcToolchainFeatures implements Serializable {
       return commandLine;
     }
 
-    /**
-     * @return the environment variables (key/value pairs) for the given {@code action}.
-     */
-    Map<String, String> getEnvironmentVariables(String action, Variables variables) {
+    /** @return the environment variables (key/value pairs) for the given {@code action}. */
+    ImmutableMap<String, String> getEnvironmentVariables(String action, Variables variables) {
       ImmutableMap.Builder<String, String> envBuilder = ImmutableMap.builder();
       for (Feature feature : enabledFeatures) {
         feature.expandEnvironment(action, variables, envBuilder);
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 04dc86895e..4c868c74d8 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
@@ -53,6 +53,7 @@ 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.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import javax.annotation.Nullable;
@@ -100,7 +101,9 @@ public final class CppLinkAction extends AbstractAction
   private final LibraryToLink outputLibrary;
   private final Artifact linkOutput;
   private final LibraryToLink interfaceOutputLibrary;
-  private final Map<String, String> toolchainEnv;
+  private final ImmutableSet<String> clientEnvironmentVariables;
+  private final ImmutableMap<String, String> actionEnv;
+  private final ImmutableMap<String, String> toolchainEnv;
   private final ImmutableSet<String> executionRequirements;
 
   private final LinkCommandLine linkCommandLine;
@@ -150,7 +153,9 @@ public final class CppLinkAction extends AbstractAction
       boolean isLTOIndexing,
       Iterable<LTOBackendArtifacts> allLTOBackendArtifacts,
       LinkCommandLine linkCommandLine,
-      Map<String, String> toolchainEnv,
+      ImmutableSet<String> clientEnvironmentVariables,
+      ImmutableMap<String, String> actionEnv,
+      ImmutableMap<String, String> toolchainEnv,
       ImmutableSet<String> executionRequirements) {
     super(owner, inputs, outputs);
     if (mnemonic == null) {
@@ -167,6 +172,8 @@ public final class CppLinkAction extends AbstractAction
     this.isLTOIndexing = isLTOIndexing;
     this.allLTOBackendArtifacts = allLTOBackendArtifacts;
     this.linkCommandLine = linkCommandLine;
+    this.clientEnvironmentVariables = clientEnvironmentVariables;
+    this.actionEnv = actionEnv;
     this.toolchainEnv = toolchainEnv;
     this.executionRequirements = executionRequirements;
   }
@@ -185,9 +192,15 @@ public final class CppLinkAction extends AbstractAction
   }
 
   @Override
+  public Iterable<String> getClientEnvironmentVariables() {
+    return clientEnvironmentVariables;
+  }
+
+  @Override
   public ImmutableMap<String, String> getEnvironment() {
-    ImmutableMap.Builder<String, String> result = ImmutableMap.<String, String>builder();
+    LinkedHashMap<String, String> result = new LinkedHashMap<>();
 
+    result.putAll(actionEnv);
     result.putAll(toolchainEnv);
 
     if (OS.getCurrent() == OS.WINDOWS) {
@@ -207,7 +220,7 @@ public final class CppLinkAction extends AbstractAction
               .getParentDirectory()
               .getPathString());
     }
-    return result.build();
+    return ImmutableMap.copyOf(result);
   }
 
   /**
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 a368cbbe36..ed09f52223 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
@@ -727,7 +727,7 @@ public class CppLinkActionBuilder {
       analysisEnvironment.registerAction(parameterFileWriteAction);
     }
 
-    Map<String, String> toolchainEnv =
+    ImmutableMap<String, String> toolchainEnv =
         featureConfiguration.getEnvironmentVariables(getActionName(), buildVariables);
 
     // If the crosstool uses action_configs to configure cc compilation, collect execution info
@@ -752,6 +752,8 @@ public class CppLinkActionBuilder {
         isLTOIndexing,
         allLTOArtifacts,
         linkCommandLine,
+        configuration.getVariableShellEnvironment(),
+        configuration.getLocalShellEnvironment(),
         toolchainEnv,
         executionRequirements.build());
   }