Don't set `LD_LIBRARY_PATH` by default on Linux (#28354)

`LD_LIBRARY_PATH` can break system executables (e.g., when an enviornment is loaded) and isn't necessary thanks to `RPATH`s.  Packages that require `LD_LIBRARY_PATH` can set this in `setup_run_environment`.

- [x] Prefix inspections no longer set `LD_LIBRARY_PATH` by default
- [x] Document changes and workarounds for people who want `LD_LIBRARY_PATH`

7 files changed, 32 insertions, 31 deletions

diff --git a/etc/spack/defaults/cray/modules.yaml b/etc/spack/defaults/cray/modules.yaml
index a86a4794f1..a80f87b16a 100644
--- a/etc/spack/defaults/cray/modules.yaml
+++ b/etc/spack/defaults/cray/modules.yaml
@@ -13,9 +13,4 @@
 # Per-user settings (overrides default and site settings):
 #   ~/.spack/modules.yaml
 # -------------------------------------------------------------------------
-modules:
-  prefix_inspections:
-    lib:
-      - LD_LIBRARY_PATH
-    lib64:
-      - LD_LIBRARY_PATH
+modules: {}
diff --git a/etc/spack/defaults/linux/modules.yaml b/etc/spack/defaults/linux/modules.yaml
index a86a4794f1..a80f87b16a 100644
--- a/etc/spack/defaults/linux/modules.yaml
+++ b/etc/spack/defaults/linux/modules.yaml
@@ -13,9 +13,4 @@
 # Per-user settings (overrides default and site settings):
 #   ~/.spack/modules.yaml
 # -------------------------------------------------------------------------
-modules:
-  prefix_inspections:
-    lib:
-      - LD_LIBRARY_PATH
-    lib64:
-      - LD_LIBRARY_PATH
+modules: {}
diff --git a/lib/spack/docs/basic_usage.rst b/lib/spack/docs/basic_usage.rst
index cc1e34912f..53bd3858e0 100644
--- a/lib/spack/docs/basic_usage.rst
+++ b/lib/spack/docs/basic_usage.rst
@@ -896,8 +896,8 @@ your path:
    $ which mpicc
    ~/spack/opt/linux-debian7-x86_64/gcc@4.4.7/mpich@3.0.4/bin/mpicc
 
-These commands will add appropriate directories to your ``PATH``,
-``MANPATH``, ``CPATH``, and ``LD_LIBRARY_PATH`` according to the
+These commands will add appropriate directories to your ``PATH``
+and ``MANPATH`` according to the
 :ref:`prefix inspections <customize-env-modifications>` defined in your
 modules configuration.
 When you no longer want to use a package, you can type unload or
diff --git a/lib/spack/docs/environments.rst b/lib/spack/docs/environments.rst
index 4c3f6d1971..036a33b19f 100644
--- a/lib/spack/docs/environments.rst
+++ b/lib/spack/docs/environments.rst
@@ -972,9 +972,6 @@ Variable            Paths
 PATH                bin
 MANPATH             man, share/man
 ACLOCAL_PATH        share/aclocal
-LD_LIBRARY_PATH     lib, lib64
-LIBRARY_PATH        lib, lib64
-CPATH               include
 PKG_CONFIG_PATH     lib/pkgconfig, lib64/pkgconfig, share/pkgconfig
 CMAKE_PREFIX_PATH   .
 =================== =========
diff --git a/lib/spack/docs/module_file_support.rst b/lib/spack/docs/module_file_support.rst
index 5913434e04..3f369a845c 100644
--- a/lib/spack/docs/module_file_support.rst
+++ b/lib/spack/docs/module_file_support.rst
@@ -113,6 +113,8 @@ from language interpreters into their extensions. The latter two instead permit
 fine tune the filesystem layout, content and creation of module files to meet
 site specific conventions.
 
+.. _overide-api-calls-in-package-py:
+
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 Override API calls in ``package.py``
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -134,7 +136,7 @@ The second method:
        pass
 
 can instead inject run-time environment modifications in the module files of packages
-that depend on it. In both cases you need to fill ``run_env`` with the desired
+that depend on it. In both cases you need to fill ``env`` with the desired
 list of environment modifications.
 
 .. admonition:: The ``r`` package and callback APIs
@@ -518,18 +520,33 @@ inspections and customize them per-module-set.
     prefix_inspections:
       bin:
         - PATH
