From 0f59a83e3bf1a67a7901f9f87c8b9b4affa620dd Mon Sep 17 00:00:00 2001 From: Lin Jian Date: Sun, 27 Sep 2026 18:10:00 +0800 Subject: [PATCH 1/3] Revert "emacs/wrapper.nix: expose extra package binaries via PATH" This reverts commit 7bd6dfde2feeb6d8472eb36f87c549b791665e38. The idea of using PATH to provide binaries added via withPackages is good, but I think this specific implementation can be improved: - When PATH is used, there is no need to modify exec-path any more because exec-path is initialized from PATH. - Setting PATH in wrapper.sh, together with other environment variables, is more maintainable. --- pkgs/applications/editors/emacs/build-support/wrapper.nix | 7 ------- 1 file changed, 7 deletions(-) diff --git a/pkgs/applications/editors/emacs/build-support/wrapper.nix b/pkgs/applications/editors/emacs/build-support/wrapper.nix index 051e5923d6b0..782fa7681974 100644 --- a/pkgs/applications/editors/emacs/build-support/wrapper.nix +++ b/pkgs/applications/editors/emacs/build-support/wrapper.nix @@ -174,13 +174,6 @@ runCommand (lib.appendToName "with-packages" emacs).name ;; "$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/") ''} From 78805715a6c14d278cf7565ab910aa6e72c24347 Mon Sep 17 00:00:00 2001 From: Lin Jian Date: Sun, 27 Sep 2026 23:44:20 +0800 Subject: [PATCH 2/3] emacs: factor out a Utils section in withPackages test --- .../wrapper-test/with-packages.el | 46 ++++++++++--------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/pkgs/applications/editors/emacs/build-support/wrapper-test/with-packages.el b/pkgs/applications/editors/emacs/build-support/wrapper-test/with-packages.el index 50715f7e5d27..9f0ed7920c2a 100644 --- a/pkgs/applications/editors/emacs/build-support/wrapper-test/with-packages.el +++ b/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,29 @@ 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)) + +;;;; 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,9 +68,6 @@ 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))) From 88accc0c92639355ce8a1c62cfaac0e07bc09de9 Mon Sep 17 00:00:00 2001 From: Lin Jian Date: Sun, 27 Sep 2026 18:55:45 +0800 Subject: [PATCH 3/3] emacs: use PATH instead of exec-path to expose wrapped binaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (info "(elisp) Subprocess Creation") says: > Generally, you should not modify ‘exec-path’ directly. Instead, > ensure that your ‘PATH’ environment variable is set appropriately > before starting Emacs. Trying to modify ‘exec-path’ independently > of ‘PATH’ can lead to confusing results. One example of "confusing results" can be found in envrc[1]. This patch follows that recommendation of Emacs lisp manual. In addition, we added tests for PATH and exec-path. [1]: https://github.com/purcell/envrc/issues/9 --- .../wrapper-test/with-packages.el | 27 +++++++++++++++++-- .../editors/emacs/build-support/wrapper.nix | 5 ++-- .../editors/emacs/build-support/wrapper.sh | 5 ++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/pkgs/applications/editors/emacs/build-support/wrapper-test/with-packages.el b/pkgs/applications/editors/emacs/build-support/wrapper-test/with-packages.el index 9f0ed7920c2a..30735966656d 100644 --- a/pkgs/applications/editors/emacs/build-support/wrapper-test/with-packages.el +++ b/pkgs/applications/editors/emacs/build-support/wrapper-test/with-packages.el @@ -41,6 +41,13 @@ which is generated by nix when building this elisp package." (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 () @@ -71,8 +78,24 @@ which is generated by nix when building this elisp package." (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)) diff --git a/pkgs/applications/editors/emacs/build-support/wrapper.nix b/pkgs/applications/editors/emacs/build-support/wrapper.nix index 782fa7681974..1f9fbb240c5a 100644 --- a/pkgs/applications/editors/emacs/build-support/wrapper.nix +++ b/pkgs/applications/editors/emacs/build-support/wrapper.nix @@ -171,9 +171,6 @@ runCommand (lib.appendToName "with-packages" emacs).name cat >"$siteStart" <