bug#64939: 30.0.50; The default auto-mode-interpreter-regexp does not match env with flags

["Kévin Le Gouguec" <kevin.legouguec@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]

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

> Is this Good Enough™ for your purposes (Malcolm, Wilhelm), or should we
> sophisticate the regexp further?  FWIW, in no particular order:
>
> (a) env(1) does seem to support mixing up arbitrary options with -S¹, so
>     in principle it would make sense to support that;
>
> (b) Eli did not seem too found of the regexp hammer², so I don't know
>     which direction we'd want to go between maximally correct (accept
>     all arguments, _as long as_ -S|--split-string is in there) or good
>     enough (just skip over --everything --that --looks --like -a
>     --switch).
>
> (c) FWIW the "maximally correct" regexp might not be _that_ ugly, since
>     "-[v]S[OPTION]" must be the *first* token after env; in other words
>     no need to support --some-option --split-string --more-options.

Well, sorry, couldn't resist.  How do the attached patches look?  The
new testcases should tell the whole story.

('make && make -C test files-tests' seems none the worse for wear)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Refine-shebang-tests-bug-64939.patch --]
[-- Type: text/x-patch, Size: 3494 bytes --]

From 0a3dcfa9d5859c8a7ecd4679b748298b0f5d3597 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
Date: Sat, 10 Feb 2024 16:14:08 +0100
Subject: [PATCH 1/3] Refine shebang tests (bug#64939)

* test/lisp/files-tests.el (files-tests--check-shebang): For
shell-script modes, verify that the correct shell is set.
(files-tests-auto-mode-interpreter): Prefer sh-base-mode to
sh-mode to stay tree-sitter-agnostic; re-organize test cases to
make future ones easier to add.
---
 test/lisp/files-tests.el | 45 ++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index 718ecd51f8b..23516ff0d7d 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -1656,30 +1656,39 @@ files-tests-file-name-base
   (should (equal (file-name-base "foo") "foo"))
   (should (equal (file-name-base "foo/bar") "bar")))
 
