Using `call-process-shell-command' in `process-lines'

Using `call-process-shell-command' in `process-lines'

[Juanma Barranquero]

Does anyone see a problem with this change, and specifically in
non-Windows environments?

The goal is to make `process-lines' (and so, `authors' and
`check-declare-directory') to be more Windows-friendly; frienldy as in
"now it works", I mean :).

             Juanma


2007-11-26  Juanma Barranquero  <lekktu@gmail.com>

	* subr.el (process-lines): Execute args as a shell command,
	not as a program.

	* emacs-lisp/authors.el (authors): Don't hardcode `find'.
	Use `shell-quote-argument'.

	* emacs-lisp/check-declare.el (check-declare-directory):
	Don't hardcode `find' and `grep'.  `Use shell-quote-argument'.



Index: lisp/subr.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/subr.el,v
retrieving revision 1.571
diff -u -2 -r1.571 subr.el
--- lisp/subr.el	22 Nov 2007 20:14:12 -0000	1.571
+++ lisp/subr.el	26 Nov 2007 12:16:25 -0000
@@ -1541,11 +1541,11 @@
 ;;;; Process stuff.

-(defun process-lines (program &rest args)
-  "Execute PROGRAM with ARGS, returning its output as a list of lines.
-Signal an error if the program returns with a non-zero exit status."
+(defun process-lines (command &rest args)
+  "Execute shell COMMAND with ARGS, returning its output as a list of lines.
+Signal an error if the command returns with a non-zero exit status."
   (with-temp-buffer
-    (let ((status (apply 'call-process program nil (current-buffer) nil args)))
+    (let ((status (apply 'call-process-shell-command command nil
(current-buffer) nil args)))
       (unless (eq status 0)
-	(error "%s exited with status %s" program status))
+	(error "%s exited with status %s" command status))
       (goto-char (point-min))
       (let (lines)
Index: lisp/emacs-lisp/authors.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/emacs-lisp/authors.el,v
retrieving revision 1.47
diff -u -2 -r1.47 authors.el
--- lisp/emacs-lisp/authors.el	17 Nov 2007 03:43:54 -0000	1.47
+++ lisp/emacs-lisp/authors.el	26 Nov 2007 12:46:30 -0000
@@ -588,5 +588,6 @@
   (interactive "DEmacs source directory: ")
   (setq root (expand-file-name root))
-  (let ((logs (process-lines "find" root "-name" "ChangeLog*"))
+  (let ((logs (process-lines find-program root
+			     "-name" (shell-quote-argument "ChangeLog*")))
 	(table (make-hash-table :test 'equal))
 	(buffer-name "*Authors*")
@@ -600,5 +601,6 @@
 	(message "Scanning %s..." log)
 	(authors-scan-change-log log table)))
-    (let ((els (process-lines "find" root "-name" "*.el")))
+    (let ((els (process-lines find-program root
+			      "-name" (shell-quote-argument "*.el"))))
       (dolist (file els)
 	(message "Scanning %s..." file)
Index: lisp/emacs-lisp/check-declare.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/emacs-lisp/check-declare.el,v
retrieving revision 1.9
diff -u -2 -r1.9 check-declare.el
--- lisp/emacs-lisp/check-declare.el	24 Nov 2007 03:11:14 -0000	1.9
+++ lisp/emacs-lisp/check-declare.el	26 Nov 2007 12:36:27 -0000
@@ -245,7 +245,8 @@
     (message "%s" m)
     (message "%s" m2)
-    (setq files (process-lines "find" root "-name" "*.el"
-                                 "-exec" "grep" "-l"
-                                 "^[ 	]*(declare-function" "{}" ";"))
+    (setq files (process-lines find-program root
+			       "-name" (shell-quote-argument "*.el")
+			       "-exec" grep-program "-l"
+			       (shell-quote-argument "^[\t ]*(declare-function") "{}" ";"))
     (message "%s%d found" m2 (length files))
     (when files

Re: Using `call-process-shell-command' in `process-lines'

[Stefan Monnier]

> Does anyone see a problem with this change, and specifically in
> non-Windows environments?

Yes: the use of `args' in call-process-shell-command is strongly
discouraged in my office because it doesn't do what it says it does: it
doesn't treat each element of the list as an argument.  Instead it just
concatenates the elements of the list and then passes the resulting
single string to the shell which will split it into arguments accoring
to its own rules.

> The goal is to make `process-lines' (and so, `authors' and
> `check-declare-directory') to be more Windows-friendly; frienldy as in
> "now it works", I mean :).

Please describe the actual problem so we can find another solution
to it.


        Stefan

Re: Using `call-process-shell-command' in `process-lines'

[Juanma Barranquero]

On Nov 26, 2007 4:35 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> Yes: the use of `args' in call-process-shell-command is strongly
> discouraged in my office because it doesn't do what it says it does: it
> doesn't treat each element of the list as an argument.

I could've bet that not commiting the patch was bound to be a good idea ;)

> Please describe the actual problem so we can find another solution
> to it.

There are two parts: the first one is hardcoded "find" and "grep"
commands, which I want to replace with find-program and grep-command.
I suppose you won't oppose to that.

As for the problem:

 - Running check-declare-directory as it is now (after changing "find"
to `find-program' and "grep" to `grep-program' and suitably setting
the variables):

