unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] Add file name handler support for 'make-process' (Bug#28691)
@ 2018-12-16 23:39 ` Philipp Stephani
  2018-12-17 17:34   ` bug#28691: " Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Philipp Stephani @ 2018-12-16 23:39 UTC (permalink / raw)
  To: emacs-devel, 28691; +Cc: Philipp Stephani

* src/process.c (Fmake_process): Add new keyword argument
':file-handler'.
(syms_of_process) <make-process, :file-handler>: Define new symbols.

* test/src/process-tests.el (make-process/file-handler): New unit
test.
---
 etc/NEWS                  |  4 ++++
 src/process.c             | 15 +++++++++++++++
 test/src/process-tests.el | 22 ++++++++++++++++++++++
 3 files changed, 41 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index c88f6ef5ca..0a5f915b33 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1407,6 +1407,10 @@ un-obsoleting it.
 +++
 ** New function 'group-name' returns a group name corresponding to GID.
 
+** 'make-process' now takes a keyword argument ':file-handler'; if
+that is non-nil, it will search for a file name handler for
+'default-directory'.
+
 \f
 * Changes in Emacs 27.1 on Non-Free Operating Systems
 
diff --git a/src/process.c b/src/process.c
index 8e0b2349f9..852431f421 100644
--- a/src/process.c
+++ b/src/process.c
@@ -1661,6 +1661,9 @@ to the standard error of subprocess.  Specifying this implies
 `:connection-type' is set to `pipe'.  If STDERR is nil, standard error
 is mixed with standard output and sent to BUFFER or FILTER.
 
+:file-handler FILE-HANDLER -- If FILE-HANDLER is non-nil, then search
+for a file name handler for `default-directory'.
+
 usage: (make-process &rest ARGS)  */)
   (ptrdiff_t nargs, Lisp_Object *args)
 {
@@ -1674,6 +1677,15 @@ usage: (make-process &rest ARGS)  */)
   /* Save arguments for process-contact and clone-process.  */
   contact = Flist (nargs, args);
 
+  if (!NILP (Fplist_get (contact, QCfile_handler)))
+    {
+      Lisp_Object file_handler
+        = Ffind_file_name_handler (BVAR (current_buffer, directory),
+                                   Qmake_process);
+      if (!NILP (file_handler))
+        return CALLN (Fapply, file_handler, Qmake_process, contact);
+    }
+
   buffer = Fplist_get (contact, QCbuffer);
   if (!NILP (buffer))
     buffer = Fget_buffer_create (buffer);
@@ -8098,6 +8110,8 @@ init_process_emacs (int sockfd)
 void
 syms_of_process (void)
 {
+  DEFSYM (Qmake_process, "make-process");
+
 #ifdef subprocesses
 
   DEFSYM (Qprocessp, "processp");
@@ -8138,6 +8152,7 @@ syms_of_process (void)
   DEFSYM (Qreal, "real");
   DEFSYM (Qnetwork, "network");
   DEFSYM (Qserial, "serial");
+  DEFSYM (QCfile_handler, ":file-handler");
   DEFSYM (QCbuffer, ":buffer");
   DEFSYM (QChost, ":host");
   DEFSYM (QCservice, ":service");
diff --git a/test/src/process-tests.el b/test/src/process-tests.el
index 551b34ff37..3e72e9210d 100644
--- a/test/src/process-tests.el
+++ b/test/src/process-tests.el
@@ -215,5 +215,27 @@ process-tests--mixable
                                       (string-to-list "stdout\n")
                                       (string-to-list "stderr\n"))))))
 
+(ert-deftest make-process/file-handler ()
+  "Check that the ‘:file-handler’ argument of ‘make-process’
+works as expected."
+  (let ((file-handler-calls 0))
+    (cl-flet ((file-handler
+               (&rest args)
+               (should (equal default-directory "test-handler:/dir/"))
+               (should (equal args '(make-process :name "name"
+                                                  :command ("/bin/true")
+                                                  :file-handler t)))
+               (cl-incf file-handler-calls)
+               'fake-process))
+      (let ((file-name-handler-alist
+             (cons (cons (rx bos "test-handler:") #'file-handler)
+                   file-name-handler-alist))
+            (default-directory "test-handler:/dir/"))
+        (should (eq (make-process :name "name"
+                                  :command '("/bin/true")
+                                  :file-handler t)
+                    'fake-process))
+        (should (= file-handler-calls 1))))))
+
 (provide 'process-tests)
 ;; process-tests.el ends here.
-- 
2.20.0.405.gbc1bbc6f85-goog




^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
       [not found] ` <20181216233936.208568-1-phst__21526.1563113474$1545003551$gmane$org@google.com>
@ 2018-12-17 12:38   ` Michael Albinus
  2018-12-17 17:35     ` Eli Zaretskii
  2018-12-17 19:07     ` Philipp Stephani
  0 siblings, 2 replies; 13+ messages in thread
From: Michael Albinus @ 2018-12-17 12:38 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: Philipp Stephani, 28691, emacs-devel

Philipp Stephani <p.stephani2@gmail.com> writes:

Hi Philipp,

> +:file-handler FILE-HANDLER -- If FILE-HANDLER is non-nil, then search
> +for a file name handler for `default-directory'.

What happens, if no file name handler is found? Should there be a local
process then, or should this be ignored (returning nil)?

> +(ert-deftest make-process/file-handler ()
> +  "Check that the ‘:file-handler’ argument of ‘make-process’
> +works as expected."
> +  (let ((file-handler-calls 0))
> +    (cl-flet ((file-handler
> +               (&rest args)
> +               (should (equal default-directory "test-handler:/dir/"))
> +               (should (equal args '(make-process :name "name"
> +                                                  :command ("/bin/true")
> +                                                  :file-handler t)))
> +               (cl-incf file-handler-calls)
> +               'fake-process))
> +      (let ((file-name-handler-alist
> +             (cons (cons (rx bos "test-handler:") #'file-handler)
> +                   file-name-handler-alist))
> +            (default-directory "test-handler:/dir/"))
> +        (should (eq (make-process :name "name"
> +                                  :command '("/bin/true")
> +                                  :file-handler t)
> +                    'fake-process))
> +        (should (= file-handler-calls 1))))))

I would make a second test, that calling `make-process' w/o the
`:file-handler' argument returns the plain process #<process name>.

The third test is for using non-nil `:file-handler', w/o finding one.
This returns either a local process, or nil (see remark above).

I also miss documentation in the Elisp manual, nodes "Magic File Names"
and "Asynchronous Processes".

And of course, the implementation of a file name handler is missing in
tramp-adb.el, tramp-sh.el and tramp-smb.el.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
  2018-12-16 23:39 ` [PATCH] Add file name handler support for 'make-process' (Bug#28691) Philipp Stephani
@ 2018-12-17 17:34   ` Eli Zaretskii
  2018-12-17 17:54     ` Stefan Monnier
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2018-12-17 17:34 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: phst, 28691, emacs-devel

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 17 Dec 2018 00:39:36 +0100
> Cc: Philipp Stephani <phst@google.com>
> 
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -1407,6 +1407,10 @@ un-obsoleting it.
>  +++
>  ** New function 'group-name' returns a group name corresponding to GID.
>  
> +** 'make-process' now takes a keyword argument ':file-handler'; if
> +that is non-nil, it will search for a file name handler for
> +'default-directory'.

I think the feature you introducing is not to "search for a file name
handler for 'default-directory'", the feature is to (attempt to) make
a remote process using that file handler.  I think the documentation
should make that more explicit, because I very much doubt it could be
otherwise gleaned from this description.

I'd also suggest to say "... the current buffer's
'default-directory'", just to make sure people don't miss that.

> +:file-handler FILE-HANDLER -- If FILE-HANDLER is non-nil, then search
> +for a file name handler for `default-directory'.

Likewise here.  Also, please describe what happens if the search for
the handler fails.

> +(ert-deftest make-process/file-handler ()
> +  "Check that the ‘:file-handler’ argument of ‘make-process’
> +works as expected."
> +  (let ((file-handler-calls 0))
> +    (cl-flet ((file-handler
> +               (&rest args)
> +               (should (equal default-directory "test-handler:/dir/"))
> +               (should (equal args '(make-process :name "name"
> +                                                  :command ("/bin/true")

/bin/true is not portable enough, I suggest to use Emacs itself
instead.

> +      (let ((file-name-handler-alist
> +             (cons (cons (rx bos "test-handler:") #'file-handler)
> +                   file-name-handler-alist))

I think it would be preferable to empty file-name-handler-alist of any
other members -- that will make this test 100% deterministic.

Thanks.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
  2018-12-17 12:38   ` Michael Albinus
@ 2018-12-17 17:35     ` Eli Zaretskii
  2018-12-17 19:30       ` Michael Albinus
  2018-12-17 19:07     ` Philipp Stephani
  1 sibling, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2018-12-17 17:35 UTC (permalink / raw)
  To: Michael Albinus; +Cc: phst, 28691, p.stephani2, emacs-devel

> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Mon, 17 Dec 2018 13:38:33 +0100
> Cc: Philipp Stephani <phst@google.com>, 28691@debbugs.gnu.org,
> 	emacs-devel@gnu.org
> 
> Philipp Stephani <p.stephani2@gmail.com> writes:
> 
> Hi Philipp,
> 
> > +:file-handler FILE-HANDLER -- If FILE-HANDLER is non-nil, then search
> > +for a file name handler for `default-directory'.
> 
> What happens, if no file name handler is found? Should there be a local
> process then, or should this be ignored (returning nil)?

The proposed code runs the process locally, which I think is
reasonable.

> I also miss documentation in the Elisp manual, nodes "Magic File Names"
> and "Asynchronous Processes".

Right.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
  2018-12-17 17:34   ` bug#28691: " Eli Zaretskii
@ 2018-12-17 17:54     ` Stefan Monnier
  2018-12-17 19:49       ` Michael Albinus
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2018-12-17 17:54 UTC (permalink / raw)
  To: emacs-devel

> I think the feature you introducing is not to "search for a file name
> handler for 'default-directory'", the feature is to (attempt to) make
> a remote process using that file handler.

"Remote" is just one possibility.  It can also be local under
a different UID, or it could be local in a virtual machine, ...

The way I think of it is that this argument to `make-process` tells
Emacs that this `make-process` call should be considered a file
operation, in the sense that the place where the process is run will be
chosen according to default-directory and file-name-handler-alist.


        Stefan




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
  2018-12-17 12:38   ` Michael Albinus
  2018-12-17 17:35     ` Eli Zaretskii
@ 2018-12-17 19:07     ` Philipp Stephani
  2018-12-17 19:20       ` Philipp Stephani
                         ` (2 more replies)
  1 sibling, 3 replies; 13+ messages in thread
From: Philipp Stephani @ 2018-12-17 19:07 UTC (permalink / raw)
  To: Michael Albinus; +Cc: Philipp Stephani, 28691, Emacs developers

Am Mo., 17. Dez. 2018 um 13:38 Uhr schrieb Michael Albinus
<michael.albinus@gmx.de>:
>
> Philipp Stephani <p.stephani2@gmail.com> writes:
>
> Hi Philipp,
>
> > +:file-handler FILE-HANDLER -- If FILE-HANDLER is non-nil, then search
> > +for a file name handler for `default-directory'.
>
> What happens, if no file name handler is found? Should there be a local
> process then, or should this be ignored (returning nil)?

I think it should be a new process, like start-file-process does.
(make-process should always either return a process object or fail.)

>
> > +(ert-deftest make-process/file-handler ()
> > +  "Check that the ‘:file-handler’ argument of ‘make-process’
> > +works as expected."
> > +  (let ((file-handler-calls 0))
> > +    (cl-flet ((file-handler
> > +               (&rest args)
> > +               (should (equal default-directory "test-handler:/dir/"))
> > +               (should (equal args '(make-process :name "name"
> > +                                                  :command ("/bin/true")
> > +                                                  :file-handler t)))
> > +               (cl-incf file-handler-calls)
> > +               'fake-process))
> > +      (let ((file-name-handler-alist
> > +             (cons (cons (rx bos "test-handler:") #'file-handler)
> > +                   file-name-handler-alist))
> > +            (default-directory "test-handler:/dir/"))
> > +        (should (eq (make-process :name "name"
> > +                                  :command '("/bin/true")
> > +                                  :file-handler t)
> > +                    'fake-process))
> > +        (should (= file-handler-calls 1))))))
>
> I would make a second test, that calling `make-process' w/o the
> `:file-handler' argument returns the plain process #<process name>.
>
> The third test is for using non-nil `:file-handler', w/o finding one.
> This returns either a local process, or nil (see remark above).
>
> I also miss documentation in the Elisp manual, nodes "Magic File Names"
> and "Asynchronous Processes".

Good points, I'll incorporate them in a follow-up patch.

>
> And of course, the implementation of a file name handler is missing in
> tramp-adb.el, tramp-sh.el and tramp-smb.el.

That should be part of a different bug. (Since make-process is more
capable than start-file-process, implementing Tramp support will be a
bit more difficult.)



^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
  2018-12-17 19:07     ` Philipp Stephani
@ 2018-12-17 19:20       ` Philipp Stephani
  2018-12-17 19:33       ` Michael Albinus
  2018-12-17 20:03       ` Drew Adams
  2 siblings, 0 replies; 13+ messages in thread
From: Philipp Stephani @ 2018-12-17 19:20 UTC (permalink / raw)
  To: emacs-devel, Michael Albinus, 28691; +Cc: Philipp Stephani

* src/process.c (Fmake_process): Add new keyword argument
':file-handler'.
(syms_of_process) <make-process, :file-handler>: Define new symbols.

* lisp/files.el (file-name-non-special): Add support for
'make-process'.

* test/src/process-tests.el (make-process/file-handler/found)
(make-process/file-handler/not-found)
(make-process/file-handler/disable): New unit tests.
(process-tests--file-handler): New helper function.

* test/lisp/files-tests.el
(files-tests-file-name-non-special-make-process): New unit test.

* doc/lispref/files.texi (Magic File Names): Document that
'make-process' can invoke file name handlers.

* doc/lispref/processes.texi (Asynchronous Processes): Document
':file-handlers' argument to 'make-process'.
---
 doc/lispref/files.texi     |  2 ++
 doc/lispref/processes.texi |  9 ++++++--
 etc/NEWS                   |  5 ++++
 lisp/files.el              | 11 +++++++--
 src/process.c              | 16 +++++++++++++
 test/lisp/files-tests.el   |  9 ++++++++
 test/src/process-tests.el  | 47 ++++++++++++++++++++++++++++++++++++++
 7 files changed, 95 insertions(+), 4 deletions(-)

diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index b795864815..00e7fc1841 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -3171,6 +3171,7 @@ Magic File Names
 @code{make-directory},
 @code{make-directory-internal},
 @code{make-nearby-temp-file},
+@code{make-process},
 @code{make-symbolic-link},@*
 @code{process-file},
 @code{rename-file}, @code{set-file-acl}, @code{set-file-modes},
@@ -3227,6 +3228,7 @@ Magic File Names
 @code{make-auto-save-file-name},
 @code{make-direc@discretionary{}{}{}tory},
 @code{make-direc@discretionary{}{}{}tory-internal},
+@code{make-process},
 @code{make-symbolic-link},
 @code{process-file},
 @code{rename-file}, @code{set-file-acl}, @code{set-file-modes},
diff --git a/doc/lispref/processes.texi b/doc/lispref/processes.texi
index d88c7fbe62..7ca0ac475f 100644
--- a/doc/lispref/processes.texi
+++ b/doc/lispref/processes.texi
@@ -696,6 +696,11 @@ Asynchronous Processes
 created with @code{make-pipe-process}, described below.  If
 @var{stderr} is @code{nil}, standard error is mixed with standard
 output, and both are sent to @var{buffer} or @var{filter}.
+
+@item :file-handler @var{file-handler}
+If @var{file-handler} is non-@code{nil}, then look for a file name
+handler for the current buffer's @code{default-directory}.  If there
+is no such handler, proceed as if @var{file-handler} were nil.
 @end table
 
 The original argument list, modified with the actual connection
@@ -704,8 +709,8 @@ Asynchronous Processes
 The current working directory of the subprocess is set to the current
 buffer's value of @code{default-directory} if that is local (as
 determined by `unhandled-file-name-directory'), or "~" otherwise.  If
-you want to run a process in a remote directory use
-@code{start-file-process}.
+you want to run a process in a remote directory, pass
+@code{:file-handler t} to @code{make-process}.
 @end defun
 
 @defun make-pipe-process &rest args
diff --git a/etc/NEWS b/etc/NEWS
index c88f6ef5ca..942b6501a8 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1407,6 +1407,11 @@ un-obsoleting it.
 +++
 ** New function 'group-name' returns a group name corresponding to GID.
 
+** 'make-process' now takes a keyword argument ':file-handler'; if
+that is non-nil, it will look for a file name handler for the current
+buffer's 'default-directory'.  That way 'make-process' can start
+remote processes.
+
 \f
 * Changes in Emacs 27.1 on Non-Free Operating Systems
 
diff --git a/lisp/files.el b/lisp/files.el
index fb6cf0193a..448df62710 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -7103,7 +7103,8 @@ file-name-non-special
         (default-directory
 	  (if (memq operation
                     '(insert-directory process-file start-file-process
-                                       shell-command temporary-file-directory))
+                                       make-process shell-command
+                                       temporary-file-directory))
 	      (directory-file-name
 	       (expand-file-name
 		(unhandled-file-name-directory default-directory)))
@@ -7151,7 +7152,13 @@ file-name-non-special
                           ;; These file-notify-* operations take a
                           ;; descriptor.
                           (file-notify-rm-watch)
-                          (file-notify-valid-p)))
+                          (file-notify-valid-p)
+                          ;; `make-process' uses keyword arguments and
+                          ;; doesn't mangle its filenames in any way.
+                          ;; It already strips /: from the binary
+                          ;; filename, so we don't have to do this
+                          ;; here.
+                          (make-process)))
 		  ;; For all other operations, treat the first
 		  ;; argument only as the file name.
 		  '(nil 0))))
diff --git a/src/process.c b/src/process.c
index 8e0b2349f9..80c700a09b 100644
--- a/src/process.c
+++ b/src/process.c
@@ -1661,6 +1661,10 @@ to the standard error of subprocess.  Specifying this implies
 `:connection-type' is set to `pipe'.  If STDERR is nil, standard error
 is mixed with standard output and sent to BUFFER or FILTER.
 
+:file-handler FILE-HANDLER -- If FILE-HANDLER is non-nil, then look
+for a file name handler for the current buffer's `default-directory'.
+If there is no such handler, proceed as if FILE-HANDLER were nil.
+
 usage: (make-process &rest ARGS)  */)
   (ptrdiff_t nargs, Lisp_Object *args)
 {
@@ -1674,6 +1678,15 @@ usage: (make-process &rest ARGS)  */)
   /* Save arguments for process-contact and clone-process.  */
   contact = Flist (nargs, args);
 
+  if (!NILP (Fplist_get (contact, QCfile_handler)))
+    {
+      Lisp_Object file_handler
+        = Ffind_file_name_handler (BVAR (current_buffer, directory),
+                                   Qmake_process);
+      if (!NILP (file_handler))
+        return CALLN (Fapply, file_handler, Qmake_process, contact);
+    }
+
   buffer = Fplist_get (contact, QCbuffer);
   if (!NILP (buffer))
     buffer = Fget_buffer_create (buffer);
@@ -8098,6 +8111,8 @@ init_process_emacs (int sockfd)
 void
 syms_of_process (void)
 {
+  DEFSYM (Qmake_process, "make-process");
+
 #ifdef subprocesses
 
   DEFSYM (Qprocessp, "processp");
@@ -8138,6 +8153,7 @@ syms_of_process (void)
   DEFSYM (Qreal, "real");
   DEFSYM (Qnetwork, "network");
   DEFSYM (Qserial, "serial");
+  DEFSYM (QCfile_handler, ":file-handler");
   DEFSYM (QCbuffer, ":buffer");
   DEFSYM (QChost, ":host");
   DEFSYM (QCservice, ":service");
diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index 3b192ee872..708d6d4659 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -1109,6 +1109,15 @@ files-tests-file-attributes-equal
     (with-temp-buffer
       (write-region nil nil nospecial nil :visit))))
 
+(ert-deftest files-tests-file-name-non-special-make-process ()
+  "Check that the ‘:file-handler’ argument of ‘make-process’
+works as expected if the default directory is quoted."
+  (skip-unless (file-accessible-directory-p "/:/bin/"))
+  (let ((default-directory "/:/bin/"))
+    (should (processp (make-process :name "name"
+                                    :command '("/:/bin/true")
+                                    :file-handler t)))))
+
 (ert-deftest files-tests--insert-directory-wildcard-in-dir-p ()
   (let ((alist (list (cons "/home/user/*/.txt" (cons "/home/user/" "*/.txt"))
                      (cons "/home/user/.txt" nil)
diff --git a/test/src/process-tests.el b/test/src/process-tests.el
index 551b34ff37..3587e3bfd8 100644
--- a/test/src/process-tests.el
+++ b/test/src/process-tests.el
@@ -215,5 +215,52 @@ process-tests--mixable
                                       (string-to-list "stdout\n")
                                       (string-to-list "stderr\n"))))))
 
+(ert-deftest make-process/file-handler/found ()
+  "Check that the ‘:file-handler’ argument of ‘make-process’
+works as expected if a file handler is found."
+  (let ((file-handler-calls 0))
+    (cl-flet ((file-handler
+               (&rest args)
+               (should (equal default-directory "test-handler:/dir/"))
+               (should (equal args '(make-process :name "name"
+                                                  :command ("/bin/true")
+                                                  :file-handler t)))
+               (cl-incf file-handler-calls)
+               'fake-process))
+      (let ((file-name-handler-alist (list (cons (rx bos "test-handler:")
+                                                 #'file-handler)))
+            (default-directory "test-handler:/dir/"))
+        (should (eq (make-process :name "name"
+                                  :command '("/bin/true")
+                                  :file-handler t)
+                    'fake-process))
+        (should (= file-handler-calls 1))))))
+
+(ert-deftest make-process/file-handler/not-found ()
+  "Check that the ‘:file-handler’ argument of ‘make-process’
+works as expected if no file handler is found."
+  (skip-unless (file-accessible-directory-p "/bin/"))
+  (let ((default-directory "/bin/"))
+    (should (processp (make-process :name "name"
+                                    :command '("/bin/true")
+                                    :file-handler t)))))
+
+(ert-deftest make-process/file-handler/disable ()
+  "Check ‘make-process’ works as expected if it shouldn’t use the
+file handler."
+  (let ((file-name-handler-alist (list (cons (rx bos "test-handler:")
+                                             #'process-tests--file-handler)))
+        (default-directory "test-handler:/dir/"))
+    (should (processp (make-process :name "name"
+                                    :command '("/bin/true"))))))
+
+(defun process-tests--file-handler (operation &rest _args)
+  (cl-ecase operation
+    (unhandled-file-name-directory "/")
+    (make-process (ert-fail "file handler called unexpectedly"))))
+
+(put #'process-tests--file-handler 'operations
+     '(unhandled-file-name-directory make-process))
+
 (provide 'process-tests)
 ;; process-tests.el ends here.
-- 
2.20.0.405.gbc1bbc6f85-goog






^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
  2018-12-17 17:35     ` Eli Zaretskii
@ 2018-12-17 19:30       ` Michael Albinus
  0 siblings, 0 replies; 13+ messages in thread
From: Michael Albinus @ 2018-12-17 19:30 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: phst, 28691, p.stephani2, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

>> What happens, if no file name handler is found? Should there be a local
>> process then, or should this be ignored (returning nil)?
>
> The proposed code runs the process locally, which I think is
> reasonable.

Yes, when default-directory is local.

If default-directory is remote, and :file-handler indicates that a
remote process shall run, the situation is different. If the
corresponding file name handler offers an own make-network-process
implementation, it returns that process. But if the file name handler
misses an implementation, no local process shall run.

This is the same situation as with start-file-process. The file name
handler for ssh returns a process object:

--8<---------------cut here---------------start------------->8---
(with-temp-buffer
  (let ((default-directory "/ssh::"))
    (start-file-process "foo" (current-buffer) "/bin/true")))
=> #<process foo>
--8<---------------cut here---------------end--------------->8---

But for Tramp methods which do not own a start-file-process
implementation, no process is created:

--8<---------------cut here---------------start------------->8---
(with-temp-buffer
  (let ((default-directory "/sftp::"))
    (start-file-process "foo" (current-buffer) "/bin/true")))
=> nil
--8<---------------cut here---------------end--------------->8---

This is documented in the Lisp Manual, see node "Asynchronous Processes":

--8<---------------cut here---------------start------------->8---
     Some file handlers may not support ‘start-file-process’ (for
     example the function ‘ange-ftp-hook-function’).  In such cases,
     this function does nothing and returns ‘nil’.
--8<---------------cut here---------------end--------------->8---

I believe make-network-process shall be have similar.

Best regards, Michael.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
  2018-12-17 19:07     ` Philipp Stephani
  2018-12-17 19:20       ` Philipp Stephani
@ 2018-12-17 19:33       ` Michael Albinus
  2018-12-17 20:03       ` Drew Adams
  2 siblings, 0 replies; 13+ messages in thread
From: Michael Albinus @ 2018-12-17 19:33 UTC (permalink / raw)
  To: Philipp Stephani; +Cc: Philipp Stephani, 28691, Emacs developers

Philipp Stephani <p.stephani2@gmail.com> writes:

Hi Philipp,

>> What happens, if no file name handler is found? Should there be a local
>> process then, or should this be ignored (returning nil)?
>
> I think it should be a new process, like start-file-process does.
> (make-process should always either return a process object or fail.)

Why fail? See my answer to Eli, I prefer that it returns nil if it isn't
possible to get a new process from the file name handler.

>> And of course, the implementation of a file name handler is missing in
>> tramp-adb.el, tramp-sh.el and tramp-smb.el.
>
> That should be part of a different bug. (Since make-process is more
> capable than start-file-process, implementing Tramp support will be a
> bit more difficult.)

No new bug needed. I have the feeling it's me who has to implement it in
Tramp. Or do you know a volunteer? I would appreciate it :-)

Best regards, Michael.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
  2018-12-17 17:54     ` Stefan Monnier
@ 2018-12-17 19:49       ` Michael Albinus
  2018-12-17 19:57         ` Stefan Monnier
  0 siblings, 1 reply; 13+ messages in thread
From: Michael Albinus @ 2018-12-17 19:49 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

Hi Stefan,

> "Remote" is just one possibility.  It can also be local under
> a different UID, or it could be local in a virtual machine, ...

The current implementation regards both cases also as "remote", provided
by Tramp. file-remote-p returns non-nil.

It might be unfortune, but I don't recommend to change the function's name.

> The way I think of it is that this argument to `make-process` tells
> Emacs that this `make-process` call should be considered a file
> operation, in the sense that the place where the process is run will be
> chosen according to default-directory and file-name-handler-alist.

Yes.

>         Stefan

Best regards, Michael.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
  2018-12-17 19:49       ` Michael Albinus
@ 2018-12-17 19:57         ` Stefan Monnier
  2018-12-17 20:01           ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2018-12-17 19:57 UTC (permalink / raw)
  To: Michael Albinus; +Cc: emacs-devel

>> "Remote" is just one possibility.  It can also be local under
>> a different UID, or it could be local in a virtual machine, ...
> The current implementation regards both cases also as "remote", provided
> by Tramp. file-remote-p returns non-nil.
> It might be unfortune, but I don't recommend to change the function's name.

I'm not suggesting to rename that function, but I think we should try and
keep this as an exception rather than start using the word "remote"
always meaning "as defined by file-remote-p" rather than the more
common meaning.


        Stefan



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
  2018-12-17 19:57         ` Stefan Monnier
@ 2018-12-17 20:01           ` Eli Zaretskii
  0 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2018-12-17 20:01 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: michael.albinus, emacs-devel

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Date: Mon, 17 Dec 2018 14:57:04 -0500
> Cc: emacs-devel@gnu.org
> 
> >> "Remote" is just one possibility.  It can also be local under
> >> a different UID, or it could be local in a virtual machine, ...
> > The current implementation regards both cases also as "remote", provided
> > by Tramp. file-remote-p returns non-nil.
> > It might be unfortune, but I don't recommend to change the function's name.
> 
> I'm not suggesting to rename that function, but I think we should try and
> keep this as an exception rather than start using the word "remote"
> always meaning "as defined by file-remote-p" rather than the more
> common meaning.

I think you can trust me to understand all that.  I used "remote" to
make my point more clear, expressed with simpler wording, that's all.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: bug#28691: [PATCH] Add file name handler support for 'make-process' (Bug#28691)
  2018-12-17 19:07     ` Philipp Stephani
  2018-12-17 19:20       ` Philipp Stephani
  2018-12-17 19:33       ` Michael Albinus
@ 2018-12-17 20:03       ` Drew Adams
  2 siblings, 0 replies; 13+ messages in thread
From: Drew Adams @ 2018-12-17 20:03 UTC (permalink / raw)
  To: Philipp Stephani, Michael Albinus
  Cc: Philipp Stephani, 28691, Emacs developers

Could these mails please be sent to only the bugs list or only emacs-devel, please?



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-12-17 20:03 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <jwvy3os889s.fsf@iro.umontreal.ca>
2018-12-16 23:39 ` [PATCH] Add file name handler support for 'make-process' (Bug#28691) Philipp Stephani
2018-12-17 17:34   ` bug#28691: " Eli Zaretskii
2018-12-17 17:54     ` Stefan Monnier
2018-12-17 19:49       ` Michael Albinus
2018-12-17 19:57         ` Stefan Monnier
2018-12-17 20:01           ` Eli Zaretskii
     [not found] ` <20181216233936.208568-1-phst__21526.1563113474$1545003551$gmane$org@google.com>
2018-12-17 12:38   ` Michael Albinus
2018-12-17 17:35     ` Eli Zaretskii
2018-12-17 19:30       ` Michael Albinus
2018-12-17 19:07     ` Philipp Stephani
2018-12-17 19:20       ` Philipp Stephani
2018-12-17 19:33       ` Michael Albinus
2018-12-17 20:03       ` Drew Adams

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).