Re: executable-set-magic update

Re: executable-set-magic update

[Andrew L. Moore]

>> It would also be nice if `executable-set-magic’ were supported beyond
>> `sh-set-shell’ (in lisp/progmodes/sh-script.el), so I implemented
>> a minor mode that adds the following hook to find-file:
>>
>> (defun script-set-magic ()
>>   "Look up interpreter associated with current major mode in
>> `script-set-magic-alist' and call `executable-set-magic'."
>>   (let ((interpreter (alist-get major-mode script-set-magic-alist)))
>>     (if interpreter (executable-set-magic interpreter)))
>>   )

> Stefan Monnier wrote:
> Doesn't this add a #! to every file using modes like
> js/ruby/awk/perl/python/...?
> 
> In multi-file programs, only the main file needs a "#!", so I don't
> understand why adding #! to all files would make sense.

Stefan,
Sorry I missed your comment.  Yeah, script libraries don’t use magic numbers, so
I updated the minor mode with a variable that allows skipping files in “project”
directories.  And since the minor mode leverages executable-set-magic, it's been
renamed `executable-set-magic-mode’:

 https://github.com/slewsys/emacs-extensions/blob/master/executable-set-magic.el

The current implementation searches for a “project root” in the file path
by brute force. Hopefully that can be updated with something more elegant.

In the mean time, function `executable-set-magic' only seems to be leveraged
by lisp/progmodes/sh-script.el for shell scripts.  So an alternative to
extending it might be removing it, e.g., to an external library.
-AM

Re: executable-set-magic update

[Andrew L. Moore]

> On Jul 24, 2017, at 2:22 PM, Andrew L. Moore <slewsys@gmail.com> wrote:
> 
>>> It would also be nice if `executable-set-magic’ were supported beyond
>>> `sh-set-shell’ (in lisp/progmodes/sh-script.el), so I implemented
>>> a minor mode that adds the following hook to find-file:
>>> 
>>> (defun script-set-magic ()
>>>  "Look up interpreter associated with current major mode in
>>> `script-set-magic-alist' and call `executable-set-magic'."
>>>  (let ((interpreter (alist-get major-mode script-set-magic-alist)))
>>>    (if interpreter (executable-set-magic interpreter)))
>>>  )
> 
>> Stefan Monnier wrote:
>> Doesn't this add a #! to every file using modes like
>> js/ruby/awk/perl/python/...?
>> 
>> In multi-file programs, only the main file needs a "#!", so I don't
>> understand why adding #! to all files would make sense.
> 
> Stefan,
> Sorry I missed your comment.  Yeah, script libraries don’t use magic numbers, so
> I updated the minor mode with a variable that allows skipping files in “project”
> directories.  And since the minor mode leverages executable-set-magic, it's been
> renamed `executable-set-magic-mode’:
> 
> https://github.com/slewsys/emacs-extensions/blob/master/executable-set-magic.el
> 
> The current implementation searches for a “project root” in the file path
> by brute force. Hopefully that can be updated with something more elegant.
> 
> In the mean time, function `executable-set-magic' only seems to be leveraged
> by lisp/progmodes/sh-script.el for shell scripts.  So an alternative to
> extending it might be removing it, e.g., to an external library.
> -AM

Oops, the “project root” search was ugly too.  Reversing the order of the functions,
hopefully it’s just inelegant now.
-AM

executable-set-magic update

[Andrew L. Moore]

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

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 “.

It would also be nice if `executable-set-magic’ were supported beyond `sh-set-shell’ (in lisp/progmodes/sh-script.el), so I implemented a minor mode that adds the following hook to find-file:

(defun script-set-magic ()
  "Look up interpreter associated with current major mode in
`script-set-magic-alist' and call `executable-set-magic'."
  (let ((interpreter (alist-get major-mode script-set-magic-alist)))
    (if interpreter (executable-set-magic interpreter)))
  )

The minor mode file is available as: <https://github.com/slewsys/emacs-extensions/blob/master/script-set-magic.el>
-AM


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

diff --git a/lisp/progmodes/executable.el b/lisp/progmodes/executable.el
index da148bd39a..9464370a91 100644
--- a/lisp/progmodes/executable.el
+++ b/lisp/progmodes/executable.el
@@ -90,6 +90,12 @@ executable-prefix
   :type 'string
   :group 'executable)
 
+(defcustom executable-interpreter-path-absolute t
+  "If non-nil, `executable-set-magic' uses the interpreter's
+absolute path. Otherwise, it's basename is used."
+  :version "26.0"
+  :type 'boolean
+  :group 'executable)
 
 (defcustom executable-chmod 73
   "After saving, if the file is not executable, set this mode.
@@ -220,6 +226,9 @@ executable-set-magic
                          (and argument (string< "" argument) " ")
                          argument))
 
+  (if (not executable-interpreter-path-absolute)
+      (setq argument (file-name-nondirectory argument)))
+
   (or buffer-read-only
       (if buffer-file-name
           (string-match executable-magicless-file-regexp
@@ -229,27 +238,26 @@ executable-set-magic
       (save-excursion
         (goto-char (point-min))
         (add-hook 'after-save-hook 'executable-chmod nil t)
+        (let ((new-magic (concat (substring executable-prefix 2) argument)))
           (if (looking-at "#![ \t]*\\(.*\\)$")
               (and (goto-char (match-beginning 1))
                    ;; If the line ends in a space,
                    ;; don't offer to change it.
                    (not (= (char-after (1- (match-end 1))) ?\s))
-		 (not (string= argument
+                   (not (string= new-magic
                                  (buffer-substring (point) (match-end 1))))
                    (if (or (not executable-query) no-query-flag
                            (save-window-excursion
                              ;; 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'? "
+                                        new-magic))))
                        (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)))))
+                         (replace-match new-magic t t nil 1)
+                         (message "Magic number changed to `#!%s'" new-magic))))
+            (insert "#!" new-magic ?\n)
+            (message "Magic number changed to `#!%s'" new-magic)))))
   interpreter)
 
 

Re: executable-set-magic update

[Eli Zaretskii]

> 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?

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.

> +  :version "26.0"

Emacs never releases N.0 versions, so this should be "26.1".

> @@ -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.

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

Re: executable-set-magic update

[Andrew L. Moore]

[-- 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)

Re: executable-set-magic update

[Eli Zaretskii]

> From: "Andrew L. Moore" <slewsys@gmail.com>
> Date: Sat, 10 Jun 2017 15:31:04 -0400
> Cc: emacs-devel@gnu.org
> 
> > 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:

Thanks, pushed to master.

With this contribution, you've exhausted the amount of changes we can
accept without legal papers, so I'd encourage you to start the
paperwork rolling, for us to be able to accept further contributions.

Re: executable-set-magic update

[Stefan Monnier]

> It would also be nice if `executable-set-magic’ were supported beyond
> `sh-set-shell’ (in lisp/progmodes/sh-script.el), so I implemented
> a minor mode that adds the following hook to find-file:
>
> (defun script-set-magic ()
>   "Look up interpreter associated with current major mode in
> `script-set-magic-alist' and call `executable-set-magic'."
>   (let ((interpreter (alist-get major-mode script-set-magic-alist)))
>     (if interpreter (executable-set-magic interpreter)))
>   )

Doesn't this add a #! to every file using modes like
js/ruby/awk/perl/python/...?

In multi-file programs, only the main file needs a "#!", so I don't
understand why adding #! to all files would make sense.


        Stefan