process-lines: c:/bin/gnuwin32/bin/find.exe exited with status 1
process-lines: c:/bin/msys/bin/find.exe exited with status 1

This is with MSYS' find ("GNU find version 4.1") and GnuWin32's find
("GNU find version 4.2.20").

 - After I wrap "*.el" and "^[\t ]*(check-function" in `shell-quote-argument':

Both programs return succesfully without finding any *.el file (root
is c:/emacs/trunk/lisp/ when I invoke check-declare-directory).

             Juanma

Re: Using `call-process-shell-command' in `process-lines'

[Stefan Monnier]

>> Please describe the actual problem so we can find another solution
>> to it.

> There are two parts: the first one is hardcoded "find" and "grep"
> commands, which I want to replace with find-program and grep-command.
> I suppose you won't oppose to that.

> As for the problem:

>  - Running check-declare-directory as it is now (after changing "find"
> to `find-program' and "grep" to `grep-program' and suitably setting
> the variables):

> process-lines: c:/bin/gnuwin32/bin/find.exe exited with status 1
> process-lines: c:/bin/msys/bin/find.exe exited with status 1

> This is with MSYS' find ("GNU find version 4.1") and GnuWin32's find
> ("GNU find version 4.2.20").

I have no idea what this means.  Could you expand on it?

>  - After I wrap "*.el" and "^[\t ]*(check-function" in `shell-quote-argument':

> Both programs return succesfully without finding any *.el file (root
> is c:/emacs/trunk/lisp/ when I invoke check-declare-directory).

Why would you want to wrap them?  They're not passed to a shell!


        Stefan

Re: Using `call-process-shell-command' in `process-lines'

[Juanma Barranquero]

On Nov 26, 2007 5:43 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> > process-lines: c:/bin/gnuwin32/bin/find.exe exited with status 1
> > process-lines: c:/bin/msys/bin/find.exe exited with status 1
>
> > This is with MSYS' find ("GNU find version 4.1") and GnuWin32's find
> > ("GNU find version 4.2.20").
>
> I have no idea what this means.  Could you expand on it?

I'm not sure what are you referring to. check-declare-directory calls
find. I've tested it with GNU find, versions 4.1 (the one included
with the newest release of MSYS, which itself is related to MinGW, see
http://www.mingw.org) and version 4.2.20 (the one in the GnuWin32
project, see http://gnuwin32.sourceforge.net). Both fail.

If I manually execute

  (apply #'call-process "c:/bin/gnuwin32/bin/find.exe" nil
(get-buffer-create "output") nil '("c:/emacs/trunk/lisp/" "-name"
"*.el"))

I get:

  c:\bin\gnuwin32\bin\find.exe: paths must precede expression
  Usage: c:\bin\gnuwin32\bin\find.exe [-H] [-L] [-P] [path...] [expression]

or

  /usr/bin/find: paths must precede expression
  Usage: /usr/bin/find [path...] [expression]

> Why would you want to wrap them?  They're not passed to a shell!

Passing "*" or "*.el" to -name fails as above. Passing ".el" or almost
anything else finishes (with 0 and no files). So there's something
fishy with the wildcard.

             Juanma

Re: Using `call-process-shell-command' in `process-lines'

[Lennart Borgman (gmail)]

Juanma Barranquero wrote:
> Passing "*" or "*.el" to -name fails as above. Passing ".el" or almost
> anything else finishes (with 0 and no files). So there's something
> fishy with the wildcard.


In a cmd.exe shell I can execute

    find -name "*.el"

with desired result (using gnuwin32 find.exe), but

    find -name *.el

fails with

    find: paths must precede expression
    Usage: find [path...] [expression]

Doesn't it look like arg passing in find.exe is a bit strange?

Re: Using `call-process-shell-command' in `process-lines'

[Juanma Barranquero]

On Nov 26, 2007 6:28 PM, Lennart Borgman (gmail)
<lennart.borgman@gmail.com> wrote:

> Doesn't it look like arg passing in find.exe is a bit strange?

I think "yes" is the only sensible answer...

             Juanma

Re: Using `call-process-shell-command' in `process-lines'

[Lennart Borgman (gmail)]

Juanma Barranquero wrote:
> On Nov 26, 2007 6:28 PM, Lennart Borgman (gmail)
> <lennart.borgman@gmail.com> wrote:
> 
>> Doesn't it look like arg passing in find.exe is a bit strange?
> 
> I think "yes" is the only sensible answer...


A summary using gnuwin32 find.exe:

