emacs: use PATH instead of exec-path to expose wrapped binaries (#567551)

pkgs/applications/editors/emacs/build-support/wrapper-test/with-packages.el

@@ -9,25 +9,7 @@
 
 ;; Try to make tests cause no side-effects.
 
-;;;; Tests that can be run in a batch Emacs
-
-(ert-deftest with-packages-requested-packages-are-available ()
-  (should (package-installed-p 'dash))
-  (should (package-installed-p 'flx-ido)))
-
-(ert-deftest with-packages-deps-of-requested-packages-are-available ()
-  "Test https://github.com/NixOS/nixpkgs/issues/388829."
-  (should (package-installed-p 'flx))
-  (should (package-installed-p 'flx-ido)))
-
-(ert-deftest with-packages-info-manual-of-requested-packages-is-available ()
-  "Test https://debbugs.gnu.org/cgi/bugreport.cgi?bug=81105."
-  ;; `package-activate-all' makes package info manuals available.
-  ;; It is called at startup normally, but not in batch mode.
-  ;; We call it if needed to emulate the "normal" case.
-  (unless package--activated
-    (package-activate-all))
-  (should (Info-find-file "dash" t)))
+;;;; Utils
 
 (defun with-packages--nix-store-dir ()
   "Return nix store dir.
@@ -56,6 +38,36 @@ which is generated by nix when building this elisp package."
   "Locate the natively-compiled LIBRARY file."
   (locate-eln-file (comp-el-to-eln-rel-filename (find-library-name library))))
 
+(defun with-packages-unwrapped-site-start-is-loaded ()
+  (fboundp 'nix--profile-paths))
+
+(defun with-packages-eval-in-sub-emacs (form)
+  "Evaluate FORM in a subprocess Emacs and return its result."
+  (pcase-exhaustive
+      (process-lines "emacs" "--batch"
+                     "--eval" (format "(prin1 %S)" form))
+    (`(,result) (read result))))
+
+;;;; Tests that can be run in a batch Emacs
+
+(ert-deftest with-packages-requested-packages-are-available ()
+  (should (package-installed-p 'dash))
+  (should (package-installed-p 'flx-ido)))
+
+(ert-deftest with-packages-deps-of-requested-packages-are-available ()
+  "Test https://github.com/NixOS/nixpkgs/issues/388829."
+  (should (package-installed-p 'flx))
+  (should (package-installed-p 'flx-ido)))
+
+(ert-deftest with-packages-info-manual-of-requested-packages-is-available ()
+  "Test https://debbugs.gnu.org/cgi/bugreport.cgi?bug=81105."
+  ;; `package-activate-all' makes package info manuals available.
+  ;; It is called at startup normally, but not in batch mode.
+  ;; We call it if needed to emulate the "normal" case.
+  (unless package--activated
+    (package-activate-all))
+  (should (Info-find-file "dash" t)))
+
 (ert-deftest with-packages-aot-native-comp-eln-files-are-available ()
   (skip-unless (native-comp-available-p))
   (ert-info ("search eln files of Emacs proper")
@@ -63,14 +75,27 @@ which is generated by nix when building this elisp package."
   (ert-info ("search eln files of requested packages")
     (should (with-packages--nix-store-file-p (with-packages--locate-eln-file "dash")))))
 
-(defun with-packages-unwrapped-site-start-is-loaded ()
-  (fboundp 'nix--profile-paths))
-
 (ert-deftest with-packages-unwrapped-site-start-is-loaded ()
   (should (with-packages-unwrapped-site-start-is-loaded)))
 
-(ert-deftest with-packages-bin-dirs-of-requested-packages-are-added-to-exec-path ()
-  (should (executable-find "cowsay")))
+(ert-deftest with-packages-binaries-of-requested-packages-are-available ()
+  (ert-info ("find binary via PATH")
+    (should (equal (call-process-shell-command "cowsay") 0)))
+  (ert-info ("find binary via exec-path")
+    (should (executable-find "cowsay")))
+  (ert-info ("each item is unique")
+    (cl-flet ((item-should-be-unique (items)
+                (should (equal items
+                               (cl-remove-duplicates items :test #'string=)))))
+      (ert-info ("PATH of this Emacs")
+        (item-should-be-unique (parse-colon-path (getenv "PATH"))))
+      (ert-info ("PATH of sub-Emacs")
+        (item-should-be-unique
+         (parse-colon-path (with-packages-eval-in-sub-emacs '(getenv "PATH")))))
+      (ert-info ("exec-path of this Emacs")
+        (item-should-be-unique exec-path))
+      (ert-info ("exec-path of sub-Emacs")
+        (item-should-be-unique (with-packages-eval-in-sub-emacs 'exec-path))))))
 
 (ert-deftest with-packages-tree-sitter-dir-is-added-to-treesit-extra-load-path ()
   (skip-unless (treesit-available-p))

pkgs/applications/editors/emacs/build-support/wrapper.nix

@@ -171,16 +171,6 @@ runCommand (lib.appendToName "with-packages" emacs).name
           cat >"$siteStart" <<EOF
           ;;; -*- lexical-binding: t -*-
           (load "$emacs/share/emacs/site-lisp/site-start" nil t)
-          ;; "$out/share/emacs/site-lisp" is added to load-path in wrapper.sh
-          ;; "$out/share/emacs/native-lisp" is added to native-comp-eln-load-path in wrapper.sh
-          (add-to-list 'exec-path "$out/bin")
-          ;; Also expose extra package binaries via PATH so that subprocesses
-          ;; which rebuild their environment from PATH (e.g. direnv/envrc) can
-          ;; still find them. See https://github.com/purcell/envrc/issues/9
-          (let ((deps-bin "$out/bin")
-                (current-path (or (getenv "PATH") "")))
-            (unless (member deps-bin (split-string current-path path-separator))
-              (setenv "PATH" (concat deps-bin path-separator current-path))))
           ${lib.optionalString withTreeSitter ''
             (add-to-list 'treesit-extra-load-path "$out/lib/")
           ''}
@@ -220,6 +210,7 @@ runCommand (lib.appendToName "with-packages" emacs).name
         --subst-var-by bash ${emacs.stdenv.shell} \
         --subst-var-by wrapperSiteLisp "$deps/share/emacs/site-lisp" \
         --subst-var-by wrapperSiteLispNative "$deps/share/emacs/native-lisp" \
+        --subst-var-by wrapperBinDir "$deps/bin" \
         --subst-var-by wrapperInvocationDirectory "$out/bin/" \
         --subst-var-by wrapperInvocationName "$progname" \
         --subst-var prog
@@ -246,6 +237,7 @@ runCommand (lib.appendToName "with-packages" emacs).name
         --subst-var-by bash ${emacs.stdenv.shell} \
         --subst-var-by wrapperSiteLisp "$deps/share/emacs/site-lisp" \
         --subst-var-by wrapperSiteLispNative "$deps/share/emacs/native-lisp" \
+        --subst-var-by wrapperBinDir "$deps/bin" \
         --subst-var-by wrapperInvocationDirectory "$out/Applications/Emacs.app/Contents/MacOS/" \
         --subst-var-by wrapperInvocationName "Emacs" \
         --subst-var-by prog "$emacs/Applications/Emacs.app/Contents/MacOS/Emacs"

pkgs/applications/editors/emacs/build-support/wrapper.sh

@@ -53,4 +53,9 @@ export emacsWithPackages_siteLispNative=@wrapperSiteLispNative@
 export emacsWithPackages_invocationDirectory=@wrapperInvocationDirectory@
 export emacsWithPackages_invocationName=@wrapperInvocationName@
 
+if [[ ${PATH-} != *"@wrapperBinDir@":* ]]
+then
+    export PATH="@wrapperBinDir@${PATH:+:$PATH}"
+fi
+
 exec @prog@ "$@"