* bug#49283: [PATCH] 27.2; `(call-process "program" null-device ...)' fails over TRAMP from local MS Windows @ 2021-06-30 5:14 Jim Porter 2021-06-30 7:24 ` Michael Albinus 0 siblings, 1 reply; 12+ messages in thread From: Jim Porter @ 2021-06-30 5:14 UTC (permalink / raw) To: 49283 [-- Attachment #1: Type: text/plain, Size: 2239 bytes --] I noticed an issue when trying to use flyspell (or ispell) using hunspell from a local MS Windows system on a TRAMP file. It results in an error that it can't find the file "/sshx:server:/path/to/NUL". I narrowed this down to the fact that `ispell-find-hunspell-dictionaries' calls `call-process' with `infile' set to `null-device'. To see this in action: emacs -Q C-x C-f /sshx:server:~/path/to/file.txt M-: (setq ispell-program-name "hunspell") RET M-x flyspell-mode ;; or... M-: (call-process "something" null-device) RET This results in the following error: ---------------------------------------- Debugger entered--Lisp error: (file-missing "Opening process input file" #("No such file or directory" 0 25 (charset windows-1252)) "/sshx:server:/path/to/NUL") call-process("something" "NUL") eval((call-process "something" null-device) t) eval-expression((call-process "something" null-device) nil nil 127) funcall-interactively(eval-expression (call-process "something" null-device) nil nil 127) call-interactively(eval-expression nil nil) command-execute(eval-expression) ---------------------------------------- It seems this is a result of the fact that `null-device' on MS Windows is "NUL", and `(expand-file-name "NUL")' is "<default-directory>/NUL". When `default-directory' is a local MS Windows path, this is ok, but when it's a TRAMP path, it looks for a real file named NUL on the remote (GNU/Linux) machine. However, since `call-process' executes from the (local) home directory if `default-directory' is a TRAMP path, I think it makes more sense for `infile' to be interpreted relative to the homedir too. I've attached a speculative patch that I think fixes this. (Note: I don't have an MS Windows build environment set up at the moment, so I only tested that this works like I'd expect from GNU/Linux. It'd probably be good to make sure it works on MS Windows too.) While I'm hesitant to touch something as low-level as `call-process', I think fixing this in general would be the best long-term solution, assuming it doesn't break something I'm unaware of. Another, less-invasive fix would be to fix `ispell-find-hunspell-dictionaries' to pass `nil' as the `infile' when invoking hunspell. [-- Attachment #2: 0001-Ensure-call-process-interprets-infile-as-a-local-pat.patch --] [-- Type: application/octet-stream, Size: 903 bytes --] From e737f400f7e57fc99222b5c351e4853735c2c6f4 Mon Sep 17 00:00:00 2001 From: Jim Porter <jporterbugs@gmail.com> Date: Tue, 29 Jun 2021 22:01:53 -0700 Subject: [PATCH] Ensure 'call-process' interprets 'infile' as a local path * src/callproc.c (Fcall_process): Interpret 'infile' relative to the directory from which 'program' is called, not 'default-directory'. --- src/callproc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/callproc.c b/src/callproc.c index aabc39313b..57cf781d28 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -270,7 +270,7 @@ DEFUN ("call-process", Fcall_process, Scall_process, 1, MANY, 0, if (nargs >= 2 && ! NILP (args[1])) { - infile = Fexpand_file_name (args[1], BVAR (current_buffer, directory)); + infile = Fexpand_file_name (args[1], encode_current_directory ()); CHECK_STRING (infile); } else -- 2.25.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#49283: [PATCH] 27.2; `(call-process "program" null-device ...)' fails over TRAMP from local MS Windows 2021-06-30 5:14 bug#49283: [PATCH] 27.2; `(call-process "program" null-device ...)' fails over TRAMP from local MS Windows Jim Porter @ 2021-06-30 7:24 ` Michael Albinus 2021-06-30 17:16 ` Jim Porter 0 siblings, 1 reply; 12+ messages in thread From: Michael Albinus @ 2021-06-30 7:24 UTC (permalink / raw) To: Jim Porter; +Cc: 49283 Jim Porter <jporterbugs@gmail.com> writes: Hi Jim, > I noticed an issue when trying to use flyspell (or ispell) using > hunspell from a local MS Windows system on a TRAMP file. It results in > an error that it can't find the file "/sshx:server:/path/to/NUL". I > narrowed this down to the fact that > `ispell-find-hunspell-dictionaries' calls `call-process' with `infile' > set to `null-device'. `call-process' doesn't know of remote files. You must use `process-file' instead. > To see this in action: > > emacs -Q > C-x C-f /sshx:server:~/path/to/file.txt > M-: (setq ispell-program-name "hunspell") RET > M-x flyspell-mode > ;; or... > M-: (call-process "something" null-device) RET It is not a good idea to use `null-device' as INFILE, just use nil. At least in the `process-file' case, Tramp shall know which value to take for `null-device'. After all, it seems to be rather a flyspell/ispell feature request to support remote files. Best regards, Michael. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#49283: [PATCH] 27.2; `(call-process "program" null-device ...)' fails over TRAMP from local MS Windows 2021-06-30 7:24 ` Michael Albinus @ 2021-06-30 17:16 ` Jim Porter 2021-07-01 11:07 ` Lars Ingebrigtsen 0 siblings, 1 reply; 12+ messages in thread From: Jim Porter @ 2021-06-30 17:16 UTC (permalink / raw) To: Michael Albinus; +Cc: 49283 [-- Attachment #1: Type: text/plain, Size: 1864 bytes --] On Wed, Jun 30, 2021 at 12:24 AM Michael Albinus <michael.albinus@gmx.de> wrote: > > Jim Porter <jporterbugs@gmail.com> writes: > > > I noticed an issue when trying to use flyspell (or ispell) using > > hunspell from a local MS Windows system on a TRAMP file. It results in > > an error that it can't find the file "/sshx:server:/path/to/NUL". I > > narrowed this down to the fact that > > `ispell-find-hunspell-dictionaries' calls `call-process' with `infile' > > set to `null-device'. > > `call-process' doesn't know of remote files. You must use `process-file' instead. That's not a problem; it's actually the right thing to do in this case, I think. flyspell/ispell is trying to use my local version of hunspell on the contents of a remote buffer. Since flyspell/ispell just look at the buffer contents and not the actual file, it can use `call-process'. > It is not a good idea to use `null-device' as INFILE, just use nil. At > least in the `process-file' case, Tramp shall know which value to take > for `null-device'. That fix would also work (see the attached patch). However, when I read the `call-process' documentation, it says that when `default-directory' is remote, it runs the program from "~". That's fine overall, and means it's a good way to be sure you're always running a process locally. It's just that when you do this, INFILE's path is expanded relative to the remote directory. I don't think that can ever work, since `call-process' doesn't know how to open a TRAMP file. Because of that, it makes more sense to me that you'd expand INFILE's path relative to wherever PROGRAM will be run from. That means that when `default-directory' is remote, both PROGRAM and INFILE are expanded relative to "~". That's more consistent, and my first patch would hopefully prevent similar errors anytime `call-process' is used from a remote buffer. [-- Attachment #2: 0001-Don-t-pass-null-device-to-call-process-in-ispell.patch --] [-- Type: application/octet-stream, Size: 915 bytes --] From 2eee9b4e70fe489eb46fb94ab000c6d0b7262e28 Mon Sep 17 00:00:00 2001 From: Jim Porter <jporterbugs@gmail.com> Date: Wed, 30 Jun 2021 10:01:32 -0700 Subject: [PATCH] Don't pass 'null-device' to 'call-process' in ispell * lisp/textmodes/ispell.el (ispell-find-hunspell-dictionaries): Replace 'null-device' with nil. --- lisp/textmodes/ispell.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lisp/textmodes/ispell.el b/lisp/textmodes/ispell.el index ce5a572085..0a82bf5a2d 100644 --- a/lisp/textmodes/ispell.el +++ b/lisp/textmodes/ispell.el @@ -1076,7 +1076,7 @@ ispell-find-hunspell-dictionaries (split-string (with-temp-buffer (ispell-call-process ispell-program-name - null-device + nil t nil "-D" -- 2.25.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#49283: [PATCH] 27.2; `(call-process "program" null-device ...)' fails over TRAMP from local MS Windows 2021-06-30 17:16 ` Jim Porter @ 2021-07-01 11:07 ` Lars Ingebrigtsen 2021-07-01 12:26 ` Michael Albinus 0 siblings, 1 reply; 12+ messages in thread From: Lars Ingebrigtsen @ 2021-07-01 11:07 UTC (permalink / raw) To: Jim Porter; +Cc: 49283, Michael Albinus Jim Porter <jporterbugs@gmail.com> writes: >> It is not a good idea to use `null-device' as INFILE, just use nil. At >> least in the `process-file' case, Tramp shall know which value to take >> for `null-device'. > > That fix would also work (see the attached patch). Applied to Emacs 28 now. > However, when I read the `call-process' documentation, it says that > when `default-directory' is remote, it runs the program from "~". > That's fine overall, and means it's a good way to be sure you're > always running a process locally. It's just that when you do this, > INFILE's path is expanded relative to the remote directory. I don't > think that can ever work, since `call-process' doesn't know how to > open a TRAMP file. Because of that, it makes more sense to me that > you'd expand INFILE's path relative to wherever PROGRAM will be run > from. That means that when `default-directory' is remote, both PROGRAM > and INFILE are expanded relative to "~". That's more consistent, and > my first patch would hopefully prevent similar errors anytime > `call-process' is used from a remote buffer. But is that what your first patch does? * src/callproc.c (Fcall_process): Interpret 'infile' relative to the directory from which 'program' is called, not 'default-directory'. --- src/callproc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/callproc.c b/src/callproc.c index aabc39313b..57cf781d28 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -270,7 +270,7 @@ DEFUN ("call-process", Fcall_process, Scall_process, 1, MANY, 0, if (nargs >= 2 && ! NILP (args[1])) { - infile = Fexpand_file_name (args[1], BVAR (current_buffer, directory)); + infile = Fexpand_file_name (args[1], encode_current_directory ()); CHECK_STRING (infile); -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#49283: [PATCH] 27.2; `(call-process "program" null-device ...)' fails over TRAMP from local MS Windows 2021-07-01 11:07 ` Lars Ingebrigtsen @ 2021-07-01 12:26 ` Michael Albinus 2021-07-01 12:34 ` Lars Ingebrigtsen 2021-07-01 13:12 ` Eli Zaretskii 0 siblings, 2 replies; 12+ messages in thread From: Michael Albinus @ 2021-07-01 12:26 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Jim Porter, 49283 Lars Ingebrigtsen <larsi@gnus.org> writes: Hi, >> However, when I read the `call-process' documentation, it says that >> when `default-directory' is remote, it runs the program from "~". >> That's fine overall, and means it's a good way to be sure you're >> always running a process locally. It's just that when you do this, >> INFILE's path is expanded relative to the remote directory. I don't >> think that can ever work, since `call-process' doesn't know how to >> open a TRAMP file. Because of that, it makes more sense to me that >> you'd expand INFILE's path relative to wherever PROGRAM will be run >> from. That means that when `default-directory' is remote, both PROGRAM >> and INFILE are expanded relative to "~". That's more consistent, and >> my first patch would hopefully prevent similar errors anytime >> `call-process' is used from a remote buffer. > > But is that what your first patch does? > > * src/callproc.c (Fcall_process): Interpret 'infile' relative to the > directory from which 'program' is called, not 'default-directory'. > --- > src/callproc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/callproc.c b/src/callproc.c > index aabc39313b..57cf781d28 100644 > --- a/src/callproc.c > +++ b/src/callproc.c > @@ -270,7 +270,7 @@ DEFUN ("call-process", Fcall_process, Scall_process, 1, MANY, 0, > > if (nargs >= 2 && ! NILP (args[1])) > { > - infile = Fexpand_file_name (args[1], BVAR (current_buffer, directory)); > + infile = Fexpand_file_name (args[1], encode_current_directory ()); > CHECK_STRING (infile); Yes, this seems TRT. encode_current_directory returns either default-directory if this is a local dir, or "~" otherwise. Expanding INFILE to that directory is OK, I believe. So we shall apply Jim's patch. Maybe the docstring could be enhanced a little bit at the end, saying that INFILE, if it is a relative file name, is expanded to the directory the process uses as cwd. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#49283: [PATCH] 27.2; `(call-process "program" null-device ...)' fails over TRAMP from local MS Windows 2021-07-01 12:26 ` Michael Albinus @ 2021-07-01 12:34 ` Lars Ingebrigtsen 2021-07-01 13:12 ` Eli Zaretskii 1 sibling, 0 replies; 12+ messages in thread From: Lars Ingebrigtsen @ 2021-07-01 12:34 UTC (permalink / raw) To: Michael Albinus; +Cc: Jim Porter, 49283 Michael Albinus <michael.albinus@gmx.de> writes: >> * src/callproc.c (Fcall_process): Interpret 'infile' relative to the >> directory from which 'program' is called, not 'default-directory'. [...] > Yes, this seems TRT. encode_current_directory returns either > default-directory if this is a local dir, or "~" otherwise. Expanding > INFILE to that directory is OK, I believe. > > So we shall apply Jim's patch. Maybe the docstring could be enhanced a > little bit at the end, saying that INFILE, if it is a relative file > name, is expanded to the directory the process uses as cwd. Oh, I interpreted the commit message as "the directory where 'program' is" and the patch didn't seem to do that. But I still don't understand "the directory from which 'program' is called", because that's `default-directory'. (Except, as you say, when it's a remote dir.) The patch looks good to me, too, but the commit message is confusing. :-) -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#49283: [PATCH] 27.2; `(call-process "program" null-device ...)' fails over TRAMP from local MS Windows 2021-07-01 12:26 ` Michael Albinus 2021-07-01 12:34 ` Lars Ingebrigtsen @ 2021-07-01 13:12 ` Eli Zaretskii 2021-07-01 19:45 ` Jim Porter 1 sibling, 1 reply; 12+ messages in thread From: Eli Zaretskii @ 2021-07-01 13:12 UTC (permalink / raw) To: Michael Albinus; +Cc: jporterbugs, larsi, 49283 > From: Michael Albinus <michael.albinus@gmx.de> > Date: Thu, 01 Jul 2021 14:26:08 +0200 > Cc: Jim Porter <jporterbugs@gmail.com>, 49283@debbugs.gnu.org > > Lars Ingebrigtsen <larsi@gnus.org> writes: > > > - infile = Fexpand_file_name (args[1], BVAR (current_buffer, directory)); > > + infile = Fexpand_file_name (args[1], encode_current_directory ()); > > CHECK_STRING (infile); > > Yes, this seems TRT. encode_current_directory returns either > default-directory if this is a local dir, or "~" otherwise. Expanding > INFILE to that directory is OK, I believe. > > So we shall apply Jim's patch. Maybe the docstring could be enhanced a > little bit at the end, saying that INFILE, if it is a relative file > name, is expanded to the directory the process uses as cwd. encode_current_directory returns an encoded file name. So if we make this change, we should avoid calling ENCODE_FILE on it (doing so is a no-op, but it's still unclean). ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#49283: [PATCH] 27.2; `(call-process "program" null-device ...)' fails over TRAMP from local MS Windows 2021-07-01 13:12 ` Eli Zaretskii @ 2021-07-01 19:45 ` Jim Porter 2021-07-02 6:37 ` Eli Zaretskii 0 siblings, 1 reply; 12+ messages in thread From: Jim Porter @ 2021-07-01 19:45 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 49283, Lars Ingebrigtsen, Michael Albinus [-- Attachment #1: Type: text/plain, Size: 1297 bytes --] On Thu, Jul 1, 2021 at 6:12 AM Eli Zaretskii <eliz@gnu.org> wrote: > > > From: Michael Albinus <michael.albinus@gmx.de> > > Date: Thu, 01 Jul 2021 14:26:08 +0200 > > Cc: Jim Porter <jporterbugs@gmail.com>, 49283@debbugs.gnu.org > > > > So we shall apply Jim's patch. Maybe the docstring could be enhanced a > > little bit at the end, saying that INFILE, if it is a relative file > > name, is expanded to the directory the process uses as cwd. > > encode_current_directory returns an encoded file name. So if we make > this change, we should avoid calling ENCODE_FILE on it (doing so is a > no-op, but it's still unclean). I'd considered that when writing my initial patch to `call-process', but I wasn't sure what the most-correct way to avoid that would be. It seems we want an encoded path before returning from `encode_current_directory' in order to check that our result is actually accessible. But then that encoded dir gets passed in to `expand-file-name'. If INFILE is an un-encoded absolute path, wouldn't `expand-file-name' be un-encoded as well? Maybe a better way would be to get the cwd *without* encoding it (see the attached patch). However, maybe there's a simpler answer to all of this that I just don't know about since I'm not very familiar with how Emacs encodes file names. [-- Attachment #2: 0001-Ensure-call-process-interprets-infile-as-a-local-pat.patch --] [-- Type: application/octet-stream, Size: 1997 bytes --] From e85d1e8db8164fe72faad4d5ef51d84f9d844ace Mon Sep 17 00:00:00 2001 From: Jim Porter <jporterbugs@gmail.com> Date: Thu, 1 Jul 2021 12:41:49 -0700 Subject: [PATCH] Ensure 'call-process' interprets 'infile' as a local path * src/callproc.c (Fcall_process): Interpret 'infile' relative to the working directory from which 'program' is run, not 'default-directory'. --- src/callproc.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/callproc.c b/src/callproc.c index aabc39313b..142fb4cb23 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -225,8 +225,9 @@ DEFUN ("call-process", Fcall_process, Scall_process, 1, MANY, 0, The remaining arguments are optional. The program's input comes from file INFILE (nil means `null-device'). -If you want to make the input come from an Emacs buffer, use -`call-process-region' instead. +If INFILE is a relative path, it will be looked for relative to the +directory where the process is run (see below). If you want to make the +input come from an Emacs buffer, use `call-process-region' instead. Third argument DESTINATION specifies how to handle program's output. If DESTINATION is a buffer, or t that stands for the current buffer, @@ -270,7 +271,19 @@ DEFUN ("call-process", Fcall_process, Scall_process, 1, MANY, 0, if (nargs >= 2 && ! NILP (args[1])) { - infile = Fexpand_file_name (args[1], BVAR (current_buffer, directory)); + /* Expand infile relative to the current buffer's current + directory, or its unhandled equivalent ("~"). */ + Lisp_Object curdir = BVAR (current_buffer, directory); + curdir = Funhandled_file_name_directory (curdir); + + /* If the file name handler says that dir is unreachable, use + a sensible default. */ + if (NILP (curdir)) + curdir = build_string ("~"); + + curdir = expand_and_dir_to_file (curdir); + + infile = Fexpand_file_name (args[1], curdir); CHECK_STRING (infile); } else -- 2.25.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#49283: [PATCH] 27.2; `(call-process "program" null-device ...)' fails over TRAMP from local MS Windows 2021-07-01 19:45 ` Jim Porter @ 2021-07-02 6:37 ` Eli Zaretskii 2021-07-02 18:47 ` Jim Porter 0 siblings, 1 reply; 12+ messages in thread From: Eli Zaretskii @ 2021-07-02 6:37 UTC (permalink / raw) To: Jim Porter; +Cc: 49283, larsi, michael.albinus > From: Jim Porter <jporterbugs@gmail.com> > Date: Thu, 1 Jul 2021 12:45:42 -0700 > Cc: Michael Albinus <michael.albinus@gmx.de>, Lars Ingebrigtsen <larsi@gnus.org>, 49283@debbugs.gnu.org > > > encode_current_directory returns an encoded file name. So if we make > > this change, we should avoid calling ENCODE_FILE on it (doing so is a > > no-op, but it's still unclean). > > I'd considered that when writing my initial patch to `call-process', > but I wasn't sure what the most-correct way to avoid that would be. It > seems we want an encoded path before returning from > `encode_current_directory' in order to check that our result is > actually accessible. But then that encoded dir gets passed in to > `expand-file-name'. If INFILE is an un-encoded absolute path, wouldn't > `expand-file-name' be un-encoded as well? expand-file-name is not a problem, it can deal with encoded file names. The problem is the calls to remove_slash_colon and report_file_error: they should receive file names in their internal representation. > Maybe a better way would be to get the cwd *without* encoding it (see > the attached patch). However, maybe there's a simpler answer to all of > this that I just don't know about since I'm not very familiar with how > Emacs encodes file names. How about adding a 'bool' argument to encode_current_directory, so that the caller could control whether or not it encodes the directory file name? Then you could in this case tell encode_current_directory not to encode the directory file name. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#49283: [PATCH] 27.2; `(call-process "program" null-device ...)' fails over TRAMP from local MS Windows 2021-07-02 6:37 ` Eli Zaretskii @ 2021-07-02 18:47 ` Jim Porter 2021-07-03 7:02 ` Eli Zaretskii 2021-07-04 13:32 ` Lars Ingebrigtsen 0 siblings, 2 replies; 12+ messages in thread From: Jim Porter @ 2021-07-02 18:47 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 49283, Lars Ingebrigtsen, Michael Albinus [-- Attachment #1: Type: text/plain, Size: 1698 bytes --] On Thu, Jul 1, 2021 at 11:37 PM Eli Zaretskii <eliz@gnu.org> wrote: > > > From: Jim Porter <jporterbugs@gmail.com> > > Date: Thu, 1 Jul 2021 12:45:42 -0700 > > Cc: Michael Albinus <michael.albinus@gmx.de>, Lars Ingebrigtsen <larsi@gnus.org>, 49283@debbugs.gnu.org > > > > I'd considered that when writing my initial patch to `call-process', > > but I wasn't sure what the most-correct way to avoid that would be. It > > seems we want an encoded path before returning from > > `encode_current_directory' in order to check that our result is > > actually accessible. But then that encoded dir gets passed in to > > `expand-file-name'. If INFILE is an un-encoded absolute path, wouldn't > > `expand-file-name' be un-encoded as well? > > expand-file-name is not a problem, it can deal with encoded file > names. The problem is the calls to remove_slash_colon and > report_file_error: they should receive file names in their internal > representation. Right, I was just worried that if I relied on `encode_current_directory' returning an encoded path, `expand-file-name' might sometimes return an encoded path (e.g. if INFILE is a simple relative path like "foo") and sometimes an unencoded path (e.g. if INFILE is an absolute path). I might be wrong though, since I didn't look closely at the implementation... > How about adding a 'bool' argument to encode_current_directory, so > that the caller could control whether or not it encodes the directory > file name? Then you could in this case tell encode_current_directory > not to encode the directory file name. Ok, I did that (and renamed it to `get_current_directory' since it doesn't always encode anymore). How does the attached patch look? [-- Attachment #2: 0001-Ensure-call-process-interprets-infile-as-a-local-pat.patch --] [-- Type: application/octet-stream, Size: 5127 bytes --] From ab6e47952944e94063a4db6442ee3549dc46bfb7 Mon Sep 17 00:00:00 2001 From: Jim Porter <jporterbugs@gmail.com> Date: Fri, 2 Jul 2021 11:41:41 -0700 Subject: [PATCH] Ensure 'call-process' interprets INFILE as a local path * src/callproc.c (get_current_directory): Rename from 'encode_current_directory' and add boolean ENCODE flag. (Fcall_process): Interpret INFILE relative to the working directory from which PROGRAM is run, not 'default-directory'. (call_process): Use 'get_current_directory'. * src/process.c (Fmake_process): Use 'get_current_directory'. * src/process.h (get_current_directory): Rename decl from 'encode_current_directory'. * src/sysdep.c (sys_subshell): Use 'get_current_directory'. --- src/callproc.c | 25 +++++++++++++++---------- src/process.c | 2 +- src/process.h | 2 +- src/sysdep.c | 2 +- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/callproc.c b/src/callproc.c index aabc39313b..675b78daf3 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -116,11 +116,13 @@ #define _P_NOWAIT 1 /* from process.h */ const char *); \f /* Return the current buffer's working directory, or the home - directory if it's unreachable, as a string suitable for a system call. - Signal an error if the result would not be an accessible directory. */ + directory if it's unreachable. If ENCODE is true, return as a string + suitable for a system call; otherwise, return a string in its + internal representation. Signal an error if the result would not be + an accessible directory. */ Lisp_Object -encode_current_directory (void) +get_current_directory (bool encode) { Lisp_Object curdir = BVAR (current_buffer, directory); Lisp_Object dir = Funhandled_file_name_directory (curdir); @@ -131,12 +133,12 @@ encode_current_directory (void) dir = build_string ("~"); dir = expand_and_dir_to_file (dir); - dir = ENCODE_FILE (remove_slash_colon (dir)); + Lisp_Object encoded_dir = ENCODE_FILE (remove_slash_colon (dir)); - if (! file_accessible_directory_p (dir)) + if (! file_accessible_directory_p (encoded_dir)) report_file_error ("Setting current directory", curdir); - return dir; + return encode ? encoded_dir : dir; } /* If P is reapable, record it as a deleted process and kill it. @@ -225,8 +227,9 @@ DEFUN ("call-process", Fcall_process, Scall_process, 1, MANY, 0, The remaining arguments are optional. The program's input comes from file INFILE (nil means `null-device'). -If you want to make the input come from an Emacs buffer, use -`call-process-region' instead. +If INFILE is a relative path, it will be looked for relative to the +directory where the process is run (see below). If you want to make the +input come from an Emacs buffer, use `call-process-region' instead. Third argument DESTINATION specifies how to handle program's output. If DESTINATION is a buffer, or t that stands for the current buffer, @@ -270,7 +273,9 @@ DEFUN ("call-process", Fcall_process, Scall_process, 1, MANY, 0, if (nargs >= 2 && ! NILP (args[1])) { - infile = Fexpand_file_name (args[1], BVAR (current_buffer, directory)); + /* Expand infile relative to the current buffer's current + directory, or its unhandled equivalent ("~"). */ + infile = Fexpand_file_name (args[1], get_current_directory (false)); CHECK_STRING (infile); } else @@ -439,7 +444,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd, buffer's current directory, or its unhandled equivalent. We can't just have the child check for an error when it does the chdir, since it's in a vfork. */ - current_dir = encode_current_directory (); + current_dir = get_current_directory (true); if (STRINGP (error_file)) { diff --git a/src/process.c b/src/process.c index c354f3a90d..b8c3e4ecfb 100644 --- a/src/process.c +++ b/src/process.c @@ -1755,7 +1755,7 @@ DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0, buffer's current directory, or its unhandled equivalent. We can't just have the child check for an error when it does the chdir, since it's in a vfork. */ - current_dir = encode_current_directory (); + current_dir = get_current_directory (true); name = Fplist_get (contact, QCname); CHECK_STRING (name); diff --git a/src/process.h b/src/process.h index 0890f253a4..4a25d13d26 100644 --- a/src/process.h +++ b/src/process.h @@ -264,7 +264,7 @@ pset_gnutls_cred_type (struct Lisp_Process *p, Lisp_Object val) /* Defined in callproc.c. */ -extern Lisp_Object encode_current_directory (void); +extern Lisp_Object get_current_directory (bool); extern void record_kill_process (struct Lisp_Process *, Lisp_Object); /* Defined in sysdep.c. */ diff --git a/src/sysdep.c b/src/sysdep.c index 51d8b5eeed..b8ec22d9dd 100644 --- a/src/sysdep.c +++ b/src/sysdep.c @@ -657,7 +657,7 @@ sys_subshell (void) #endif pid_t pid; struct save_signal saved_handlers[5]; - char *str = SSDATA (encode_current_directory ()); + char *str = SSDATA (get_current_directory (true)); #ifdef DOS_NT pid = 0; -- 2.25.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#49283: [PATCH] 27.2; `(call-process "program" null-device ...)' fails over TRAMP from local MS Windows 2021-07-02 18:47 ` Jim Porter @ 2021-07-03 7:02 ` Eli Zaretskii 2021-07-04 13:32 ` Lars Ingebrigtsen 1 sibling, 0 replies; 12+ messages in thread From: Eli Zaretskii @ 2021-07-03 7:02 UTC (permalink / raw) To: Jim Porter; +Cc: 49283, larsi, michael.albinus > From: Jim Porter <jporterbugs@gmail.com> > Date: Fri, 2 Jul 2021 11:47:12 -0700 > Cc: Michael Albinus <michael.albinus@gmx.de>, Lars Ingebrigtsen <larsi@gnus.org>, 49283@debbugs.gnu.org > > > expand-file-name is not a problem, it can deal with encoded file > > names. The problem is the calls to remove_slash_colon and > > report_file_error: they should receive file names in their internal > > representation. > > Right, I was just worried that if I relied on > `encode_current_directory' returning an encoded path, > `expand-file-name' might sometimes return an encoded path (e.g. if > INFILE is a simple relative path like "foo") and sometimes an > unencoded path (e.g. if INFILE is an absolute path). I might be wrong > though, since I didn't look closely at the implementation... > > > How about adding a 'bool' argument to encode_current_directory, so > > that the caller could control whether or not it encodes the directory > > file name? Then you could in this case tell encode_current_directory > > not to encode the directory file name. > > Ok, I did that (and renamed it to `get_current_directory' since it > doesn't always encode anymore). How does the attached patch look? LGTM, thanks. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#49283: [PATCH] 27.2; `(call-process "program" null-device ...)' fails over TRAMP from local MS Windows 2021-07-02 18:47 ` Jim Porter 2021-07-03 7:02 ` Eli Zaretskii @ 2021-07-04 13:32 ` Lars Ingebrigtsen 1 sibling, 0 replies; 12+ messages in thread From: Lars Ingebrigtsen @ 2021-07-04 13:32 UTC (permalink / raw) To: Jim Porter; +Cc: 49283, Michael Albinus Jim Porter <jporterbugs@gmail.com> writes: > Ok, I did that (and renamed it to `get_current_directory' since it > doesn't always encode anymore). How does the attached patch look? Looks good; so I've now pushed it to Emacs 28, and with that I'm closing this bug report. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-07-04 13:32 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-06-30 5:14 bug#49283: [PATCH] 27.2; `(call-process "program" null-device ...)' fails over TRAMP from local MS Windows Jim Porter 2021-06-30 7:24 ` Michael Albinus 2021-06-30 17:16 ` Jim Porter 2021-07-01 11:07 ` Lars Ingebrigtsen 2021-07-01 12:26 ` Michael Albinus 2021-07-01 12:34 ` Lars Ingebrigtsen 2021-07-01 13:12 ` Eli Zaretskii 2021-07-01 19:45 ` Jim Porter 2021-07-02 6:37 ` Eli Zaretskii 2021-07-02 18:47 ` Jim Porter 2021-07-03 7:02 ` Eli Zaretskii 2021-07-04 13:32 ` Lars Ingebrigtsen
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.