- This gives the error "paths must precede expressions"

   (apply #'call-process "find.exe" nil
          (get-buffer-create "output")
          nil
          '("c:/emacs-lisp/old/" "-name" "*.el" ))

- The same for this in cmd.exe

   find -name *.el


- These two works:

   (apply #'call-process "find.exe" nil
          (get-buffer-create "output")
          nil
          '("c:/emacs-lisp/old/" "-name" "*l.el" ))

   (apply #'call-process "find.exe" nil
          (get-buffer-create "output")
          nil
          '("c:/emacs-lisp/old/" "-name" "m*.el" ))


- and these works in cmd.exe:

   find -name *l.el
   find -name m*.el
   find -name "*.el"

The arg parsing loop in find looks like this (though I do not know if it 
matters here):

   parse_open (argv, &argc);
   /* Build the input order list. */
   while (i < argc)
     {
       if (strchr ("-!(),", argv[i][0]) == NULL)
	usage (_("paths must precede expression"));
       predicate_name = argv[i];
       parse_function = find_parser (predicate_name);
       if (parse_function == NULL)
	/* Command line option not recognized */
	error (1, 0, _("invalid predicate `%s'"), predicate_name);
       i++;
       if (!(*parse_function) (argv, &i))
	{
	  if (argv[i] == NULL)
	    /* Command line option requires an argument */
	    error (1, 0, _("missing argument to `%s'"), predicate_name);
	  else
	    error (1, 0, _("invalid argument `%s' to `%s'"),
		   argv[i], predicate_name);
	}
     }

Trying to understand what parse_function does is beyond my time table 
;-) -- However from the loop above one can see that somehow 
parse_function seems to make a bad guess, but strangely enough not if 
the argument *.el is enclosed like "*.el" when it is called from the 
cmd.exe shell.

Is anyone able to understand what is going on?

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> Date: Mon, 26 Nov 2007 20:58:55 +0100
> From: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,
> 	Emacs Devel <emacs-devel@gnu.org>
> 
> - This gives the error "paths must precede expressions"
> 
>    (apply #'call-process "find.exe" nil
>           (get-buffer-create "output")
>           nil
>           '("c:/emacs-lisp/old/" "-name" "*.el" ))
> 
> - The same for this in cmd.exe
> 
>    find -name *.el

Yes, you need to quote the *.el, so that find.exe gets it intact.

> - These two works:
> 
>    (apply #'call-process "find.exe" nil
>           (get-buffer-create "output")
>           nil
>           '("c:/emacs-lisp/old/" "-name" "*l.el" ))
> 
>    (apply #'call-process "find.exe" nil
>           (get-buffer-create "output")
>           nil
>           '("c:/emacs-lisp/old/" "-name" "m*.el" ))
> 
> 
> - and these works in cmd.exe:
> 
>    find -name *l.el
>    find -name m*.el
>    find -name "*.el"

Sheer luck, the first tow ones.  You _must_ quote the wildcard to get
predictable behavior.

> Is anyone able to understand what is going on?

I explained that in another message.

Re: Using `call-process-shell-command' in `process-lines'

[Lennart Borgman (gmail)]

Eli Zaretskii wrote:
>> Date: Mon, 26 Nov 2007 20:58:55 +0100
>> From: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,
>> 	Emacs Devel <emacs-devel@gnu.org>
>>
>> - This gives the error "paths must precede expressions"
>>
>>    (apply #'call-process "find.exe" nil
>>           (get-buffer-create "output")
>>           nil
>>           '("c:/emacs-lisp/old/" "-name" "*.el" ))
>>
>> - The same for this in cmd.exe
>>
>>    find -name *.el
> 
> Yes, you need to quote the *.el, so that find.exe gets it intact.
> 
>> - These two works:
>>
>>    (apply #'call-process "find.exe" nil
>>           (get-buffer-create "output")
>>           nil
>>           '("c:/emacs-lisp/old/" "-name" "*l.el" ))
>>
>>    (apply #'call-process "find.exe" nil
>>           (get-buffer-create "output")
>>           nil
>>           '("c:/emacs-lisp/old/" "-name" "m*.el" ))
>>
>>
>> - and these works in cmd.exe:
>>
>>    find -name *l.el
>>    find -name m*.el
>>    find -name "*.el"
> 
> Sheer luck, the first tow ones.  You _must_ quote the wildcard to get
> predictable behavior.
> 
>> Is anyone able to understand what is going on?
> 
> I explained that in another message.

I think there is more to it. I suspect that the find arg parsing code 
handles a single "*" differently and that this is a bug. (But I am not 
sure, that code takes long time to read.)

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> Date: Mon, 26 Nov 2007 22:25:43 +0100
> From: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>
> CC: lekktu@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> >>    find -name *l.el
> >>    find -name m*.el
> >>    find -name "*.el"
> > 
> > Sheer luck, the first tow ones.  You _must_ quote the wildcard to get
> > predictable behavior.
> > 
> >> Is anyone able to understand what is going on?
> > 
> > I explained that in another message.
> 
> I think there is more to it. I suspect that the find arg parsing code 
> handles a single "*" differently and that this is a bug.

What do you mean by ``find arg parsing code''?  Quoted wildcards are
parsed by the application code called from `find's `main' function,
while unquoted wildcards are parsed by the startup code which runs
before `main'.  These two are different: the former uses GNU `fnmatch'
function (and thus you can use wildcards like "[a-d]*.el"), while the
latter uses a function from Microsoft' runtime, which supports only
the limited Windows semantics of wildcards.

But all this has no direct relation to the problem at hand.  The
problem at hand is that we pass the "*.el" arg incorrectly to `find',
not what `find' does with it.

Re: Using `call-process-shell-command' in `process-lines'

[Lennart Borgman (gmail)]

Eli Zaretskii wrote:
>> Date: Mon, 26 Nov 2007 22:25:43 +0100
>> From: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>
>> CC: lekktu@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>>>>    find -name *l.el
>>>>    find -name m*.el
>>>>    find -name "*.el"
>>> Sheer luck, the first tow ones.  You _must_ quote the wildcard to get
>>> predictable behavior.
>>>
>>>> Is anyone able to understand what is going on?
>>> I explained that in another message.
>> I think there is more to it. I suspect that the find arg parsing code 
>> handles a single "*" differently and that this is a bug.
> 
> What do you mean by ``find arg parsing code''?  Quoted wildcards are
> parsed by the application code called from `find's `main' function,
> while unquoted wildcards are parsed by the startup code which runs
> before `main'.  These two are different: the former uses GNU `fnmatch'
> function (and thus you can use wildcards like "[a-d]*.el"), while the
> latter uses a function from Microsoft' runtime, which supports only
> the limited Windows semantics of wildcards.

I mean the code that it is called from find's main function. But I am 
not sure what runs when. Actually the two calls below

    find -name "[ni]*.el"
    find -name [ni]*.el

both works for me when I run them in cmd.exe. So both this seems to be 
parsed by find's main function. The case that fails is

    find -name *.el

See also above for the two cases that might be "sheer luck".

> But all this has no direct relation to the problem at hand.  The
> problem at hand is that we pass the "*.el" arg incorrectly to `find',
> not what `find' does with it.
> 

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> Date: Mon, 26 Nov 2007 23:16:05 +0100
> From: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>
> CC: lekktu@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> Actually the two calls below
> 
>     find -name "[ni]*.el"
>     find -name [ni]*.el
> 
> both works for me when I run them in cmd.exe.

That's because the unquoted wildcard [ni]*.el does not match anything
when the startup code tries to expand it.  Such a wildcard is passed
unaltered to the `find's `main' function, which has the same effect as
if it were quoted.

> The case that fails is
> 
>     find -name *.el

Because *.el matches something.  Try running it from a directory that
doesn't have any .el files, like this:

   find /path/to/emacs/lisp -name *.el

and you will see the difference.

Again, this has nothing to do with the problem at hand.

Re: Using `call-process-shell-command' in `process-lines'

[Lennart Borgman (gmail)]

Eli Zaretskii wrote:
>> Date: Mon, 26 Nov 2007 23:16:05 +0100
>> From: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>
>> CC: lekktu@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>> Actually the two calls below
>>
>>     find -name "[ni]*.el"
>>     find -name [ni]*.el
>>
>> both works for me when I run them in cmd.exe.
> 
> That's because the unquoted wildcard [ni]*.el does not match anything
> when the startup code tries to expand it.  Such a wildcard is passed
> unaltered to the `find's `main' function, which has the same effect as
> if it were quoted.
> 
>> The case that fails is
>>
>>     find -name *.el
> 
> Because *.el matches something.

But

   find -name m*.el

also matches something and it works. Is perhaps m*.el still passed 
unaltered? This should explain the differences I see then.

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> Date: Mon, 26 Nov 2007 23:39:59 +0100
> From: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>
> CC: lekktu@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> >> The case that fails is
> >>
> >>     find -name *.el
> > 
> > Because *.el matches something.
> 
> But
> 
>    find -name m*.el
> 
> also matches something and it works.

As I explained in other messages, whether this works or not depends on
what files are there in the directory where you run the command (not
in the directory where `find' starts its search).  This is specific to
each machine, so you will need to research this yourself.  In
particular, the number of matching files (zero, one, or more than one)
matters.

Re: Using `call-process-shell-command' in `process-lines'

[Lennart Borgman (gmail)]

Eli Zaretskii wrote:
>> Date: Mon, 26 Nov 2007 23:39:59 +0100
>> From: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>
>> CC: lekktu@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>>>> The case that fails is
>>>>
>>>>     find -name *.el
>>> Because *.el matches something.
>> But
>>
>>    find -name m*.el
>>
>> also matches something and it works.
> 
> As I explained in other messages, whether this works or not depends on
> what files are there in the directory where you run the command (not
> in the directory where `find' starts its search).  This is specific to
> each machine, so you will need to research this yourself.  In
> particular, the number of matching files (zero, one, or more than one)
> matters.

Thanks, I think I missed the note you made before about more than one 
file matching.

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> Date: Tue, 27 Nov 2007 00:04:02 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: lekktu@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > >>    find -name *l.el
> > >>    find -name m*.el
> > >>    find -name "*.el"
> > > 
> > > Sheer luck, the first tow ones.  You _must_ quote the wildcard to get
> > > predictable behavior.

To give you a hint: whether unquoted wildcards work or not depends on
whether files in the directory where you run the above commands do or
do not have files that match the wildcards you give to `find'.  That's
because the startup code which tries to expand wildcards leaves them
intact if they don't match anything in the current directory, which
has the same effect as quoting them.

Take for example the following command:

  find c:/emacs -name *l.el

Assuming c:/emacs is the root of the Emacs source tree, try to type
this once from the c:/emacs/lisp directory and then from a directory
that doesn't have any *.el files, and you will see different behavior
(the first one should err out).

That's why I said that you _must_ quote wildcards to get predictable
behavior from `find'.

Re: Using `call-process-shell-command' in `process-lines'

[Lennart Borgman (gmail)]

Eli Zaretskii wrote:
>> Date: Tue, 27 Nov 2007 00:04:02 +0200
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: lekktu@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>>>>>    find -name *l.el
>>>>>    find -name m*.el
>>>>>    find -name "*.el"
>>>> Sheer luck, the first tow ones.  You _must_ quote the wildcard to get
>>>> predictable behavior.
> 
> To give you a hint: whether unquoted wildcards work or not depends on
> whether files in the directory where you run the above commands do or
> do not have files that match the wildcards you give to `find'.  That's
> because the startup code which tries to expand wildcards leaves them
> intact if they don't match anything in the current directory, which
> has the same effect as quoting them.
> 
> Take for example the following command:
> 
>   find c:/emacs -name *l.el
> 
> Assuming c:/emacs is the root of the Emacs source tree, try to type
> this once from the c:/emacs/lisp directory and then from a directory
> that doesn't have any *.el files, and you will see different behavior
> (the first one should err out).

Actually that does not happen to me. You may of course still be right 
that quoting is required, but I have never argued that it is not 
required. I am just trying to tell how things work and obviously that 
may be important since the assumption you made is not entirely correct.

> That's why I said that you _must_ quote wildcards to get predictable
> behavior from `find'.
> 

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> Date: Mon, 26 Nov 2007 18:28:12 +0100
> From: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,
> 	Emacs Devel <emacs-devel@gnu.org>
> 
> In a cmd.exe shell I can execute
> 
>     find -name "*.el"
> 
> with desired result (using gnuwin32 find.exe), but
> 
>     find -name *.el
> 
> fails with
> 
>     find: paths must precede expression
>     Usage: find [path...] [expression]
> 
> Doesn't it look like arg passing in find.exe is a bit strange?

No, it's not a problem with find.exe, it's a problem with the function
spawnve (called by call-process) in the Windows runtime library.

Unlike on Posix platforms, where the sibling function execve passes
the separate command-line arguments directly to the child process (in
this case, find), the Windows implementation first constructs a single
command-line string from all the separate arguments, and then passes
the result to the Windows API CreateProcess.  The concatenation of the
arguments is mandatory, since CreateProcess requires a single string
command, but the misfeature of spawnve is that it does not quote the
separate arguments as appropriate while concatenating them, and thus
the semantics of some of the arguments changes.

We could fix that by wrapping spawnve with code that quotes arguments
which need quoting.  I have working code somewhere that I can dust off
(bumped into this problem while porting RCS to Windows).  Then the
application Lisp code could be left intact.

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> Date: Mon, 26 Nov 2007 18:01:23 +0100
> From: "Juanma Barranquero" <lekktu@gmail.com>
> Cc: Emacs Devel <emacs-devel@gnu.org>
> 
> If I manually execute
> 
>   (apply #'call-process "c:/bin/gnuwin32/bin/find.exe" nil
> (get-buffer-create "output") nil '("c:/emacs/trunk/lisp/" "-name"
> "*.el"))
> 
> I get:
> 
>   c:\bin\gnuwin32\bin\find.exe: paths must precede expression
>   Usage: c:\bin\gnuwin32\bin\find.exe [-H] [-L] [-P] [path...] [expression]

If this minor variant works:

   (apply #'call-process "c:/bin/gnuwin32/bin/find.exe" nil
     (get-buffer-create "output") nil '("c:/emacs/trunk/lisp/" "-name"
     (shell-quote-argument "*.el")))

it might give you a clue to the solution.  The call to
shell-quote-argument should be limited to Windows only, since the
Windows implementation of the library function called by call-process
constructs a single command line out of all arguments, which of course
means you need an extra pair of quotes.

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> Date: Mon, 26 Nov 2007 22:23:09 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> If this minor variant works:
> 
>    (apply #'call-process "c:/bin/gnuwin32/bin/find.exe" nil
>      (get-buffer-create "output") nil '("c:/emacs/trunk/lisp/" "-name"
>      (shell-quote-argument "*.el")))

Sorry, you need one more change:

   (apply #'call-process "c:/bin/gnuwin32/bin/find.exe" nil
     (get-buffer-create "output") nil (list "c:/emacs/trunk/lisp/" "-name"
     (shell-quote-argument "*.el")))

Re: Using `call-process-shell-command' in `process-lines'

[Lennart Borgman (gmail)]

Eli Zaretskii wrote:
>> Date: Mon, 26 Nov 2007 18:01:23 +0100
>> From: "Juanma Barranquero" <lekktu@gmail.com>
>> Cc: Emacs Devel <emacs-devel@gnu.org>
>>
>> If I manually execute
>>
>>   (apply #'call-process "c:/bin/gnuwin32/bin/find.exe" nil
>> (get-buffer-create "output") nil '("c:/emacs/trunk/lisp/" "-name"
>> "*.el"))
>>
>> I get:
>>
>>   c:\bin\gnuwin32\bin\find.exe: paths must precede expression
>>   Usage: c:\bin\gnuwin32\bin\find.exe [-H] [-L] [-P] [path...] [expression]
> 
> If this minor variant works:
> 
>    (apply #'call-process "c:/bin/gnuwin32/bin/find.exe" nil
>      (get-buffer-create "output") nil '("c:/emacs/trunk/lisp/" "-name"
>      (shell-quote-argument "*.el")))
> 
> it might give you a clue to the solution.  The call to
> shell-quote-argument should be limited to Windows only, since the
> Windows implementation of the library function called by call-process
> constructs a single command line out of all arguments, which of course
> means you need an extra pair of quotes.


Maybe you mean something like

   (apply #'call-process "find.exe" nil
          (get-buffer-create "output")
          nil
          (list "c:/emacs-lisp/old/" "-name"
                (shell-quote-argument "*.el" )))

but it does not work AFAICS.

There might be some clues in the message I sent before.

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> Date: Mon, 26 Nov 2007 21:31:03 +0100
> From: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>
> CC: Juanma Barranquero <lekktu@gmail.com>, monnier@iro.umontreal.ca, 
>  emacs-devel@gnu.org
> 
> Maybe you mean something like
> 
>    (apply #'call-process "find.exe" nil
>           (get-buffer-create "output")
>           nil
>           (list "c:/emacs-lisp/old/" "-name"
>                 (shell-quote-argument "*.el" )))

Yes, sorry; I tried to correct myself in another message.

> but it does not work AFAICS.

??? It does for me.

Re: Using `call-process-shell-command' in `process-lines'

[Lennart Borgman (gmail)]

Eli Zaretskii wrote:
>> Date: Mon, 26 Nov 2007 21:31:03 +0100
>> From: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>
>> CC: Juanma Barranquero <lekktu@gmail.com>, monnier@iro.umontreal.ca, 
>>  emacs-devel@gnu.org
>>
>> Maybe you mean something like
>>
>>    (apply #'call-process "find.exe" nil
>>           (get-buffer-create "output")
>>           nil
>>           (list "c:/emacs-lisp/old/" "-name"
>>                 (shell-quote-argument "*.el" )))
> 
> Yes, sorry; I tried to correct myself in another message.
> 
>> but it does not work AFAICS.
> 
> ??? It does for me.

It doesn't for me, just tested with the unpatched version (because I did 
not remember if I already patched spawnve or if I thrown away that patch 
long ago).

But my unpatched version is from 2007-07-04.

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> Date: Mon, 26 Nov 2007 22:08:17 +0100
> From: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>
> CC: lekktu@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> >>    (apply #'call-process "find.exe" nil
> >>           (get-buffer-create "output")
> >>           nil
> >>           (list "c:/emacs-lisp/old/" "-name"
> >>                 (shell-quote-argument "*.el" )))
> > 
> > Yes, sorry; I tried to correct myself in another message.
> > 
> >> but it does not work AFAICS.
> > 
> > ??? It does for me.
> 
> It doesn't for me

Please describe how it fails.

Re: Using `call-process-shell-command' in `process-lines'

[Lennart Borgman (gmail)]

Eli Zaretskii wrote:
>> Date: Mon, 26 Nov 2007 22:08:17 +0100
>> From: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>
>> CC: lekktu@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>>>>    (apply #'call-process "find.exe" nil
>>>>           (get-buffer-create "output")
>>>>           nil
>>>>           (list "c:/emacs-lisp/old/" "-name"
>>>>                 (shell-quote-argument "*.el" )))
>>> Yes, sorry; I tried to correct myself in another message.
>>>
>>>> but it does not work AFAICS.
>>> ??? It does for me.
>> It doesn't for me
> 
> Please describe how it fails.

   (apply #'call-process "find.exe" nil
          (get-buffer-create "output")
          nil
          (list "c:/emacs-lisp/old/"
                "-name"
                (shell-quote-argument "*.el" )
                ))

gives nothing in the "output" buffer and returns 0.

   (apply #'call-process "find.exe" nil
          (get-buffer-create "output")
          nil
          (list "c:/emacs-lisp/old/"
                "-name"
                "m*.el"
                ))

gives the correct output and returns 0.

   (apply #'call-process "find.exe" nil
          (get-buffer-create "output")
          nil
          (list "c:/emacs-lisp/old/"
                "-name"
                "*.el"
                ))

gives the error "paths must precede expressions" and returns 1.

   (apply #'call-process "find.exe" nil
          (get-buffer-create "output")
          nil
          (list "c:/emacs-lisp/old/"
                (shell-quote-argument "-name")
                (shell-quote-argument "*.el" )
                ))

matches all files (not only *.el) and returns 1.

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> Date: Mon, 26 Nov 2007 22:23:18 +0100
> From: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>
> CC: lekktu@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
>    (apply #'call-process "find.exe" nil
>           (get-buffer-create "output")
>           nil
>           (list "c:/emacs-lisp/old/"
>                 "-name"
>                 (shell-quote-argument "*.el" )
>                 ))
> 
> gives nothing in the "output" buffer and returns 0.

Well, ``return 0'' and ``fails'' are not exactly the same.  The
original complaint by Juanma was that a non-zero status was returned.

Anyway, I see now that I missed something important: we already have a
wrapper for spawnve (w32proc.c:sys_spawnve), and it already tries to
quote arguments.  In addition, it invokes the command thru cmdproxy.
So somewhere between the quoting code and cmdproxy there's a bug that
causes `find' to not find anything.  AFAICS, quoting *.el with
shell-quote-argument causes it to wind up quoted in `find's argv[]
array, which explains why it finds nothing.  Someone(tm) needs to
debug this.

Re: Using `call-process-shell-command' in `process-lines'

[Jason Rumney]

Eli Zaretskii wrote:
> Anyway, I see now that I missed something important: we already have a
> wrapper for spawnve (w32proc.c:sys_spawnve), and it already tries to
> quote arguments.

But not in the presence of wildcards, only whitespace.

The version of find that I have (4.2.20 from GnuWin32) works with the
original test case, so I can't test this (other than to verify that find
still works OK after the patch), but it seems like the following patch
might help:

*** w32proc.c.~1.73.~    2007-10-10 22:46:40.593750000 +0100
--- w32proc.c    2007-11-26 21:33:19.609375000 +0000
***************
*** 781,787 ****
       variable in their environment.  */
    char ppid_env_var_buffer[64];
    char *extra_env[] = {ppid_env_var_buffer, NULL};
!   char *sepchars = " \t";
 
    /* We don't care about the other modes */
    if (mode != _P_NOWAIT)
--- 781,787 ----
       variable in their environment.  */
    char ppid_env_var_buffer[64];
    char *extra_env[] = {ppid_env_var_buffer, NULL};
!   char *sepchars = " \t*?";
 
    /* We don't care about the other modes */
    if (mode != _P_NOWAIT)
***************
*** 882,888 ****
 
    /* Cygwin apps needs quoting a bit more often */
    if (escape_char == '"')
!     sepchars = "\r\n\t\f '";
 
    /* do argv...  */
    arglen = 0;
--- 882,888 ----
 
    /* Cygwin apps needs quoting a bit more often */
    if (escape_char == '"')
!     sepchars = "\r\n\t\f '*?";
 
    /* do argv...  */
    arglen = 0;


Diffs between working revision and workfile end here.

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> Date: Mon, 26 Nov 2007 22:17:02 +0000
> From: Jason Rumney <jasonr@gnu.org>
> Cc: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>,
> 	lekktu@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> The version of find that I have (4.2.20 from GnuWin32) works with the
> original test case

Try invoking the original test case from a buffer whose default
directory has at least 2 files that match the wildcard *.el, and I
think you will see the failure.

Re: Using `call-process-shell-command' in `process-lines'

[Jason Rumney]

Jason Rumney wrote:
> The version of find that I have (4.2.20 from GnuWin32) works with the
> original test case
Correction, it was working because the current directory of the
*scratch* buffer did not contain any *.el files.

The patch I posted does seem to fix the problem.

Re: Using `call-process-shell-command' in `process-lines'

[Juanma Barranquero]

On Nov 26, 2007 11:30 PM, Jason Rumney <jasonr@gnu.org> wrote:

> The patch I posted does seem to fix the problem.

Yes, it does.

Thanks,

             Juanma

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> Date: Mon, 26 Nov 2007 22:17:02 +0000
> From: Jason Rumney <jasonr@gnu.org>
> Cc: "Lennart Borgman (gmail)" <lennart.borgman@gmail.com>,
> 	lekktu@gmail.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> *** w32proc.c.~1.73.~    2007-10-10 22:46:40.593750000 +0100
> --- w32proc.c    2007-11-26 21:33:19.609375000 +0000
> ***************
> *** 781,787 ****
>        variable in their environment.  */
>     char ppid_env_var_buffer[64];
>     char *extra_env[] = {ppid_env_var_buffer, NULL};
> !   char *sepchars = " \t";
>  
>     /* We don't care about the other modes */
>     if (mode != _P_NOWAIT)
> --- 781,787 ----
>        variable in their environment.  */
>     char ppid_env_var_buffer[64];
>     char *extra_env[] = {ppid_env_var_buffer, NULL};
> !   char *sepchars = " \t*?";

What about <, |, and > -- should we quote them as well?

Re: Using `call-process-shell-command' in `process-lines'

[Jason Rumney]

Eli Zaretskii wrote:
> What about <, |, and > -- should we quote them as well?

Those seem to get through to the subprocess OK, since they are handled
by the shell even on DOS/Windows.
I can't get the Cygwin version working (I tried both escaping and
quoting wildcards), so I will check in a change just for the standard
definition of sepchars.

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> Date: Mon, 26 Nov 2007 23:01:22 +0000
> From: Jason Rumney <jasonr@gnu.org>
> Cc: lekktu@gmail.com, lennart.borgman@gmail.com,
> 	monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> Eli Zaretskii wrote:
> > What about <, |, and > -- should we quote them as well?
> 
> Those seem to get through to the subprocess OK, since they are handled
> by the shell even on DOS/Windows.

What command did you try to run?

The example I had in mind was when these characters are not used for
redirection, but are actually part of one of the command-line
arguments, and need to wind up verbatim in the subprocess.  On Posix
platforms, execve will do that automatically, since it doesn't go
through a shell or any similar facilities for which these characters
are special.

Re: Using `call-process-shell-command' in `process-lines'

[Stefan Monnier]

> If this minor variant works:

>    (apply #'call-process "c:/bin/gnuwin32/bin/find.exe" nil
>      (get-buffer-create "output") nil '("c:/emacs/trunk/lisp/" "-name"
>      (shell-quote-argument "*.el")))

> it might give you a clue to the solution.  The call to
> shell-quote-argument should be limited to Windows only, since the
> Windows implementation of the library function called by call-process
> constructs a single command line out of all arguments, which of course
> means you need an extra pair of quotes.

But this library function already does the necessary quoting.
The problem seems to be that under Windows, the globbing is not done by
the shell but by the application, so while under Unix no globbing is
performed (because no shell is involved) under Windows `find'
performs globbing.

I'd be surprised if it's the first time this happens.  What solutions
have we used in the other cases where this problem showed up?


        Stefan

Re: Using `call-process-shell-command' in `process-lines'

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: "Juanma Barranquero" <lekktu@gmail.com>,  emacs-devel@gnu.org
> Date: Mon, 26 Nov 2007 15:35:51 -0500
> 
> > If this minor variant works:
> 
> >    (apply #'call-process "c:/bin/gnuwin32/bin/find.exe" nil
> >      (get-buffer-create "output") nil '("c:/emacs/trunk/lisp/" "-name"
> >      (shell-quote-argument "*.el")))
> 
> > it might give you a clue to the solution.  The call to
> > shell-quote-argument should be limited to Windows only, since the
> > Windows implementation of the library function called by call-process
> > constructs a single command line out of all arguments, which of course
> > means you need an extra pair of quotes.
> 
> But this library function already does the necessary quoting.

