GNU bug report logs - #66902
30.0.50; Recognize env -S/--split-string in shebangs

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 66902 in the body.
You can then email your comments to 66902 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; Recognize env -S/--split-string in shebangs
Date: Thu, 02 Nov 2023 21:57:25 +0100

[Message part 1 (text/plain, inline)]

Hello,

While Emacs correctly picks makefile-gmake-mode when visiting a file
with the following shebang:

  #!/usr/bin/make -f

It fails to do so for this shebang:

  #!/usr/bin/env -S make -f

env(1) suggests -S is the idiomatic way to pass arguments to programs in
shebangs:

>    -S/--split-string usage in scripts
>        The -S option allows specifying multiple parameters in a script.
>        Running a script named 1.pl containing the following first line:
> 
>               #!/usr/bin/env -S perl -w -T
>               ...
> 
>        Will execute perl -w -T 1.pl .
> 
>        Without the '-S' parameter the script will likely fail with:
> 
>               /usr/bin/env: 'perl -w -T': No such file or directory

I've poked at lisp/files.el; the attached diff seems sufficient to make
Emacs pick makefile-gmake-mode .

[files.diff (text/x-diff, inline)]

diff --git a/lisp/files.el b/lisp/files.el
index 3d838cd3b8c..97594ff8a13 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3246,7 +3246,8 @@ inhibit-local-variables-p
 
 (defvar auto-mode-interpreter-regexp
   (purecopy "#![ \t]?\\([^ \t\n]*\
-/bin/env[ \t]\\)?\\([^ \t\n]+\\)")
+/bin/env[ \t]\\(?:\\(?:-S\\|--split-string\\)[ \t]\\)?\\)?\
+\\([^ \t\n]+\\)")
   "Regexp matching interpreters, for file mode determination.
 This regular expression is matched against the first line of a file
 to determine the file's mode in `set-auto-mode'.  If it matches, the file

[Message part 3 (text/plain, inline)]

Questions before proceeding to ChangeLog entries & regression tests:

1. Is this something we would like Emacs to recognize out of the box, or
is it too niche?

2. What about the more general forms shown in (info "(coreutils) env
invocation")?

  #!/usr/bin/env -[v]S[OPTION]... [NAME=VALUE]... COMMAND [ARGS]...

3. Assuming we do want to amend that regexp, would it be possible to use
rx here?  OT1H guessing "no" because files.el is pre-reloaded, whereas
rx.el is not; OTOH I see that files.el requires easy-mmode at
compile-time, and that package does not show up in loadup.el, so…
settling for "maybe?"

WDYT?


In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.37, cairo version 1.16.0) of 2023-08-20 built on hirondell
Repository revision: 652e45b70d82e6f615febe00553dbded80557845
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101007
System Description: Debian GNU/Linux 12 (bookworm)

Configured using:
 'configure
 --cache-file=/home/peniblec/.cache/emacs/config,src,emacs,master
 --with-cairo --with-gconf --with-sqlite3 --with-xinput2'

Configured features:
ACL CAIRO DBUS FREETYPE GCONF GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ
JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES
NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Message #8 received at 66902 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: 66902 <at> debbugs.gnu.org
Subject: Re: bug#66902: 30.0.50; Recognize env -S/--split-string in shebangs
Date: Sun, 12 Nov 2023 18:53:40 +0100

[Message part 1 (text/plain, inline)]

Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> Questions before proceeding to ChangeLog entries & regression tests:

For better or worse, I ended up proceeding to both these things, and
then some.  Let me know if the attached patches make sense; tested with

  make -j8 bootstrap && make -C test files-tests


Tentative answers to my questions:

> 1. Is this something we would like Emacs to recognize out of the box, or
> is it too niche?

Assuming yes.

