diff mbox series

[bug#68180,1/4] gnu: emacs: Add awk, find, sed and sh to PATH wrapper.

Message ID e91dcea9f480d8e855d87b47fae844e5cdb831d7.1704041948.git.maxim.cournoyer@gmail.com
State New
Headers show
Series Add emacs-pde | expand

Commit Message

Maxim Cournoyer Dec. 31, 2023, 4:59 p.m. UTC
Before this change, using Emacs in a pure environment, e.g. 'guix shell --pure
emacs', would cause problems such as:

  jka-compr-insert-file-contents: Uncompression program ‘sh’ not found

And other problems were found requiring the other tools.  While the above
could be patched in place for 'sh', it seems more robust and universally
useful to have the commands appear on PATH, should other Elisp modules want to
call to these directly as well.

* gnu/packages/emacs.scm (emacs-minimal) [arguments] <phases>: Adjust the
wrap-emacs-paths phase to wrap additional inputs.
[inputs]: Add findutils, gawk and sed.
(%emacs-modules): Add (srfi srfi-26).

Change-Id: Ifb4fe2fc12ddc9eae387adb3da3f7821fab78e65
---

 gnu/packages/emacs.scm | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

Comments

Liliana Marie Prikler Dec. 31, 2023, 7:13 p.m. UTC | #1
Am Sonntag, dem 31.12.2023 um 11:59 -0500 schrieb Maxim Cournoyer:
> Before this change, using Emacs in a pure environment, e.g. 'guix
> shell --pure emacs', would cause problems such as:
> 
>   jka-compr-insert-file-contents: Uncompression program ‘sh’ not
> found
> 
> And other problems were found requiring the other tools.  While the
> above could be patched in place for 'sh', it seems more robust and
> universally useful to have the commands appear on PATH, should other
> Elisp modules want to call to these directly as well.
> 
> * gnu/packages/emacs.scm (emacs-minimal) [arguments] <phases>: Adjust
> the
> wrap-emacs-paths phase to wrap additional inputs.
> [inputs]: Add findutils, gawk and sed.
> (%emacs-modules): Add (srfi srfi-26).
> 
> Change-Id: Ifb4fe2fc12ddc9eae387adb3da3f7821fab78e65
> ---
We already have a phase to patch in the real path of /bin/sh where it's
used.  This appears to be an odd case that's missed.

Cheers
Maxim Cournoyer Dec. 31, 2023, 9:10 p.m. UTC | #2
Hi Liliana,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

> Am Sonntag, dem 31.12.2023 um 11:59 -0500 schrieb Maxim Cournoyer:
>> Before this change, using Emacs in a pure environment, e.g. 'guix
>> shell --pure emacs', would cause problems such as:
>> 
>>   jka-compr-insert-file-contents: Uncompression program ‘sh’ not
>> found
>> 
>> And other problems were found requiring the other tools.  While the
>> above could be patched in place for 'sh', it seems more robust and
>> universally useful to have the commands appear on PATH, should other
>> Elisp modules want to call to these directly as well.
>> 
>> * gnu/packages/emacs.scm (emacs-minimal) [arguments] <phases>: Adjust
>> the
>> wrap-emacs-paths phase to wrap additional inputs.
>> [inputs]: Add findutils, gawk and sed.
>> (%emacs-modules): Add (srfi srfi-26).
>> 
>> Change-Id: Ifb4fe2fc12ddc9eae387adb3da3f7821fab78e65
>> ---
> We already have a phase to patch in the real path of /bin/sh where it's
> used.  This appears to be an odd case that's missed.

I appreciate exactness, but it seems fragile to rely on nobody adding
new references or someone catching them as new Emacs modules get added
or changed :-).

My reasoning was that since Emacs already depends on bash, why not
ensure it'll always be found on PATH, by wrapping instead of
substituting.

Does it make sense?
Andrew Tropin Jan. 1, 2024, 7:33 a.m. UTC | #3
On 2023-12-31 16:10, Maxim Cournoyer wrote:

> Hi Liliana,
>
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>
>> Am Sonntag, dem 31.12.2023 um 11:59 -0500 schrieb Maxim Cournoyer:
>>> Before this change, using Emacs in a pure environment, e.g. 'guix
>>> shell --pure emacs', would cause problems such as:
>>> 
>>>   jka-compr-insert-file-contents: Uncompression program ‘sh’ not
>>> found
>>> 
>>> And other problems were found requiring the other tools.  While the
>>> above could be patched in place for 'sh', it seems more robust and
>>> universally useful to have the commands appear on PATH, should other
>>> Elisp modules want to call to these directly as well.
>>> 
>>> * gnu/packages/emacs.scm (emacs-minimal) [arguments] <phases>: Adjust
>>> the
>>> wrap-emacs-paths phase to wrap additional inputs.
>>> [inputs]: Add findutils, gawk and sed.
>>> (%emacs-modules): Add (srfi srfi-26).
>>> 
>>> Change-Id: Ifb4fe2fc12ddc9eae387adb3da3f7821fab78e65
>>> ---
>> We already have a phase to patch in the real path of /bin/sh where it's
>> used.  This appears to be an odd case that's missed.
>
> I appreciate exactness, but it seems fragile to rely on nobody adding
> new references or someone catching them as new Emacs modules get added
> or changed :-).
>
> My reasoning was that since Emacs already depends on bash, why not
> ensure it'll always be found on PATH, by wrapping instead of
> substituting.
>
> Does it make sense?

Yep, make sense to me. I also find cases from time to time, when some
binary or another isn't found by some elisp code.

However, providing those binaries via PATH can make some code or
programs to work, when executed from inside Emacs and not to work in the
environment outside, which can be really confusing in some cases.

A simple example, imaging we have a script: 1.sh, which contains:
sh --version

This one will work:
guix shell emacs-with-bash --pure -- emacs --eval '(shell-command "./1.sh")'

This one will not:
guix shell emacs-with-bash --pure -- ./1.sh

That said, the idea of patching all the pathes to binaries seems better
to me.
Maxim Cournoyer Jan. 2, 2024, 2:07 a.m. UTC | #4
Hi Andrew,

Andrew Tropin <andrew@trop.in> writes:

[...]

>>> We already have a phase to patch in the real path of /bin/sh where it's
>>> used.  This appears to be an odd case that's missed.
>>
>> I appreciate exactness, but it seems fragile to rely on nobody adding
>> new references or someone catching them as new Emacs modules get added
>> or changed :-).
>>
>> My reasoning was that since Emacs already depends on bash, why not
>> ensure it'll always be found on PATH, by wrapping instead of
>> substituting.
>>
>> Does it make sense?
>
> Yep, make sense to me. I also find cases from time to time, when some
> binary or another isn't found by some elisp code.
>
> However, providing those binaries via PATH can make some code or
> programs to work, when executed from inside Emacs and not to work in the
> environment outside, which can be really confusing in some cases.
>
> A simple example, imaging we have a script: 1.sh, which contains:
> sh --version
>
> This one will work:
> guix shell emacs-with-bash --pure -- emacs --eval '(shell-command "./1.sh")'
>
> This one will not:
> guix shell emacs-with-bash --pure -- ./1.sh
>
> That said, the idea of patching all the pathes to binaries seems better
> to me.

I'm not sure if I got you correctly: do you prefer to wrap Emacs with
the tools it needs in PATH, or patch the references exactly in its
source, as Liliana suggested?

I've tried the "exact" patch suggested by Liliana in v2.  I tested that
reading a manual page was possible in a containerized environment still
worked.
Andrew Tropin Jan. 2, 2024, 7 a.m. UTC | #5
On 2024-01-01 21:07, Maxim Cournoyer wrote:

> Hi Andrew,
>
> Andrew Tropin <andrew@trop.in> writes:
>
> [...]
>
>>>> We already have a phase to patch in the real path of /bin/sh where it's
>>>> used.  This appears to be an odd case that's missed.
>>>
>>> I appreciate exactness, but it seems fragile to rely on nobody adding
>>> new references or someone catching them as new Emacs modules get added
>>> or changed :-).
>>>
>>> My reasoning was that since Emacs already depends on bash, why not
>>> ensure it'll always be found on PATH, by wrapping instead of
>>> substituting.
>>>
>>> Does it make sense?
>>
>> Yep, make sense to me. I also find cases from time to time, when some
>> binary or another isn't found by some elisp code.
>>
>> However, providing those binaries via PATH can make some code or
>> programs to work, when executed from inside Emacs and not to work in the
>> environment outside, which can be really confusing in some cases.
>>
>> A simple example, imaging we have a script: 1.sh, which contains:
>> sh --version
>>
>> This one will work:
>> guix shell emacs-with-bash --pure -- emacs --eval '(shell-command "./1.sh")'
>>
>> This one will not:
>> guix shell emacs-with-bash --pure -- ./1.sh
>>
>> That said, the idea of patching all the pathes to binaries seems better
>> to me.
>
> I'm not sure if I got you correctly: do you prefer to wrap Emacs with
> the tools it needs in PATH, or patch the references exactly in its
> source, as Liliana suggested?

