bug#19394: 25.0.50; mpc does not handle MPD_HOST=socket

bug#19394: 25.0.50; mpc does not handle MPD_HOST=socket

[Mark Oteiza]

Hi,

mpc.el treats MPD_HOST only as an address or hostname, when it can also
be a socket. So, doing ‘M-x mpc RET’ here gives

    /run/user/1000/mpd/socket/6600 Name or service not known

From the mpd manual:

    bind_to_address <ip address or hostname or any>
           This specifies which address mpd binds to and listens on.
           Multiple bind_to_address parameters may be specified.  The
           default is "any", which binds to all available addresses.

           You can set a port that is different from the global port
           setting, e.g.  "localhost:6602".  IPv6 addresses must be
           enclosed in square brackets if you want to configure a
           port, e.g. "[::1]:6602".

           To bind to a Unix domain socket, specify an absolute path
           or a path starting with a tilde (~).  For a system-wide
           MPD, we suggest the path "/var/run/mpd/socket".

‘bind_to_address’ is the mpd config option corresponding to MPD_HOST.

bug#19394: 25.0.50; mpc does not handle MPD_HOST=socket

[Mark Oteiza]

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

Mark Oteiza <mvoteiza@udel.edu> writes:

> mpc.el treats MPD_HOST only as an address or hostname, when it can also
> be a socket. So, doing ‘M-x mpc RET’ here gives
>
>     /run/user/1000/mpd/socket/6600 Name or service not known
>
>>From the mpd manual:
>
>     bind_to_address <ip address or hostname or any>
>            This specifies which address mpd binds to and listens on.
>            Multiple bind_to_address parameters may be specified.  The
>            default is "any", which binds to all available addresses.
>
>            You can set a port that is different from the global port
>            setting, e.g.  "localhost:6602".  IPv6 addresses must be
>            enclosed in square brackets if you want to configure a
>            port, e.g. "[::1]:6602".
>
>            To bind to a Unix domain socket, specify an absolute path
>            or a path starting with a tilde (~).  For a system-wide
>            MPD, we suggest the path "/var/run/mpd/socket".
>
> ‘bind_to_address’ is the mpd config option corresponding to MPD_HOST.

Here's a patch for handling the MPD_HOST being a socket.  I don't really
get the wording "or a path starting with a tilde (~)." because no mpd
clients that I have (mpc, ncmpcpp) actually do tilde expansion
themselves, so I ignored that case.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-mpc.el-mpc-proc-connect-attempt-socket-connection-if.patch --]
[-- Type: text/x-diff, Size: 1221 bytes --]

