Re: executable-set-magic update

["Andrew L. Moore" <slewsys@gmail.com>]

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


> On Jun 10, 2017, at 3:17 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: "Andrew L. Moore" <slewsys@gmail.com>
>> Date: Fri, 9 Jun 2017 17:31:30 -0400
>> 
>> lisp/progmodes/executable.el does not appear to support magic numbers of the form `#/usr/bin/env interpreter’.  One way to extend support is via the attached diff which merely adds a new variable `executable-interpreter-path-absolute’.  Set the new variable to nil and variable `executable-prefix’ to “#!/usr/bin/env “.
> 
> Thanks.
> 
> Wouldn't it be more elegant (and perhaps also safer, security-wise) if
> we supported the special prefix "/usr/bin/env" directly, i.e. without
> feeding it via some kind of "back door", and allowing arbitrary
> strings there?

Attached below is a patch following your suggestions. From NEWS:

** The new variable 'executable-prefix-env' affects the format of
the magic number inserted by 'executable-set-magic'. If non-nil,
the magic number takes the form "#!/usr/bin/env interpreter",
otherwise "#!/path/to/interpreter", which is the default.

+++

As you advised, if ‘executable-prefix’ has been customized to 
something other than “#!” or “#!/usr/bin/env”, it masks the effect of
‘executable-prefix-env’. This behavior is not documented. Instead,
the doc string for ‘executable-prefix’  now adds:

 “… Use of `executable-prefix' is deprecated in favor of `executable-prefix-env’."

> If your proposal is accepted, I think at least its documentation parts
> should be improved:
> 
>> +(defcustom executable-interpreter-path-absolute t
>> +  "If non-nil, `executable-set-magic' uses the interpreter's
>> +absolute path. Otherwise, it's basename is used."
> 
> This doc string leaves out the important stuff: the reason why the
> variable is introduced and how it should be used.  I think the doc
> string should be more helpful by explicitly describing its intended
> usage.

New name and new definition:

(defcustom executable-prefix-env nil
  "If non-nil, the magic number inserted by function `executable-set-magic'
takes the form \"#!/usr/bin/env interpreter\", otherwise
\"#!/path/to/interpreter\"."
  :version "26.1"
  :type 'boolean
  :group 'executable)

> 
>> +  :version "26.0"
> 
> Emacs never releases N.0 versions, so this should be "26.1”.

Okay.

> 
>> @@ -220,6 +226,9 @@ executable-set-magic
>>                          (and argument (string< "" argument) " ")
>>                          argument))
> 
> The doc string of executable-set-magic should mention the variable you
> introduce.

Done - although just by changing a reference from ‘executable-prefix’ to
‘executable-prefix-env’ - i.e., ‘executable-prefix’ is now not referenced in its
deprecated role.


> Finally, there should be a NEWS entry about this new facility.

Thank you!
-AM


[-- Attachment #2: lisp_progmodes_executable.el.diff --]
[-- Type: application/octet-stream, Size: 3945 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index 7972511f7a..a81949efa6 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -94,6 +94,12 @@ required capabilities are found in terminfo.  See the FAQ node
 \f
 * Changes in Emacs 26.1

+** The new variable 'executable-prefix-env' affects the format of
+the magic number inserted by 'executable-set-magic'. If non-nil,
+the magic number takes the form "#!/usr/bin/env interpreter",
+otherwise "#!/path/to/interpreter", which is the default.
+
++++
 ** The variable 'emacs-version' no longer includes the build number.
 This is now stored separately in a new variable, 'emacs-build-number'.

diff --git a/lisp/progmodes/executable.el b/lisp/progmodes/executable.el
index da148bd39a..5decc92e27 100644
--- a/lisp/progmodes/executable.el
+++ b/lisp/progmodes/executable.el
@@ -83,13 +83,20 @@ executable-magicless-file-regexp
   :type 'regexp
   :group 'executable)

-
 (defcustom executable-prefix "#!"
-  "Interpreter magic number prefix inserted when there was no magic number."
-  :version "24.3"                       ; "#! " -> "#!"
+  "Interpreter magic number prefix inserted when there was no magic number.
+Use of `executable-prefix' is deprecated in favor of `executable-prefix-env'."
+  :version "26.1"                       ; deprecated
   :type 'string
   :group 'executable)

+(defcustom executable-prefix-env nil
+  "If non-nil, the magic number inserted by function `executable-set-magic'
+takes the form \"#!/usr/bin/env interpreter\", otherwise
+\"#!/path/to/interpreter\"."
+  :version "26.1"
+  :type 'boolean
+  :group 'executable)

 (defcustom executable-chmod 73
   "After saving, if the file is not executable, set this mode.
@@ -199,7 +206,7 @@ executable-interpret
 (defun executable-set-magic (interpreter &optional argument
                                          no-query-flag insert-flag)
   "Set this buffer's interpreter to INTERPRETER with optional ARGUMENT.
-The variables `executable-magicless-file-regexp', `executable-prefix',
+The variables `executable-magicless-file-regexp', `executable-prefix-env',
 `executable-insert', `executable-query' and `executable-chmod' control
 when and how magic numbers are inserted or replaced and scripts made
 executable."
@@ -220,6 +227,14 @@ executable-set-magic
                          (and argument (string< "" argument) " ")
                          argument))

+  ;; For backward compatibilty, allow `executable-prefix-env' to be
+  ;; overriden by custom `executable-prefix'.
+  (if (string-match "#!\\([ \t]*/usr/bin/env[ \t]*\\)?$" executable-prefix)
+      (if executable-prefix-env
+          (setq argument (concat "/usr/bin/env "
+                                 (file-name-nondirectory argument))))
+    (setq argument (concat (substring executable-prefix 2) argument)))
+
   (or buffer-read-only
       (if buffer-file-name
           (string-match executable-magicless-file-regexp
@@ -241,15 +256,13 @@ executable-set-magic
                            ;; Make buffer visible before question.
                            (switch-to-buffer (current-buffer))
                            (y-or-n-p (format-message
-          "Replace magic number by `%s%s'? "
-          executable-prefix argument))))
+                                      "Replace magic number by `#!%s'? "
+                                      argument))))
                      (progn
                        (replace-match argument t t nil 1)
-         (message "Magic number changed to `%s'"
-				(concat executable-prefix argument)))))
-   (insert executable-prefix argument ?\n)
-   (message "Magic number changed to `%s'"
-     (concat executable-prefix argument)))))
+                       (message "Magic number changed to `#!%s'" argument))))
+          (insert "#!" argument ?\n)
+          (message "Magic number changed to `#!%s'" argument))))
   interpreter)