-(defun files-tests--check-shebang (shebang expected-mode)
-  "Assert that mode for SHEBANG derives from EXPECTED-MODE."
-  (let ((actual-mode
-         (ert-with-temp-file script-file
-           :text shebang
-           (find-file script-file)
-           (if (derived-mode-p expected-mode)
-               expected-mode
-             major-mode))))
-    ;; Tuck all the information we need in the `should' form: input
-    ;; shebang, expected mode vs actual.
-    (should
-     (equal (list shebang actual-mode)
-            (list shebang expected-mode)))))
+(defvar sh-shell)
+
+(defun files-tests--check-shebang (shebang expected-mode &optional expected-dialect)
+  "Assert that mode for SHEBANG derives from EXPECTED-MODE.
+
+If EXPECTED-MODE is sh-base-mode, DIALECT says what `sh-shell' should be
+set to."
+  (ert-with-temp-file script-file
+    :text shebang
+    (find-file script-file)
+    (let ((actual-mode (if (derived-mode-p expected-mode)
+                           expected-mode
+                         major-mode)))
+      ;; Tuck all the information we need in the `should' form: input
+      ;; shebang, expected mode vs actual.
+      (should
+       (equal (list shebang actual-mode)
+              (list shebang expected-mode)))
+      (when (eq expected-mode 'sh-base-mode)
+        (should (eq sh-shell expected-dialect))))))
 
 (ert-deftest files-tests-auto-mode-interpreter ()
   "Test that `set-auto-mode' deduces correct modes from shebangs."
-  (files-tests--check-shebang "#!/bin/bash" 'sh-mode)
-  (files-tests--check-shebang "#!/usr/bin/env bash" 'sh-mode)
+  ;; Straightforward interpreter invocation.
+  (files-tests--check-shebang "#!/bin/bash" 'sh-base-mode 'bash)
+  (files-tests--check-shebang "#!/usr/bin/make -f" 'makefile-mode)
+  ;; Invocation through env.
+  (files-tests--check-shebang "#!/usr/bin/env bash" 'sh-base-mode 'bash)
   (files-tests--check-shebang "#!/usr/bin/env python" 'python-base-mode)
   (files-tests--check-shebang "#!/usr/bin/env python3" 'python-base-mode)
+  ;; Invocation through env, with supplementary arguments.
   (files-tests--check-shebang "#!/usr/bin/env -S awk -v FS=\"\\t\" -v OFS=\"\\t\" -f" 'awk-mode)
-  (files-tests--check-shebang "#!/usr/bin/env -S make -f" 'makefile-mode)
-  (files-tests--check-shebang "#!/usr/bin/make -f" 'makefile-mode))
+  (files-tests--check-shebang "#!/usr/bin/env -S make -f" 'makefile-mode))
 
 (ert-deftest files-test-dir-locals-auto-mode-alist ()
   "Test an `auto-mode-alist' entry in `.dir-locals.el'"
-- 
2.43.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Support-more-complex-env-invocations-in-shebang-line.patch --]
[-- Type: text/x-patch, Size: 3003 bytes --]

From ec011e258fa3a7697dad631d82bbd53e5bc93f50 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
Date: Sat, 10 Feb 2024 17:37:35 +0100
Subject: [PATCH 2/3] Support more complex env invocations in shebang lines

This is not an exact re-implementation of what env accepts, but
hopefully it should be "good enough".

Example of known limitation: we assume that arguments for
--long-options will be set with '=', but that is not
necessarily the case.  '--unset' (mandatory argument) can be
passed as '--unset=VAR' or '--unset VAR', but
'--default-signal' (optional argument) requires an '=' sign.

For bug#64939.

* lisp/files.el (auto-mode-interpreter-regexp): Account for
supplementary arguments passed beside -S/--split-string.
* test/lisp/files-tests.el (files-tests-auto-mode-interpreter):
Test some of these combinations.
---
 lisp/files.el            | 8 +++++++-
 test/lisp/files-tests.el | 8 +++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index f67b650cb92..5098d49048e 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3274,7 +3274,13 @@ auto-mode-interpreter-regexp
     ;; Optional group 1: env(1) invocation.
     "\\("
     "[^ \t\n]*/bin/env[ \t]*"
-    "\\(?:-S[ \t]*\\|--split-string\\(?:=\\|[ \t]*\\)\\)?"
+    ;; Within group 1: possible -S/--split-string.
+    "\\(?:"
+    ;; -S/--split-string
+    "\\(?:-[0a-z]*S[ \t]*\\|--split-string=\\)"
+    ;; More env arguments.
+    "\\(?:-[^ \t\n]+[ \t]+\\)*"
+    "\\)?"
     "\\)?"
     ;; Group 2: interpreter.
     "\\([^ \t\n]+\\)"))
diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index 23516ff0d7d..0a5c3b897e4 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -1687,8 +1687,14 @@ files-tests-auto-mode-interpreter
   (files-tests--check-shebang "#!/usr/bin/env python" 'python-base-mode)
   (files-tests--check-shebang "#!/usr/bin/env python3" 'python-base-mode)
   ;; Invocation through env, with supplementary arguments.
+  (files-tests--check-shebang "#!/usr/bin/env --split-string=bash -eux" 'sh-base-mode 'bash)
+  (files-tests--check-shebang "#!/usr/bin/env --split-string=-iv --default-signal bash -eux" 'sh-base-mode 'bash)
   (files-tests--check-shebang "#!/usr/bin/env -S awk -v FS=\"\\t\" -v OFS=\"\\t\" -f" 'awk-mode)
-  (files-tests--check-shebang "#!/usr/bin/env -S make -f" 'makefile-mode))
+  (files-tests--check-shebang "#!/usr/bin/env -S make -f" 'makefile-mode)
+  (files-tests--check-shebang "#!/usr/bin/env -S-vi bash -eux" 'sh-base-mode 'bash)
+  (files-tests--check-shebang "#!/usr/bin/env -ivS --default-signal=INT bash -eux" 'sh-base-mode 'bash)
+  (files-tests--check-shebang "#!/usr/bin/env -ivS --default-signal bash -eux" 'sh-base-mode 'bash)
+  (files-tests--check-shebang "#!/usr/bin/env -vS -uFOOBAR bash -eux" 'sh-base-mode 'bash))
 
 (ert-deftest files-test-dir-locals-auto-mode-alist ()
   "Test an `auto-mode-alist' entry in `.dir-locals.el'"
-- 
2.43.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Support-shebang-lines-with-amended-environment.patch --]
[-- Type: text/x-patch, Size: 2275 bytes --]

From 9e24e7f000011105a89e5ce81cd9f16eb9ef15b5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
Date: Sat, 10 Feb 2024 17:56:57 +0100
Subject: [PATCH 3/3] Support shebang lines with amended environment

For bug#64939.

* lisp/files.el (auto-mode-interpreter-regexp): Account for possible
VARIABLE=[VALUE] operands.
* test/lisp/files-tests.el (files-tests-auto-mode-interpreter):
Add an example from the coreutils manual.
---
 lisp/files.el            | 5 ++++-
 test/lisp/files-tests.el | 4 +++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 5098d49048e..524385edc84 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3274,12 +3274,15 @@ auto-mode-interpreter-regexp
     ;; Optional group 1: env(1) invocation.
     "\\("
     "[^ \t\n]*/bin/env[ \t]*"
-    ;; Within group 1: possible -S/--split-string.
+    ;; Within group 1: possible -S/--split-string and environment
+    ;; adjustments.
     "\\(?:"
     ;; -S/--split-string
     "\\(?:-[0a-z]*S[ \t]*\\|--split-string=\\)"
     ;; More env arguments.
     "\\(?:-[^ \t\n]+[ \t]+\\)*"
+    ;; Interpreter environment modifications.
+    "\\(?:[^ \t\n]+=[^ \t\n]*[ \t]+\\)*"
     "\\)?"
     "\\)?"
     ;; Group 2: interpreter.
diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index 0a5c3b897e4..d4c1ef3ba67 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -1694,7 +1694,9 @@ files-tests-auto-mode-interpreter
   (files-tests--check-shebang "#!/usr/bin/env -S-vi bash -eux" 'sh-base-mode 'bash)
   (files-tests--check-shebang "#!/usr/bin/env -ivS --default-signal=INT bash -eux" 'sh-base-mode 'bash)
   (files-tests--check-shebang "#!/usr/bin/env -ivS --default-signal bash -eux" 'sh-base-mode 'bash)
-  (files-tests--check-shebang "#!/usr/bin/env -vS -uFOOBAR bash -eux" 'sh-base-mode 'bash))
+  (files-tests--check-shebang "#!/usr/bin/env -vS -uFOOBAR bash -eux" 'sh-base-mode 'bash)
+  ;; Invocation through env, with modified environment.
+  (files-tests--check-shebang "#!/usr/bin/env -S PYTHONPATH=/...:${PYTHONPATH} python" 'python-base-mode))
 
 (ert-deftest files-test-dir-locals-auto-mode-alist ()
   "Test an `auto-mode-alist' entry in `.dir-locals.el'"
-- 
2.43.0