From 706b3c005aac0d3b606b4265c315bdd3bc6d1a5a Mon Sep 17 00:00:00 2001
From: Mark Oteiza <mvoteiza@udel.edu>
Date: Sat, 1 Aug 2015 00:46:02 -0400
Subject: [PATCH] mpc.el (mpc--proc-connect): attempt socket connection if host
 is an absolute path (Bug#19394)

---
 lisp/mpc.el | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lisp/mpc.el b/lisp/mpc.el
index 76c08db..4d04cf8 100644
--- a/lisp/mpc.el
+++ b/lisp/mpc.el
@@ -279,7 +279,10 @@ defaults to 6600 and HOST defaults to localhost."
       (let* ((coding-system-for-read 'utf-8-unix)
              (coding-system-for-write 'utf-8-unix)
              (proc (condition-case err
-                       (open-network-stream "MPC" (current-buffer) host port)
+                       (make-network-process :name "MPC" :buffer (current-buffer)
+                                             :host host :service port
+                                             :remote (if (string-prefix-p "/" host)
+                                                         host))
                      (error (user-error (error-message-string err))))))
         (when (processp mpc-proc)
           ;; Inherit the properties of the previous connection.
-- 
2.5.0

bug#19394: 25.0.50; mpc does not handle MPD_HOST=socket

[Eli Zaretskii]

> From: Mark Oteiza <mvoteiza@udel.edu>
> Date: Sun, 02 Aug 2015 14:36:37 -0400
> 
> Here's a patch for handling the MPD_HOST being a socket.

Thanks.

> +                                             :remote (if (string-prefix-p "/" host)
> +                                                         host))

This looks wrong to me, the proper test for a remote file name is
file-remote-p (if this is what you want to test here).

bug#19394: 25.0.50; mpc does not handle MPD_HOST=socket

[Mark Oteiza]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Mark Oteiza <mvoteiza@udel.edu>
>> Date: Sun, 02 Aug 2015 14:36:37 -0400
>> 
>> Here's a patch for handling the MPD_HOST being a socket.
>
> Thanks.
>
>> +                                             :remote (if (string-prefix-p "/" host)
>> +                                                         host))
>
> This looks wrong to me, the proper test for a remote file name is
> file-remote-p (if this is what you want to test here).

Yeah you're right, I was abusing this wording for :remote

  When specified for a client process, the FAMILY, HOST, and SERVICE
   args are ignored.

when I should have been using :family and :service together. This should
be better:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-mpc.el-mpc-proc-connect-support-UNIX-addresses-Bug-1.patch --]
[-- Type: text/x-diff, Size: 1817 bytes --]

From 83d7ef4cc27580248c6584d2f141afa50624c951 Mon Sep 17 00:00:00 2001
From: Mark Oteiza <mvoteiza@udel.edu>
Date: Sun, 2 Aug 2015 17:44:27 -0400
Subject: [PATCH] mpc.el (mpc--proc-connect): support UNIX addresses
 (Bug#19394)

---
 lisp/mpc.el | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lisp/mpc.el b/lisp/mpc.el
index 76c08db..396e067 100644
--- a/lisp/mpc.el
+++ b/lisp/mpc.el
@@ -253,6 +253,7 @@ defaults to 6600 and HOST defaults to localhost."
 
 (defun mpc--proc-connect (host)
   (let ((port 6600)
+        local
         pass)
 
     (when (string-match "\\`\\(?:\\(.*\\)@\\)?\\(.*?\\)\\(?::\\(.*\\)\\)?\\'"
@@ -267,6 +268,8 @@ defaults to 6600 and HOST defaults to localhost."
                 (if (string-match "[^[:digit:]]" v)
                     (string-to-number v)
                   v)))))
+    (when (string-prefix-p "/" host)
+      (setq local t))
 
     (mpc--debug "Connecting to %s:%s..." host port)
     (with-current-buffer (get-buffer-create (format " *mpc-%s:%s*" host port))
@@ -279,7 +282,10 @@ defaults to 6600 and HOST defaults to localhost."
       (let* ((coding-system-for-read 'utf-8-unix)
              (coding-system-for-write 'utf-8-unix)
              (proc (condition-case err
-                       (open-network-stream "MPC" (current-buffer) host port)
+                       (make-network-process :name "MPC" :buffer (current-buffer)
+                                             :host (unless local host)
+                                             :service (if local host port)
+                                             :family (if local 'local))
                      (error (user-error (error-message-string err))))))
         (when (processp mpc-proc)
           ;; Inherit the properties of the previous connection.
-- 
2.5.0

bug#19394: 25.0.50; mpc does not handle MPD_HOST=socket

[Mark Oteiza]

Mark Oteiza <mvoteiza@udel.edu> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Mark Oteiza <mvoteiza@udel.edu>
>>> Date: Sun, 02 Aug 2015 14:36:37 -0400
>>> 
>>> Here's a patch for handling the MPD_HOST being a socket.
>>
>> Thanks.
>>
>>> +                                             :remote (if (string-prefix-p "/" host)
>>> +                                                         host))
>>
>> This looks wrong to me, the proper test for a remote file name is
>> file-remote-p (if this is what you want to test here).
>
> Yeah you're right, I was abusing this wording for :remote
>
>   When specified for a client process, the FAMILY, HOST, and SERVICE
>    args are ignored.
>
> when I should have been using :family and :service together. This should
> be better:
>
>>From 83d7ef4cc27580248c6584d2f141afa50624c951 Mon Sep 17 00:00:00 2001
> From: Mark Oteiza <mvoteiza@udel.edu>
> Date: Sun, 2 Aug 2015 17:44:27 -0400
> Subject: [PATCH] mpc.el (mpc--proc-connect): support UNIX addresses
>  (Bug#19394)
>
> ---
>  lisp/mpc.el | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/mpc.el b/lisp/mpc.el
> index 76c08db..396e067 100644
> --- a/lisp/mpc.el
> +++ b/lisp/mpc.el
> @@ -253,6 +253,7 @@ defaults to 6600 and HOST defaults to localhost."
>  
>  (defun mpc--proc-connect (host)
>    (let ((port 6600)
> +        local
>          pass)
>  
>      (when (string-match "\\`\\(?:\\(.*\\)@\\)?\\(.*?\\)\\(?::\\(.*\\)\\)?\\'"
> @@ -267,6 +268,8 @@ defaults to 6600 and HOST defaults to localhost."
>                  (if (string-match "[^[:digit:]]" v)
>                      (string-to-number v)
>                    v)))))
> +    (when (string-prefix-p "/" host)
> +      (setq local t))
>  
>      (mpc--debug "Connecting to %s:%s..." host port)
>      (with-current-buffer (get-buffer-create (format " *mpc-%s:%s*" host port))
> @@ -279,7 +282,10 @@ defaults to 6600 and HOST defaults to localhost."
>        (let* ((coding-system-for-read 'utf-8-unix)
>               (coding-system-for-write 'utf-8-unix)
>               (proc (condition-case err
> -                       (open-network-stream "MPC" (current-buffer) host port)
> +                       (make-network-process :name "MPC" :buffer (current-buffer)
> +                                             :host (unless local host)
> +                                             :service (if local host port)
> +                                             :family (if local 'local))
>                       (error (user-error (error-message-string err))))))
>          (when (processp mpc-proc)
>            ;; Inherit the properties of the previous connection.

I've been rolling with this patch for a while and have tested with mpd
configured for socket and for port.  Could this please be applied?

bug#19394: 25.0.50; mpc does not handle MPD_HOST=socket

[Stefan Monnier]

> I've been rolling with this patch for a while and have tested with mpd
> configured for socket and for port.  Could this please be applied?

Done, thanks,


        Stefan