From 0127013b0f2a27ee8eec886283948648ef66fbf0 Mon Sep 17 00:00:00 2001
From: Alyssa Ross <hi@alyssa.is>
Date: Wed, 9 Dec 2020 15:53:10 +0000
Subject: [PATCH 1/2] emacsWithPackages: mutate EMACSLOADPATH correctly

An empty entry in EMACSLOADPATH gets filled with the default value.
This is presumably why the wrapper inserted a colon after the entry it
added for the dependencies.  But this naive approach wasn't always
correct.

For example, if the user ran emacs with EMACSLOADPATH=foo, the wrapper
would insert the default value (by adding the trailing `:') even
though the user was trying to expressly opt out of it.

To do this correctly, here I've replaced makeWrapper with a bespoke
script that will actually parse the EMACSLOADPATH provided in the
environment (if given), and insert the wrapper's load path just before
the default value.  If EMACSLOADPATH is given but contains no default
value, we respect that and don't add the wrapped dependencies at all.
If no EMACSLOADPATH is given, we insert the wrapped dependencies
before the default value, just like before.  In this way, the wrapped
Emacs should now behave as if the wrapped dependencies were part of
Emacs's default load-path value.
---
 pkgs/build-support/emacs/wrapper.nix | 16 ++++++++++++----
 pkgs/build-support/emacs/wrapper.sh  | 26 ++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 4 deletions(-)
 create mode 100644 pkgs/build-support/emacs/wrapper.sh

diff --git a/pkgs/build-support/emacs/wrapper.nix b/pkgs/build-support/emacs/wrapper.nix
index 1f2fbd8068e7..a3ab30afc632 100644
--- a/pkgs/build-support/emacs/wrapper.nix
+++ b/pkgs/build-support/emacs/wrapper.nix
@@ -155,8 +155,12 @@ runCommand
     for prog in $emacs/bin/*; do # */
       local progname=$(basename "$prog")
       rm -f "$out/bin/$progname"
-      makeWrapper "$prog" "$out/bin/$progname" \
-        --suffix EMACSLOADPATH ":" "$deps/share/emacs/site-lisp:"
+
+      substitute ${./wrapper.sh} $out/bin/$progname \
+        --subst-var-by bash ${emacs.stdenv.shell} \
+        --subst-var-by wrapperSiteLisp "$deps/share/emacs/site-lisp" \
+        --subst-var prog
+      chmod +x $out/bin/$progname
     done
 
     # Wrap MacOS app
@@ -168,8 +172,12 @@ runCommand
             $emacs/Applications/Emacs.app/Contents/PkgInfo \
             $emacs/Applications/Emacs.app/Contents/Resources \
             $out/Applications/Emacs.app/Contents
-      makeWrapper $emacs/Applications/Emacs.app/Contents/MacOS/Emacs $out/Applications/Emacs.app/Contents/MacOS/Emacs \
-        --suffix EMACSLOADPATH ":" "$deps/share/emacs/site-lisp:"
+
+      substitute ${./wrapper.sh} $out/Applications/Emacs.app/Contents/MacOS/Emacs \
+        --subst-var-by bash ${emacs.stdenv.shell} \
+        --subst-var-by wrapperSiteLisp "$emacs/Applications/Emacs.app/Contents/MacOS/Emacs" \
+        --subst-var prog
+      chmod +x $out/bin/$progname
     fi
 
     mkdir -p $out/share
diff --git a/pkgs/build-support/emacs/wrapper.sh b/pkgs/build-support/emacs/wrapper.sh
new file mode 100644
index 000000000000..85657ac5619a
--- /dev/null
+++ b/pkgs/build-support/emacs/wrapper.sh
@@ -0,0 +1,26 @@
+#!@bash@
+
+IFS=:
+
+newLoadPath=()
+added=
+
+if [[ -n $EMACSLOADPATH ]]
+then
+    while read -rd: entry
+    do
+        if [[ -z $entry && -z $added ]]
+        then
+            newLoadPath+=(@wrapperSiteLisp@)
+            added=1
+        fi
+        newLoadPath+=("$entry")
+    done <<< "$EMACSLOADPATH:"
+else
+    newLoadPath+=(@wrapperSiteLisp@)
+    newLoadPath+=("")
+fi
+
+export EMACSLOADPATH="${newLoadPath[*]}"
+
+exec @prog@ "$@"

From 23d4bfb6661ca57a9e331a2cf4184232d38ac38b Mon Sep 17 00:00:00 2001
From: Alyssa Ross <hi@alyssa.is>
Date: Mon, 7 Dec 2020 18:53:54 +0000
Subject: [PATCH 2/2] emacsWithPackages: don't tell sub-Emacs about pkgs

If I'm running an Emacs executable from emacsWithPackages as my main
programming environment, and I'm hacking on Emacs, or the Emacs
packaging in Nixpkgs, or whatever, I don't want the Emacs packages
from the wrapper to show up in the load path of that child Emacs.  It
results in differing behaviour depending on whether the child Emacs is
run from Emacs or from, for example, an external terminal emulator,
which is very surprising.

To avoid this, pass another environment variable containing the
wrapper site-lisp path, and use that value to remove the corresponding
entry in EMACSLOADPATH, so it won't be propagated to child Emacsen.
---
 pkgs/applications/editors/emacs/site-start.el | 11 +++++++++++
 pkgs/build-support/emacs/wrapper.sh           |  1 +
 2 files changed, 12 insertions(+)

diff --git a/pkgs/applications/editors/emacs/site-start.el b/pkgs/applications/editors/emacs/site-start.el
index 2f02d6d1a86d..86cad1132f64 100644
--- a/pkgs/applications/editors/emacs/site-start.el
+++ b/pkgs/applications/editors/emacs/site-start.el
@@ -22,6 +22,17 @@ least specific (the system profile)"
                              (nix--profile-paths)))))
   (setq load-path (append paths load-path)))
 
+;;; Remove wrapper site-lisp from EMACSLOADPATH so it's not propagated
+;;; to any other Emacsen that might be started as subprocesses.
+(let ((wrapper-site-lisp (getenv "emacsWithPackages_siteLisp"))
+      (env-load-path (getenv "EMACSLOADPATH")))
+  (when wrapper-site-lisp
+    (setenv "emacsWithPackages_siteLisp" nil))
+  (when (and wrapper-site-lisp env-load-path)
+    (let* ((env-list (split-string env-load-path ":"))
+           (new-env-list (delete wrapper-site-lisp env-list)))
+      (setenv "EMACSLOADPATH" (when new-env-list
+                                (mapconcat 'identity new-env-list ":"))))))
 
 ;;; Make `woman' find the man pages
 (defvar woman-manpath)
diff --git a/pkgs/build-support/emacs/wrapper.sh b/pkgs/build-support/emacs/wrapper.sh
index 85657ac5619a..96c9a8a60ea4 100644
--- a/pkgs/build-support/emacs/wrapper.sh
+++ b/pkgs/build-support/emacs/wrapper.sh
@@ -22,5 +22,6 @@ else
 fi
 
 export EMACSLOADPATH="${newLoadPath[*]}"
+export emacsWithPackages_siteLisp=@wrapperSiteLisp@
 
 exec @prog@ "$@"