I'm more on the "patching the references exactly" side to avoid the
problem mentioned above.

>
> I've tried the "exact" patch suggested by Liliana in v2.  I tested that
> reading a manual page was possible in a containerized environment still
> worked.

👍
Maxim Cournoyer Jan. 2, 2024, 4:34 p.m. UTC | #6
Hi!

Andrew Tropin <andrew@trop.in> writes:

[...]

> I'm more on the "patching the references exactly" side to avoid the
> problem mentioned above.

OK, thanks for clarifying!  You'll see this done in the v2 already sent
to this ticket.
Maxim Cournoyer Jan. 19, 2024, 3:17 a.m. UTC | #7
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hi!
>
> Andrew Tropin <andrew@trop.in> writes:
>
> [...]
>
>> I'm more on the "patching the references exactly" side to avoid the
>> problem mentioned above.
>
> OK, thanks for clarifying!  You'll see this done in the v2 already sent
> to this ticket.

I've now applied this to master, in its exact variant posted in v2.
diff mbox series

Patch

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index b9d9e2b891..85d40d01de 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -9,7 +9,7 @@ 
 ;;; Copyright © 2016 David Thompson <dthompson2@worcester.edu>
 ;;; Copyright © 2016 Nikita <nikita@n0.is>
 ;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
-;;; Copyright © 2017, 2019, 2020 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2017, 2019, 2020, 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2017 Alex Vong <alexvong1995@gmail.com>
 ;;; Copyright © 2017, 2018 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2017, 2023 Janneke Nieuwenhuizen <janneke@gnu.org>
@@ -59,6 +59,7 @@  (define-module (gnu packages emacs)
   #:use-module (gnu packages fontutils)
   #:use-module (gnu packages freedesktop)
   #:use-module (gnu packages fribidi)
+  #:use-module (gnu packages gawk)
   #:use-module (gnu packages gcc)
   #:use-module (gnu packages gd)
   #:use-module (gnu packages gettext)
@@ -94,6 +95,7 @@  (define (%emacs-modules build-system)
     `((guix build ,(symbol-append which '-build-system))
       (guix build utils)
       (srfi srfi-1)
+      (srfi srfi-26)
       (ice-9 ftw))))
 
 (define-public emacs-minimal
@@ -231,12 +233,17 @@  (define-public emacs-minimal
                    (wrap-program prog
                      ;; Some variants rely on uname being in PATH for Tramp.
                      ;; Tramp paths can't be hardcoded, because they need to
-                     ;; be portable.
+                     ;; be portable.  "sh", "find", "awk" and "sed" are also
+                     ;; needed by common Elisp modules e.g., for reading
+                     ;; documentation, so have them on PATH.
                      `("PATH" suffix
-                       ,(map dirname
-                             (list (search-input-file inputs "/bin/gzip")
-                                   ;; for coreutils
-                                   (search-input-file inputs "/bin/yes"))))
+                       ,(map (compose dirname (cut search-input-file inputs <>))
+                             (list "bin/awk"
+                                   "bin/find" ;findutils
+                                   "bin/gzip"
+                                   "bin/sed"
+                                   "bin/sh"     ;bash
+                                   "bin/yes"))) ;coreutils
                      `("EMACSLOADPATH" suffix ,lisp-dirs)))
                  (find-files (string-append out "/bin")
                              ;; Matches versioned and unversioned emacs binaries.
@@ -254,7 +261,7 @@  (define-public emacs-minimal
                 (copy-file
                  (car (find-files "bin" "^emacs-([0-9]+\\.)+[0-9]+$"))
                  "bin/emacs")))))))
-    (inputs (list bash-minimal coreutils gzip ncurses))
+    (inputs (list bash-minimal coreutils findutils gawk gzip ncurses sed))
     (native-inputs (list autoconf pkg-config texinfo))
     (home-page "https://www.gnu.org/software/emacs/")
     (synopsis "The extensible text editor (minimal build for byte-compilation)")