??? What library function do you have in mind, and where do you see
its quoting?

> The problem seems to be that under Windows, the globbing is not done by
> the shell but by the application

True.  But the result is the same in most cases.  Note that ``by the
application'' means here the wildcards are expanded by the startup
code that runs before the `main' function.  And `find' clearly wants
to expand the wildcards itself, in the code invoked by its own `main'.

> so while under Unix no globbing is
> performed (because no shell is involved) under Windows `find'
> performs globbing.

Yes, and therein lies the misfeature I described earlier: since
spawnve _knows_ it will be invoking the child program in a way that
might expand wildcards before the wildcards get to the application
code, it _must_ quote wildcards (and other special characters,
including whitespace), so as to emulate the semantics of the Posix
execve.

> I'd be surprised if it's the first time this happens.  What solutions
> have we used in the other cases where this problem showed up?

I'm guessing that other invocations of `find', for example, leave it
to the user to type arguments, so Windows users quote the wildcards,
as they are accustomed to do from the shell prompt.

Anyway, the right solution, IMO, is to wrap spawnve with quoting code.

Re: Using `call-process-shell-command' in `process-lines'

[Lennart Borgman (gmail)]

Eli Zaretskii wrote:
> Anyway, the right solution, IMO, is to wrap spawnve with quoting code.


But would not that result in the same call as

   (apply #'call-process "find.exe" nil
          (get-buffer-create "output")
          nil
          (list "c:/emacs-lisp/old/"
                (shell-quote-argument "-name")
                (shell-quote-argument "*.el" )
                ))

which matches all files for me? Or

Perhaps are we using different find.exe? Mine says "GNU find version 
4.1" and is from gnuwin32.

Re: Using `call-process-shell-command' in `process-lines'

[Juanma Barranquero]

On Nov 26, 2007 4:35 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> Yes: the use of `args' in call-process-shell-command is strongly
> discouraged in my office because it doesn't do what it says it does: it
> doesn't treat each element of the list as an argument.  Instead it just
> concatenates the elements of the list and then passes the resulting
> single string to the shell which will split it into arguments accoring
> to its own rules.

Aside: that shouldn't cause compatibility problems, because
`process-lines' wasn't in subr.el until 2007-11-19; I don't think
there were many users of `process-line' from admin/admin.el or of
`authors-process-lines' in emacs-lisp/authors.el. But I could be
wrong...

             Juanma