-      lib:
-        - LIBRARY_PATH
+      man:
+        - MANPATH
       '':
         - CMAKE_PREFIX_PATH
 
 Prefix inspections are only applied if the relative path inside the
 installation prefix exists. In this case, for a Spack package ``foo``
 installed to ``/spack/prefix/foo``, if ``foo`` installs executables to
-``bin`` but no libraries in ``lib``, the generated module file for
+``bin`` but no manpages in ``man``, the generated module file for
 ``foo`` would update ``PATH`` to contain ``/spack/prefix/foo/bin`` and
 ``CMAKE_PREFIX_PATH`` to contain ``/spack/prefix/foo``, but would not
-update ``LIBRARY_PATH``.
+update ``MANPATH``.
+
+The default list of environment variables in this config section
+inludes ``PATH``, ``MANPATH``, ``ACLOCAL_PATH``, ``PKG_CONFIG_PATH``
+and ``CMAKE_PREFIX_PATH``, as well as ``DYLD_FALLBACK_LIBRARY_PATH``
+on macOS. On Linux however, the corresponding ``LD_LIBRARY_PATH``
+variable is *not* set, because it affects the behavior of
+system executables too.
+
+.. note::
+
+   In general, the ``LD_LIBRARY_PATH`` variable is not required
+   when using packages built with Spack, thanks to the use of RPATH.
+   Some packages may still need the variable, which is best handled
+   on a per-package basis instead of globally, as explained in
+   :ref:`overide-api-calls-in-package-py`.
 
 There is a special case for prefix inspections relative to environment
 views. If all of the following conditions hold for a module set
diff --git a/lib/spack/spack/user_environment.py b/lib/spack/spack/user_environment.py
index 721150aae9..da5fd12e61 100644
--- a/lib/spack/spack/user_environment.py
+++ b/lib/spack/spack/user_environment.py
@@ -32,12 +32,9 @@ def prefix_inspections(platform):
 
     inspections = {
         "bin": ["PATH"],
-        "lib": ["LD_LIBRARY_PATH", "LIBRARY_PATH"],
-        "lib64": ["LD_LIBRARY_PATH", "LIBRARY_PATH"],
         "man": ["MANPATH"],
         "share/man": ["MANPATH"],
         "share/aclocal": ["ACLOCAL_PATH"],
-        "include": ["CPATH"],
         "lib/pkgconfig": ["PKG_CONFIG_PATH"],
         "lib64/pkgconfig": ["PKG_CONFIG_PATH"],
         "share/pkgconfig": ["PKG_CONFIG_PATH"],
@@ -45,8 +42,8 @@ def prefix_inspections(platform):
     }
 
     if platform == "darwin":
-        for subdir in ("lib", "lib64"):
-            inspections[subdir].append("DYLD_FALLBACK_LIBRARY_PATH")
+        inspections["lib"] = ["DYLD_FALLBACK_LIBRARY_PATH"]
+        inspections["lib64"] = ["DYLD_FALLBACK_LIBRARY_PATH"]
 
     return inspections
 
diff --git a/share/spack/qa/setup-env-test.fish b/share/spack/qa/setup-env-test.fish
index ffe3d71036..28deea7e41 100755
--- a/share/spack/qa/setup-env-test.fish
+++ b/share/spack/qa/setup-env-test.fish
@@ -331,17 +331,17 @@ spt_contains "usage: spack module " spack -m module
 
 title 'Testing `spack load`'
 set _b_loc (spack -m location -i b)
-set _b_ld $_b_loc"/lib"
+set _b_bin $_b_loc"/bin"
 set _a_loc (spack -m location -i a)
-set _a_ld $_a_loc"/lib"
+set _a_bin $_a_loc"/bin"
 
-spt_contains "set -gx LD_LIBRARY_PATH $_b_ld" spack -m load --only package --fish b
+spt_contains "set -gx PATH $_b_bin" spack -m load --only package --fish b
 spt_succeeds spack -m load b
 set LIST_CONTENT (spack -m load b; spack load --list)
 spt_contains "b@" echo $LIST_CONTENT
 spt_does_not_contain "a@" echo $LIST_CONTENT
 # test a variable MacOS clears and one it doesn't for recursive loads
-spt_contains "set -gx LD_LIBRARY_PATH $_a_ld:$_b_ld" spack -m load --fish a
+spt_contains "set -gx PATH $_a_bin:$_b_bin" spack -m load --fish a
 spt_succeeds spack -m load --only dependencies a
 spt_succeeds spack -m load --only package a
 spt_fails spack -m load d