process-file instead of call-process in proced.el?

process-file instead of call-process in proced.el?

[Filipp Gunbin]

There's this TODO in proced.el:

--8<---------------cut here---------------start------------->8---
;; - Allow "sudo kill PID", "sudo renice PID"
--8<---------------cut here---------------end--------------->8---

WDYT, can we just use process-file instead of call-process?  Then, if
proced is opened from a /sudo:: (or su) buffer, and (setq
proced-signal-function "kill") is set, proced-send-signal "just works"
for processes owned by different user, with no special code in proced.

The simple patch is below.

Filipp.


diff --git a/lisp/proced.el b/lisp/proced.el
index c1d599afc4..7d324a37a3 100644
--- a/lisp/proced.el
+++ b/lisp/proced.el
@@ -1833,7 +1833,7 @@ proced-send-signal
         (dolist (process process-alist)
           (with-temp-buffer
             (condition-case nil
-                (unless (zerop (call-process
+                (unless (zerop (process-file
                                 proced-signal-function nil t nil
                                 signal (number-to-string (car process))))
                   (proced-log (current-buffer))
@@ -1875,7 +1875,7 @@ proced-renice
     (dolist (process process-alist)
       (with-temp-buffer
         (condition-case nil
-            (unless (zerop (call-process
+            (unless (zerop (process-file
                             proced-renice-command nil t nil
                             priority (number-to-string (car process))))
               (proced-log (current-buffer))

Re: process-file instead of call-process in proced.el?

[Eli Zaretskii]

> From: Filipp Gunbin <fgunbin@fastmail.fm>
> Date: Fri, 25 Mar 2022 03:09:12 +0300
> Cc: winkler@gnu.org
> 
> There's this TODO in proced.el:
> 
> --8<---------------cut here---------------start------------->8---
> ;; - Allow "sudo kill PID", "sudo renice PID"
> --8<---------------cut here---------------end--------------->8---
> 
> WDYT, can we just use process-file instead of call-process?  Then, if
> proced is opened from a /sudo:: (or su) buffer, and (setq
> proced-signal-function "kill") is set, proced-send-signal "just works"
> for processes owned by different user, with no special code in proced.
> 
> The simple patch is below.

Thanks, but this should be at least conditioned on systems that have
these commands available.  MS-Windows, for example, has neither 'kill'
nor 'renice', and the equivalents of 'kill' that I know about don't
accept a signal ID as a command-line argument.

Re: process-file instead of call-process in proced.el?

[Filipp Gunbin]

On 25/03/2022 09:54 +0300, Eli Zaretskii wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Date: Fri, 25 Mar 2022 03:09:12 +0300
>> Cc: winkler@gnu.org
>> 
>> There's this TODO in proced.el:
>> 
>> --8<---------------cut here---------------start------------->8---
>> ;; - Allow "sudo kill PID", "sudo renice PID"
>> --8<---------------cut here---------------end--------------->8---
>> 
>> WDYT, can we just use process-file instead of call-process?  Then, if
>> proced is opened from a /sudo:: (or su) buffer, and (setq
>> proced-signal-function "kill") is set, proced-send-signal "just works"
>> for processes owned by different user, with no special code in proced.
>> 
>> The simple patch is below.
>
> Thanks, but this should be at least conditioned on systems that have
> these commands available.  MS-Windows, for example, has neither 'kill'
> nor 'renice', and the equivalents of 'kill' that I know about don't
> accept a signal ID as a command-line argument.

The patch only changes call-process to process-file, to support file
handlers.  Setting "kill" into proced-signal-function is a user
customization.

Thanks.

Re: process-file instead of call-process in proced.el?

[Eli Zaretskii]

> From: Filipp Gunbin <fgunbin@fastmail.fm>
> Cc: emacs-devel@gnu.org,  winkler@gnu.org
> Date: Fri, 25 Mar 2022 13:24:10 +0300
> 
> On 25/03/2022 09:54 +0300, Eli Zaretskii wrote:
> 
> >> From: Filipp Gunbin <fgunbin@fastmail.fm>
> >> Date: Fri, 25 Mar 2022 03:09:12 +0300
> >> Cc: winkler@gnu.org
> >> 
> >> There's this TODO in proced.el:
> >> 
> >> --8<---------------cut here---------------start------------->8---
> >> ;; - Allow "sudo kill PID", "sudo renice PID"
> >> --8<---------------cut here---------------end--------------->8---
> >> 
> >> WDYT, can we just use process-file instead of call-process?  Then, if
> >> proced is opened from a /sudo:: (or su) buffer, and (setq
> >> proced-signal-function "kill") is set, proced-send-signal "just works"
> >> for processes owned by different user, with no special code in proced.
> >> 
> >> The simple patch is below.
> >
> > Thanks, but this should be at least conditioned on systems that have
> > these commands available.  MS-Windows, for example, has neither 'kill'
> > nor 'renice', and the equivalents of 'kill' that I know about don't
> > accept a signal ID as a command-line argument.
> 
> The patch only changes call-process to process-file, to support file
> handlers.  Setting "kill" into proced-signal-function is a user
> customization.

Sorry, from my POV fixing something means making it work as correctly
as we can reasonably do.  If it didn't work correctly before, we
should fix that as well, as part of any work on the relevant code.  I
understand that, if the doing TRT is complicated, it could be left to
a separate patch, but in this case it isn't complicated at all.

So let's fix this part of proced to work on all supported platforms,
okay?

Re: process-file instead of call-process in proced.el?

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

>> The patch only changes call-process to process-file, to support file
>> handlers.  Setting "kill" into proced-signal-function is a user
>> customization.
>
> Sorry, from my POV fixing something means making it work as correctly
> as we can reasonably do.  If it didn't work correctly before, we
> should fix that as well, as part of any work on the relevant code.  I
> understand that, if the doing TRT is complicated, it could be left to
> a separate patch, but in this case it isn't complicated at all.
>
> So let's fix this part of proced to work on all supported platforms,
> okay?

I don't understand why an external "kill" process is applied. Couldn't
we simply call always `signal-process'? A comment in
`proced-send-signal' recommends this already.

Then we could give `signal-process' a handler, like `interrupt-process'
has already. (Well, we would need a mean to indicate, that a process-id
is meant for a remote host.)

Best regards, Michael.

Re: process-file instead of call-process in proced.el?

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Filipp Gunbin <fgunbin@fastmail.fm>,  winkler@gnu.org,  emacs-devel@gnu.org
> Date: Fri, 25 Mar 2022 12:46:06 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> The patch only changes call-process to process-file, to support file
> >> handlers.  Setting "kill" into proced-signal-function is a user
> >> customization.
> >
> > Sorry, from my POV fixing something means making it work as correctly
> > as we can reasonably do.  If it didn't work correctly before, we
> > should fix that as well, as part of any work on the relevant code.  I
> > understand that, if the doing TRT is complicated, it could be left to
> > a separate patch, but in this case it isn't complicated at all.
> >
> > So let's fix this part of proced to work on all supported platforms,
> > okay?
> 
> I don't understand why an external "kill" process is applied. Couldn't
> we simply call always `signal-process'? A comment in
> `proced-send-signal' recommends this already.
> 
> Then we could give `signal-process' a handler, like `interrupt-process'
> has already. (Well, we would need a mean to indicate, that a process-id
> is meant for a remote host.)

Yes, that'd be much better, IMO.

Re: process-file instead of call-process in proced.el?

[Filipp Gunbin]

On 25/03/2022 15:12 +0300, Eli Zaretskii wrote:

>> From: Michael Albinus <michael.albinus@gmx.de>
>> Cc: Filipp Gunbin <fgunbin@fastmail.fm>,  winkler@gnu.org,  emacs-devel@gnu.org
>> Date: Fri, 25 Mar 2022 12:46:06 +0100
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> The patch only changes call-process to process-file, to support file
>> >> handlers.  Setting "kill" into proced-signal-function is a user
>> >> customization.
>> >
>> > Sorry, from my POV fixing something means making it work as correctly
>> > as we can reasonably do.  If it didn't work correctly before, we
>> > should fix that as well, as part of any work on the relevant code.  I
>> > understand that, if the doing TRT is complicated, it could be left to
>> > a separate patch, but in this case it isn't complicated at all.
>> >
>> > So let's fix this part of proced to work on all supported platforms,
>> > okay?
>> 
>> I don't understand why an external "kill" process is applied. Couldn't
>> we simply call always `signal-process'? A comment in
>> `proced-send-signal' recommends this already.
>> 
>> Then we could give `signal-process' a handler, like `interrupt-process'
>> has already. (Well, we would need a mean to indicate, that a process-id
>> is meant for a remote host.)
>
> Yes, that'd be much better, IMO.

These are different things.  What Michael suggests would fix the case
for signal-process, which would be great.  But what I originally
suggested would fix the case for when proced-signal-function is
customized to something else, like "kill", for whatever reasons.  Then
it can be used like I described, running the command from "remote"
sudo/su buffer, whatever it be.

Filipp

Re: process-file instead of call-process in proced.el?

[Roland Winkler]

I have been copied on this thread.  Yet the subtleties it discusses go
beyond my expertise.  I expect I cannot contribute in a meaningful way.

Re: process-file instead of call-process in proced.el?

[Michael Albinus]

Filipp Gunbin <fgunbin@fastmail.fm> writes:

> These are different things.  What Michael suggests would fix the case
> for signal-process, which would be great.

Yes :-)

> But what I originally
> suggested would fix the case for when proced-signal-function is
> customized to something else, like "kill", for whatever reasons.  Then
> it can be used like I described, running the command from "remote"
> sudo/su buffer, whatever it be.

My point is that there's no reason to use proced-signal-function.
signal-process is sufficient, and when we make it remote-host aware,
even better.

> Filipp

Best regards, Michael.

Re: process-file instead of call-process in proced.el?

[Filipp Gunbin]

On 25/03/2022 14:58 +0100, Michael Albinus wrote:

> Filipp Gunbin <fgunbin@fastmail.fm> writes:
>
>> These are different things.  What Michael suggests would fix the case
>> for signal-process, which would be great.
>
> Yes :-)
>
>> But what I originally
>> suggested would fix the case for when proced-signal-function is
>> customized to something else, like "kill", for whatever reasons.  Then
>> it can be used like I described, running the command from "remote"
>> sudo/su buffer, whatever it be.
>
> My point is that there's no reason to use proced-signal-function.
> signal-process is sufficient, and when we make it remote-host aware,
> even better.
>

Currently, there is a reason - signal-process is not remote-host aware
:-) Also there may be other systems on which it does not work for other
reasons.  I mean, if the support for user-specified command is there,
and we don't want to remove it (do we?), then why not fix it, in
parallel with improving signal-process?

Re: process-file instead of call-process in proced.el?

[Michael Albinus]

Filipp Gunbin <fgunbin@fastmail.fm> writes:

> Currently, there is a reason - signal-process is not remote-host aware
> :-) Also there may be other systems on which it does not work for other
> reasons.  I mean, if the support for user-specified command is there,
> and we don't want to remove it (do we?), then why not fix it, in
> parallel with improving signal-process?

My proposal is to remove the user-specified command, yes. There's no
need for it.

Best regards, Michael.

Re: process-file instead of call-process in proced.el?

[Filipp Gunbin]

On 25/03/2022 15:29 +0100, Michael Albinus wrote:

> Filipp Gunbin <fgunbin@fastmail.fm> writes:
>
>> Currently, there is a reason - signal-process is not remote-host aware
>> :-) Also there may be other systems on which it does not work for other
>> reasons.  I mean, if the support for user-specified command is there,
>> and we don't want to remove it (do we?), then why not fix it, in
>> parallel with improving signal-process?
>
> My proposal is to remove the user-specified command, yes. There's no
> need for it.
>
> Best regards, Michael.

So what is best to do now?  Leave it alone until you or someone else
improve signal-process?

Thanks
Filipp

Re: process-file instead of call-process in proced.el?

[Michael Albinus]

Filipp Gunbin <fgunbin@fastmail.fm> writes:

> So what is best to do now?  Leave it alone until you or someone else
> improve signal-process?

Yes, that's the plan. Hopefully, I can work on this, soon.

> Thanks
> Filipp

Best regards, Michael.

Re: process-file instead of call-process in proced.el?

[Michael Albinus]

Filipp Gunbin <fgunbin@fastmail.fm> writes:

Hi Filipp,

> There's this TODO in proced.el:
>
> ;; - Allow "sudo kill PID", "sudo renice PID"
>
> WDYT, can we just use process-file instead of call-process?  Then, if
> proced is opened from a /sudo:: (or su) buffer, and (setq
> proced-signal-function "kill") is set, proced-send-signal "just works"
> for processes owned by different user, with no special code in proced.

I'mn working on the idea to extend `signal-process'. However, when I
call `proced' from a remote buffer, the `proced' listing shows always
the local processes.

Is there a way to let `proced' show remote processes?

> Filipp.

Best regards, Michael.

Re: process-file instead of call-process in proced.el?

[Roland Winkler]

On Sat, Mar 26 2022, Michael Albinus wrote:
> I'mn working on the idea to extend `signal-process'. However, when I
> call `proced' from a remote buffer, the `proced' listing shows always
> the local processes.
>
> Is there a way to let `proced' show remote processes?

I am not all certain I understand the question.  In some sense proced is
a front-end for the built-in functions list-system-processes and
process-attributes, which is the Emacs equivalent of ps(1).  So this
covers only processes of the local machine, and I have no clue how this
could cover another host.

Re: process-file instead of call-process in proced.el?

[Michael Albinus]

Roland Winkler <winkler@gnu.org> writes:

Hi Roland,

>> Is there a way to let `proced' show remote processes?
>
> I am not all certain I understand the question.  In some sense proced is
> a front-end for the built-in functions list-system-processes and
> process-attributes, which is the Emacs equivalent of ps(1).  So this
> covers only processes of the local machine, and I have no clue how this
> could cover another host.

Thanks for the clarification. So in order to apply the proposal of
Fillip, we also need to implement a "remote" version of list-system-processes.

`proced' could show then the processes of the remote system, if
indicated. For example, if the command is executed with a prefix
argument.

Best regards, Michael.

Re: process-file instead of call-process in proced.el?

[Roland Winkler]

On Sat, Mar 26 2022, Michael Albinus wrote:
> Thanks for the clarification. So in order to apply the proposal of
> Fillip, we also need to implement a "remote" version of list-system-processes.
>
> `proced' could show then the processes of the remote system, if
> indicated. For example, if the command is executed with a prefix
> argument.

To clarify the history of proced: it started out as a front-end for
ps(1).  That was a mess because there are too many variants of ps(1) out
in the wild to allow for anything robust.  So Eli kindly wrote the
built-in functions list-system-processes and process-attributes that
provide a well-defined base for proced.  These built-in functions are
not implemented for all platforms and proced does not work for the other
platforms; that's life.  I believe that again it would be a poor crutch
if Emacs tried to run ps(1) remotely for a process listing.  Is it
possible to use a remote instance of emacs to get a process listing on a
remote machine (and let proced do nothing otherwise)?

Re: process-file instead of call-process in proced.el?

[Michael Albinus]

Roland Winkler <winkler@gnu.org> writes:

Hi Roland,

>> Thanks for the clarification. So in order to apply the proposal of
>> Fillip, we also need to implement a "remote" version of list-system-processes.
>>
>> `proced' could show then the processes of the remote system, if
>> indicated. For example, if the command is executed with a prefix
>> argument.
>
> To clarify the history of proced: it started out as a front-end for
> ps(1).  That was a mess because there are too many variants of ps(1) out
> in the wild to allow for anything robust.  So Eli kindly wrote the
> built-in functions list-system-processes and process-attributes that
> provide a well-defined base for proced.  These built-in functions are
> not implemented for all platforms and proced does not work for the other
> platforms; that's life.  I believe that again it would be a poor crutch
> if Emacs tried to run ps(1) remotely for a process listing.  Is it
> possible to use a remote instance of emacs to get a process listing on a
> remote machine (and let proced do nothing otherwise)?

I see. So if we don't want to emulate list-system-processes on a remote
host, we need another mean to determine, that a process is killed in a
sudo environment.

The proposal of Filipp (replace call-process by process-file in
proced.el) doesn't fly, because we cannot guarantee, that
default-directory is related to something like a "sudo remote
host". Even worse, it could be anything remote, and we would (try to)
kill a process with a local pid somewhere else ...

Best regards, Michael.

Re: process-file instead of call-process in proced.el?

[Michael Albinus]

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

Michael Albinus <michael.albinus@gmx.de> writes:

Hi,

> I see. So if we don't want to emulate list-system-processes on a remote
> host, we need another mean to determine, that a process is killed in a
> sudo environment.

I've prepared a first version of a patch (still missing documentation),
see appended. The changes are:

- `signal-process' is capable now to signal remote processes. This is
  triggered when either the PROCESS object has the property `remote-pid'
  (set by Tramp), or the PROCESS is just a number, and the additional
  optional parameter REMOTE is a remote file name.

- There is a new user option `proced-remote-directory', which defaults
  to "/sudo::". When `proced-send-signal' or `proced-renice' are invoked
  with a prefix arg ("C-u k" or "C-u r" in the *Proced* buffer), the
  respective signal is sent with the credentials as identified by this
  user option.

Comments?

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 11026 bytes --]

diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 0192a63a10..ff42a22765 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5957,6 +5957,43 @@ tramp-interrupt-process
  (lambda ()
    (remove-hook 'interrupt-process-functions #'tramp-interrupt-process)))

+(defun tramp-signal-process (process sigcode &optional remote)
+  "Send PROCESS the signal with code SIGCODE.
+PROCESS may also be a number specifying the process id of the
+process to signal; in this case, the process need not be a child of
+this Emacs.  Furthermore, if REMOTE is a remote file name, this
+number is interpreted as process id on the respective remote
+host, which will be the process to signal.
+SIGCODE may be an integer, or a symbol whose name is a signal name."
+  (let (pid vec)
+    (cond
+     ((processp process)
+      (setq pid (process-get process 'remote-pid)
+            vec (process-get process 'vector)))
+     ((numberp process)
+      (setq pid process
+            vec (and (stringp remote) (tramp-dissect-file-name remote))))
+     (t (signal 'wrong-type-argument (list #'processp process))))
+    (unless (or (numberp sigcode) (symbolp sigcode))
+      (signal 'wrong-type-argument (list #'numberp sigcode)))
+    ;; If it's a Tramp process, send SIGCODE remotely.
+    (when (and pid vec)
+      (tramp-message
+       vec 5 "Send signal %s to process %s with pid %s" sigcode process pid)
+      ;; This is for tramp-sh.el.  Other backends do not support this (yet).
+      (if (tramp-compat-funcall
+           'tramp-send-command-and-check
+           vec (format "\\kill -%s %d" sigcode pid))
+          0 -1))))
+
+;; `signal-process-functions' exists since Emacs 29.1.
+(when (boundp 'signal-process-functions)
+  (add-hook 'signal-process-functions #'tramp-signal-process)
+  (add-hook
+   'tramp-unload-hook
+   (lambda ()
+     (remove-hook 'signal-process-functions #'tramp-signal-process))))
+
 (defun tramp-get-remote-null-device (vec)
   "Return null device on the remote host identified by VEC.
 If VEC is `tramp-null-hop', return local null device."
diff --git a/lisp/proced.el b/lisp/proced.el
index c1d599afc4..7966ccfb08 100644
--- a/lisp/proced.el
+++ b/lisp/proced.el
@@ -29,10 +29,6 @@
 ;;
 ;; To do:
 ;; - Interactive temporary customizability of flags in `proced-grammar-alist'
-;; - Allow "sudo kill PID", "sudo renice PID"
-;;   `proced-send-signal' operates on multiple processes one by one.
-;;   With "sudo" we want to execute one "kill" or "renice" command
-;;   for all marked processes.  Is there a `sudo-call-process'?
 ;;
 ;; Thoughts and Ideas
 ;; - Currently, `process-attributes' returns the list of
@@ -61,6 +57,14 @@ proced-signal-function
 the external command (usually \"kill\")."
   :type '(choice (function :tag "function")
                  (string :tag "command")))
+(make-obsolete-variable 'proced-signal-function "no longer used." "29.1")
+
+(defcustom proced-remote-directory "/sudo::"
+  "Remote directory to be used when sending a signal.
+It must point to the local host, via a `sudo' or `doas' method,
+or alike.  See `proced-send-signal' and `proced-renice'."
+  :version "29.1"
+  :type '(string :tag "remote directory"))

 (defcustom proced-renice-command "renice"
   "Name of renice command."
@@ -626,6 +630,9 @@ proced-mode
 Type \\[proced] to start a Proced session.  In a Proced buffer
 type \\<proced-mode-map>\\[proced-mark] to mark a process for later commands.
 Type \\[proced-send-signal] to send signals to marked processes.
+Type \\[proced-renice] to renice marked processes.
+With a prefix argument \\[universal-argument], sending signals to and renicing of processes
+will be performed with the credentials of `proced-remote-directory'.

 The initial content of a listing is defined by the variable `proced-filter'
 and the variable `proced-format'.
@@ -1766,7 +1773,10 @@ proced-send-signal
 For backward compatibility SIGNAL and PROCESS-ALIST may be nil.
 Then PROCESS-ALIST contains the marked processes or the process point is on
 and SIGNAL is queried interactively.  This noninteractive usage is still
-supported but discouraged.  It will be removed in a future version of Emacs."
+supported but discouraged.  It will be removed in a future version of Emacs.
+
+With a prefix argument \\[universal-argument], send the signal with the credentials of
+`proced-remote-directory'."
   (interactive
    (let* ((process-alist (proced-marked-processes))
           (pnum (if (= 1 (length process-alist))
@@ -1808,7 +1818,10 @@ proced-send-signal
                                         proced-signal-list
                                         nil nil nil nil "TERM"))))))

-  (let (failures)
+  (let ((default-directory
+         (if (and current-prefix-arg (stringp proced-remote-directory))
+             proced-remote-directory temporary-file-directory))
+        failures)
     ;; Why not always use `signal-process'?  See
     ;; https://lists.gnu.org/r/emacs-devel/2008-03/msg02955.html
     (if (functionp proced-signal-function)
@@ -1821,7 +1834,8 @@ proced-send-signal
           (dolist (process process-alist)
             (condition-case err
                 (unless (zerop (funcall
-                                proced-signal-function (car process) signal))
+                                proced-signal-function (car process) signal
+                                (file-remote-p default-directory)))
                   (proced-log "%s\n" (cdr process))
                   (push (cdr process) failures))
               (error ; catch errors from failed signals
@@ -1833,7 +1847,7 @@ proced-send-signal
         (dolist (process process-alist)
           (with-temp-buffer
             (condition-case nil
-                (unless (zerop (call-process
+                (unless (zerop (process-file
                                 proced-signal-function nil t nil
                                 signal (number-to-string (car process))))
                   (proced-log (current-buffer))
@@ -1862,7 +1876,10 @@ proced-renice
 Interactively, PROCESS-ALIST contains the marked processes.
 If no process is marked, it contains the process point is on,
 After renicing all processes in PROCESS-ALIST, this command runs
-the normal hook `proced-after-send-signal-hook'."
+the normal hook `proced-after-send-signal-hook'.
+
+With a prefix argument \\[universal-argument], apply renice with the credentials of
+`proced-remote-directory'."
   (interactive
    (let ((process-alist (proced-marked-processes)))
      (proced-with-processes-buffer process-alist
@@ -1871,11 +1888,14 @@ proced-renice
    proced-mode)
   (if (numberp priority)
       (setq priority (number-to-string priority)))
-  (let (failures)
+  (let ((default-directory
+         (if (and current-prefix-arg (stringp proced-remote-directory))
+             proced-remote-directory temporary-file-directory))
+        failures)
     (dolist (process process-alist)
       (with-temp-buffer
         (condition-case nil
-            (unless (zerop (call-process
+            (unless (zerop (process-file
                             proced-renice-command nil t nil
                             priority (number-to-string (car process))))
               (proced-log (current-buffer))
diff --git a/src/process.c b/src/process.c
index 993e1c5603..fcff9c71ac 100644
--- a/src/process.c
+++ b/src/process.c
@@ -7034,14 +7034,13 @@ abbr_to_signal (char const *name)
   return -1;
 }

-DEFUN ("signal-process", Fsignal_process, Ssignal_process,
-       2, 2, "sProcess (name or number): \nnSignal code: ",
-       doc: /* Send PROCESS the signal with code SIGCODE.
-PROCESS may also be a number specifying the process id of the
-process to signal; in this case, the process need not be a child of
-this Emacs.
-SIGCODE may be an integer, or a symbol whose name is a signal name.  */)
-  (Lisp_Object process, Lisp_Object sigcode)
+DEFUN ("internal-default-signal-process",
+       Finternal_default_signal_process,
+       Sinternal_default_signal_process, 2, 3, 0,
+       doc: /* Default function to send PROCESS the signal with code SIGCODE.
+It shall be the last element in list `signal-process-functions'.
+See function `signal-process' for more details on usage.  */)
+  (Lisp_Object process, Lisp_Object sigcode, Lisp_Object remote)
 {
   pid_t pid;
   int signo;
@@ -7091,6 +7090,21 @@ DEFUN ("signal-process", Fsignal_process, Ssignal_process,
   return make_fixnum (kill (pid, signo));
 }

+DEFUN ("signal-process", Fsignal_process, Ssignal_process,
+       2, 3, "sProcess (name or number): \nnSignal code: ",
+       doc: /* Send PROCESS the signal with code SIGCODE.
+PROCESS may also be a number specifying the process id of the
+process to signal; in this case, the process need not be a child of
+this Emacs.  Furthermore, if REMOTE is a remote file name, this
+number is interpreted as process id on the respective remote
+host, which will be the process to signal.
+SIGCODE may be an integer, or a symbol whose name is a signal name.  */)
+  (Lisp_Object process, Lisp_Object sigcode, Lisp_Object remote)
+{
+  return CALLN (Frun_hook_with_args_until_success, Qsignal_process_functions,
+		process, sigcode, remote);
+}
+
 DEFUN ("process-send-eof", Fprocess_send_eof, Sprocess_send_eof, 0, 1, 0,
        doc: /* Make PROCESS see end-of-file in its input.
 EOF comes after any text already sent to it.
@@ -8580,6 +8594,13 @@ syms_of_process (void)
 returns non-nil.  */);
   Vinterrupt_process_functions = list1 (Qinternal_default_interrupt_process);

+  DEFVAR_LISP ("signal-process-functions", Vsignal_process_functions,
+	       doc: /* List of functions to be called for `signal-process'.
+The arguments of the functions are the same as for `signal-process'.
+These functions are called in the order of the list, until one of them
+returns non-nil.  */);
+  Vsignal_process_functions = list1 (Qinternal_default_signal_process);
+
   DEFVAR_LISP ("internal--daemon-sockname", Vinternal__daemon_sockname,
 	       doc: /* Name of external socket passed to Emacs, or nil if none.  */);
   Vinternal__daemon_sockname = Qnil;
@@ -8600,6 +8621,10 @@ syms_of_process (void)
 	  "internal-default-interrupt-process");
   DEFSYM (Qinterrupt_process_functions, "interrupt-process-functions");

+  DEFSYM (Qinternal_default_signal_process,
+	  "internal-default-signal-process");
+  DEFSYM (Qsignal_process_functions, "signal-process-functions");
+
   DEFSYM (Qnull, "null");
   DEFSYM (Qpipe_process_p, "pipe-process-p");

@@ -8654,6 +8679,7 @@ syms_of_process (void)
   defsubr (&Scontinue_process);
   defsubr (&Sprocess_running_child_p);
   defsubr (&Sprocess_send_eof);
+  defsubr (&Sinternal_default_signal_process);
   defsubr (&Ssignal_process);
   defsubr (&Swaiting_for_user_input_p);
   defsubr (&Sprocess_type);

Re: process-file instead of call-process in proced.el?

[Roland Winkler]

On Sun, Mar 27 2022, Michael Albinus wrote:
> Michael Albinus <michael.albinus@gmx.de> writes:
>> I see. So if we don't want to emulate list-system-processes on a remote
>> host, we need another mean to determine, that a process is killed in a
>> sudo environment.
>
> I've prepared a first version of a patch (still missing documentation),
> see appended. The changes are:
>
> - `signal-process' is capable now to signal remote processes. This is
>   triggered when either the PROCESS object has the property `remote-pid'
>   (set by Tramp), or the PROCESS is just a number, and the additional
>   optional parameter REMOTE is a remote file name.
>
> - There is a new user option `proced-remote-directory', which defaults
>   to "/sudo::". When `proced-send-signal' or `proced-renice' are invoked
>   with a prefix arg ("C-u k" or "C-u r" in the *Proced* buffer), the
>   respective signal is sent with the credentials as identified by this
>   user option.

I have no meaningful scenario to test the code myself.  Just from
looking at the patch, I am wondering:

How is it supposed to work that one signals remote processes if
list-system-processes is not available on a remote host?  Say, how
should one find out the PIDs?  How does this integrate into proced that
is otherwise only running locally?

Maybe my ignorance stems from not having any usage scenarios in the
first place.

Re: process-file instead of call-process in proced.el?

[Michael Albinus]

Roland Winkler <winkler@gnu.org> writes:

Hi Roland,

> How is it supposed to work that one signals remote processes if
> list-system-processes is not available on a remote host?  Say, how
> should one find out the PIDs?  How does this integrate into proced that
> is otherwise only running locally?

You have explained to me why proced is handling local processes only. So
I haven't changed this.

I have just added the possibility, to send a signal to a local process
with root permissions.

Best regards, Michael.

Re: process-file instead of call-process in proced.el?

[Roland Winkler]

On Mon, Mar 28 2022, Michael Albinus wrote:
> I have just added the possibility, to send a signal to a local process
> with root permissions.

Thanks, make sense!  I thought I must be missing something.

Re: process-file instead of call-process in proced.el?

[Filipp Gunbin]

Michael, thanks for working on this,

On 27/03/2022 16:08 +0200, Michael Albinus wrote:

> +      (if (tramp-compat-funcall
> +           'tramp-send-command-and-check
> +           vec (format "\\kill -%s %d" sigcode pid))
> +          0 -1))))
> +

Can the command be a connection-local variable?.  Then the ability to
customize, existing before deprecation of proced-signal-function, would
not be lost.

>        (setq priority (number-to-string priority)))
> -  (let (failures)
> +  (let ((default-directory
> +         (if (and current-prefix-arg (stringp proced-remote-directory))
> +             proced-remote-directory temporary-file-directory))
> +        failures)

As for me, I'd leave the proced-remote-directory thing out.  IIUC, it
allows to send signals to remote processes from "normal" proced buffer,
with local default-directory (with prefix arg).  I'd be fine with
requiring user to open proced from remote directory, with
default-directory already "prepared".

Thanks.
Filipp

Re: process-file instead of call-process in proced.el?

[Michael Albinus]

Filipp Gunbin <fgunbin@fastmail.fm> writes:

Hi Filipp,

>> +      (if (tramp-compat-funcall
>> +           'tramp-send-command-and-check
>> +           vec (format "\\kill -%s %d" sigcode pid))
>> +          0 -1))))
>> +
>
> Can the command be a connection-local variable?.  Then the ability to
> customize, existing before deprecation of proced-signal-function, would
> not be lost.

Would be possible. However, I fail to understand which command you have
in mind instead of "kill".

>> +  (let ((default-directory
>> +         (if (and current-prefix-arg (stringp proced-remote-directory))
>> +             proced-remote-directory temporary-file-directory))
>> +        failures)
>
> As for me, I'd leave the proced-remote-directory thing out.  IIUC, it
> allows to send signals to remote processes from "normal" proced buffer,
> with local default-directory (with prefix arg).  I'd be fine with
> requiring user to open proced from remote directory, with
> default-directory already "prepared".

What do you do if a user starts proced from a buffer with a remote
default directory to, let's say, "/ssh:otherhost:/path"?
proced-send-signal would try to send the signal to otherhost instead of
your local host.

And no, you cannot prevent this scenario. Users execute proced whenever
they like.

> Thanks.
> Filipp

Best regards, Michael.

Re: process-file instead of call-process in proced.el?

[Filipp Gunbin]

Hi,

On 28/03/2022 17:40 +0200, Michael Albinus wrote:

> Filipp Gunbin <fgunbin@fastmail.fm> writes:
>
> Hi Filipp,
>
>>> +      (if (tramp-compat-funcall
>>> +           'tramp-send-command-and-check
>>> +           vec (format "\\kill -%s %d" sigcode pid))
>>> +          0 -1))))
>>> +
>>
>> Can the command be a connection-local variable?.  Then the ability to
>> customize, existing before deprecation of proced-signal-function, would
>> not be lost.
>
> Would be possible. However, I fail to understand which command you have
> in mind instead of "kill".

Well, at least kill builtin in shell.  I don't know, I'd just prefer it
be customizable.  This is highly system-dependent, so it'd be more
future-proof.

>>> +  (let ((default-directory
>>> +         (if (and current-prefix-arg (stringp proced-remote-directory))
>>> +             proced-remote-directory temporary-file-directory))
>>> +        failures)
>>
>> As for me, I'd leave the proced-remote-directory thing out.  IIUC, it
>> allows to send signals to remote processes from "normal" proced buffer,
>> with local default-directory (with prefix arg).  I'd be fine with
>> requiring user to open proced from remote directory, with
>> default-directory already "prepared".
>
> What do you do if a user starts proced from a buffer with a remote
> default directory to, let's say, "/ssh:otherhost:/path"?
> proced-send-signal would try to send the signal to otherhost instead of
> your local host.
>
> And no, you cannot prevent this scenario. Users execute proced whenever
> they like.

Yeah, I see, the pid will be taken from localhost, but "kill" will be
sent to remote.  Can we for now just raise an error if we see that the
remote is not simple sudo/su/whatever?  Maybe later we'll be able to
implement "get remote process list".

Filipp

Re: process-file instead of call-process in proced.el?

[Michael Albinus]

Filipp Gunbin <fgunbin@fastmail.fm> writes:

> Hi,

Hi Filipp,

>> Would be possible. However, I fail to understand which command you have
>> in mind instead of "kill".
>
> Well, at least kill builtin in shell.  I don't know, I'd just prefer it
> be customizable.  This is highly system-dependent, so it'd be more
> future-proof.

Tramp sends already the "kill builtin in shell". Could we just leave it
as it is, and add connection-local behavior when requested by real use
cases? We're still in Emacs master, years from being released.

> Yeah, I see, the pid will be taken from localhost, but "kill" will be
> sent to remote.  Can we for now just raise an error if we see that the
> remote is not simple sudo/su/whatever?  Maybe later we'll be able to
> implement "get remote process list".

For now, we support only local pids, as Roland has explained. My patch
does this, w/o the need to raise an error in case of a pilot error.

If you are concerned about the prefix argument for "C-u k" and "C-u r"
I'm open to another mean for an indication to proced, that root
permissions are desired.

And if we decide to support also remote pids being target of a signal,
that would be another game. Nothing to prepare now, it would be a major
change anyway.

> Filipp

Best regards, Michael.

Re: process-file instead of call-process in proced.el?

[Filipp Gunbin]

Hi,

On 28/03/2022 18:39 +0200, Michael Albinus wrote:

> Filipp Gunbin <fgunbin@fastmail.fm> writes:
>
>> Hi,
>
> Hi Filipp,
>
>>> Would be possible. However, I fail to understand which command you have
>>> in mind instead of "kill".
>>
>> Well, at least kill builtin in shell.  I don't know, I'd just prefer it
>> be customizable.  This is highly system-dependent, so it'd be more
>> future-proof.
>
> Tramp sends already the "kill builtin in shell". Could we just leave it
> as it is, and add connection-local behavior when requested by real use
> cases? We're still in Emacs master, years from being released.

I think yes..

>> Yeah, I see, the pid will be taken from localhost, but "kill" will be
>> sent to remote.  Can we for now just raise an error if we see that the
>> remote is not simple sudo/su/whatever?  Maybe later we'll be able to
>> implement "get remote process list".
>
> For now, we support only local pids, as Roland has explained. My patch
> does this, w/o the need to raise an error in case of a pilot error.
>
> If you are concerned about the prefix argument for "C-u k" and "C-u r"
> I'm open to another mean for an indication to proced, that root
> permissions are desired.
>
> And if we decide to support also remote pids being target of a signal,
> that would be another game. Nothing to prepare now, it would be a major
> change anyway.

I was suggesting a way to get rid of proced-remote-directory, which
looks a bit obscure for me.  You noted that it's easy to get unexpected
behavior (local pid but remote signal), then I suggested a way to handle
it (raise an error).  If you think proced-remote-directory is better -
let it be, I'm perfectly fine with it.

Thanks.

Re: process-file instead of call-process in proced.el?

[Michael Albinus]

Filipp Gunbin <fgunbin@fastmail.fm> writes:

> Hi,

Hi,

>> For now, we support only local pids, as Roland has explained. My patch
>> does this, w/o the need to raise an error in case of a pilot error.
>>
>> If you are concerned about the prefix argument for "C-u k" and "C-u r"
>> I'm open to another mean for an indication to proced, that root
>> permissions are desired.
>>
>> And if we decide to support also remote pids being target of a signal,
>> that would be another game. Nothing to prepare now, it would be a major
>> change anyway.
>
> I was suggesting a way to get rid of proced-remote-directory, which
> looks a bit obscure for me.  You noted that it's easy to get unexpected
> behavior (local pid but remote signal), then I suggested a way to handle
> it (raise an error).  If you think proced-remote-directory is better -
> let it be, I'm perfectly fine with it.

No further complaints, so I've pushed the patch (plus documentation and
test) to master.

> Thanks.

Best regards, Michael.

Re: process-file instead of call-process in proced.el?

[Filipp Gunbin]

On 30/03/2022 13:20 +0200, Michael Albinus wrote:

> Filipp Gunbin <fgunbin@fastmail.fm> writes:
>
>> Hi,
>
> Hi,
>
>>> For now, we support only local pids, as Roland has explained. My patch
>>> does this, w/o the need to raise an error in case of a pilot error.
>>>
>>> If you are concerned about the prefix argument for "C-u k" and "C-u r"
>>> I'm open to another mean for an indication to proced, that root
>>> permissions are desired.
>>>
>>> And if we decide to support also remote pids being target of a signal,
>>> that would be another game. Nothing to prepare now, it would be a major
>>> change anyway.
>>
>> I was suggesting a way to get rid of proced-remote-directory, which
>> looks a bit obscure for me.  You noted that it's easy to get unexpected
>> behavior (local pid but remote signal), then I suggested a way to handle
>> it (raise an error).  If you think proced-remote-directory is better -
>> let it be, I'm perfectly fine with it.
>
> No further complaints, so I've pushed the patch (plus documentation and
> test) to master.
>

Tested from master, works ok, thanks!

Filipp

Re: process-file instead of call-process in proced.el?

[Jim Porter]

On 3/27/2022 7:08 AM, Michael Albinus wrote:
> I've prepared a first version of a patch (still missing documentation),
> see appended. The changes are:
> 
> - `signal-process' is capable now to signal remote processes. This is
>    triggered when either the PROCESS object has the property `remote-pid'
>    (set by Tramp), or the PROCESS is just a number, and the additional
>    optional parameter REMOTE is a remote file name.
...
> 
> Comments?

I don't have any opinion about the proced parts (since I don't use 
proced), but `signal-process' for remote processes will definitely be 
useful. That will eliminate the need for a workaround I added in 
Eshell's handling of SIGPIPE; when passing a SIGPIPE along to a remote 
process, we previously had to use `delete-process' as a reasonable 
facsimile (see commit 9df5e3080066341d78489f0b18eabeeccac76b0c).

- Jim