> 2. What about the more general forms shown in (info "(coreutils) env
> invocation")?
>
>   #!/usr/bin/env -[v]S[OPTION]... [NAME=VALUE]... COMMAND [ARGS]...

Didn't go as far as handling -v nor NAME=VALUE pairs, but that could be
added later if we ever feel like it.

> 3. Assuming we do want to amend that regexp, would it be possible to use
> rx here?  OT1H guessing "no" because files.el is pre-reloaded, whereas
> rx.el is not; OTOH I see that files.el requires easy-mmode at
> compile-time, and that package does not show up in loadup.el, so…
> settling for "maybe?"

Figured rx was similar to pcase in that regard:

* They need to be required explicitly despite their macros being
  "autoloaded", because files.el is loaded during bootstrap before
  autoloading is set up.

* Somehow that does not cause them to be preloaded?  At least going by
  emacs -Q,
  * featurep returns nil,
  * preloaded-file-list does not include them.

[0001-Add-basic-tests-for-interpreter-mode-alist.patch (text/x-patch, attachment)]

[0002-Convert-auto-mode-interpreter-regexp-to-an-rx-form.patch (text/x-patch, attachment)]

[0003-Recognize-shebang-lines-that-pass-S-split-string-to-.patch (text/x-patch, attachment)]

Message #11 received at 66902 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 66902 <at> debbugs.gnu.org
Subject: Re: bug#66902: 30.0.50; Recognize env -S/--split-string in shebangs
Date: Sat, 18 Nov 2023 11:41:52 +0200

> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Date: Sun, 12 Nov 2023 18:53:40 +0100
> 
> > 3. Assuming we do want to amend that regexp, would it be possible to use
> > rx here?  OT1H guessing "no" because files.el is pre-reloaded, whereas
> > rx.el is not; OTOH I see that files.el requires easy-mmode at
> > compile-time, and that package does not show up in loadup.el, so…
> > settling for "maybe?"
> 
> Figured rx was similar to pcase in that regard:
> 
> * They need to be required explicitly despite their macros being
>   "autoloaded", because files.el is loaded during bootstrap before
>   autoloading is set up.
> 
> * Somehow that does not cause them to be preloaded?  At least going by
>   emacs -Q,
>   * featurep returns nil,
>   * preloaded-file-list does not include them.

I'd prefer not to have rx required in files.el, so could you please
rewrite those parts of your patch and resubmit?  Also, please add a
NEWS entry about the change.  I think otherwise your patch is ready to
go in.

Thanks.

Message #14 received at 66902 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 66902 <at> debbugs.gnu.org
Subject: Re: bug#66902: 30.0.50; Recognize env -S/--split-string in shebangs
Date: Sat, 18 Nov 2023 11:31:29 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
>> Date: Sun, 12 Nov 2023 18:53:40 +0100
>> 
>> > 3. Assuming we do want to amend that regexp, would it be possible to use
>> > rx here?  OT1H guessing "no" because files.el is pre-reloaded, whereas
>> > rx.el is not; OTOH I see that files.el requires easy-mmode at
>> > compile-time, and that package does not show up in loadup.el, so…
>> > settling for "maybe?"
>> 
>> Figured rx was similar to pcase in that regard:
>> 
>> * They need to be required explicitly despite their macros being
>>   "autoloaded", because files.el is loaded during bootstrap before
>>   autoloading is set up.
>> 
>> * Somehow that does not cause them to be preloaded?  At least going by
>>   emacs -Q,
>>   * featurep returns nil,
>>   * preloaded-file-list does not include them.
>
> I'd prefer not to have rx required in files.el, so could you please
> rewrite those parts of your patch and resubmit?  Also, please add a
> NEWS entry about the change.  

ACK; will get to it in the coming days.

>                               I think otherwise your patch is ready to
> go in.
>
> Thanks.

Thank you for the review!

Message #17 received at 66902 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 66902 <at> debbugs.gnu.org
Subject: Re: bug#66902: 30.0.50; Recognize env -S/--split-string in shebangs
Date: Sat, 18 Nov 2023 18:44:06 +0100

[Message part 1 (text/plain, inline)]

Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>> I'd prefer not to have rx required in files.el, so could you please
>> rewrite those parts of your patch and resubmit?  Also, please add a
>> NEWS entry about the change.  
>
> ACK; will get to it in the coming days.

s/days/hours/

I left a 'concat' in, because (a) it lets us interleave comments (b) the
byte-compiler seems to smartly condense it all to one big string literal
anyway.  (Though if files.el is preloaded, everything happens at
build-time and the .elc does not matter much, IIUC?)

Let me know if we would prefer a plain raw string literal.

Added a NEWS entry (under § 'Changes in Emacs 30.1 / Miscellaneous',
assuming 'master'); added a bug reference; squashed it all.

[0001-Recognize-shebang-lines-that-pass-S-split-string-to-.patch (text/x-patch, attachment)]

Message #22 received at 66902-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 66902-done <at> debbugs.gnu.org
Subject: Re: bug#66902: 30.0.50; Recognize env -S/--split-string in shebangs
Date: Sun, 19 Nov 2023 11:09:38 +0200

> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: 66902 <at> debbugs.gnu.org
> Date: Sat, 18 Nov 2023 18:44:06 +0100
> 
> Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
> 
> > Eli Zaretskii <eliz <at> gnu.org> writes:
> >
> >> I'd prefer not to have rx required in files.el, so could you please
> >> rewrite those parts of your patch and resubmit?  Also, please add a
> >> NEWS entry about the change.  
> >
> > ACK; will get to it in the coming days.
> 
> s/days/hours/
> 
> I left a 'concat' in, because (a) it lets us interleave comments (b) the
> byte-compiler seems to smartly condense it all to one big string literal
> anyway.  (Though if files.el is preloaded, everything happens at
> build-time and the .elc does not matter much, IIUC?)
> 
> Let me know if we would prefer a plain raw string literal.
> 
> Added a NEWS entry (under § 'Changes in Emacs 30.1 / Miscellaneous',
> assuming 'master'); added a bug reference; squashed it all.

Thanks, installed on the master branch, and closing the bug.

Message #25 received at 66902-done <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 66902-done <at> debbugs.gnu.org
Subject: Re: bug#66902: 30.0.50; Recognize env -S/--split-string in shebangs
Date: Sun, 19 Nov 2023 11:51:19 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

> Thanks, installed on the master branch, and closing the bug.

Thanks for all that, and the copyedit 🙏

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.