Stefan Reichör <[EMAIL PROTECTED]> writes:

>> (defun dvc-build-dvc-command (dvc list-args)
>>   "Build a shell command to run DVC with args LIST-ARGS.
>> DVC can be one of 'baz, 'xhg, ..."
>>   (let* ((executable (dvc-variable dvc "executable"))
>>          (cmd (mapconcat 'shell-quote-argument
>>                          (cons executable
>>                                (delq nil list-args))
>>                          " ")))
>>     (when (eq system-type 'windows-nt)
>>        (setq cmd (replace-regexp-in-string "\\\"" "" cmd)))
>>     cmd))
>>
>> If I comment out the (when windows-nt ...) part, the unit tests work
>> fine. That part seems quite odd; it is undoing the quoting that was
>> just done.
>>
>> Why was it put in?
>
> I don't use windows regulary.
>
> The discussion that resulted in the current functionality can be found here:
> http://thread.gmane.org/gmane.emacs.xtla.devel/1738

There are several related issues.

The core problem is that dvc-run-dvc-async wants to capture stderr as
well as stdout from the child processes. Emacs 'start-process' does
not allow that, and in fact does not say what happens to stderr.
Perhaps we could request that Emacs add a variant of start-process
that captures stderr in a separate buffer, but that won't happen for a
long time, if at all.

So dvc-run-dvc-async explicitly spawns "sh", rather than the backend
executable.

This means two things:

1) The user must have "sh" on their machine.

    You can run Emacs on Windows without a POSIX shell, and some parts
    of Emacs will cope nicely. But there are many other places in
    Emacs that also have "sh" hardcoded, so to get the full benefit,
    you usually need a POSIX shell.

    There are two POSIX shells easily available; MinGW and Cygwin.
    There are also others. So it's not too bad to require this, but it
    would be nice if it could be relaxed.

2) The arguments need to be quoted.

    File-name arguments need to be quoted, because they might have
    spaces. Other arguments might have shell meta-characters, and thus
    need to be quoted. I assume that's why dvc-run-dvc-async applies
    shell-quote-argument to all arguments. It should not hurt to quote
    arguments that don't really need to be quoted.

    Note that if we were spawning the backend executable directly, the
    arguments would _not_ need to be quoted; each lisp argument turns
    into one process argument.

The quoting can be done using POSIX or DOS syntax. This is controlled
by the name of the shell, which is determined by the variable
explicit-shell-file-name, environment variables, and defaults (in that
order). Examples:

(let ((explicit-shell-file-name "c:/bin/bash.exe"))
  (shell-quote-argument "c:/Documents and Settings"))
"c\\:/Documents\\ and\\ Settings"

(let ((explicit-shell-file-name "cmdproxy.exe"))
  (shell-quote-argument "c:/Documents and Settings"))
"\"c:/Documents and Settings\""

Note that the single '\' characters are just elisp display format, not
part of the string actually sent to the subprocess.

On the other hand, DOS syntax works for POSIX shells, so this choice
doesn't actually matter! There may be cases where it does matter, but
I've never run into one.

I don't have explicit-shell-file-name set, which means I get DOS
syntax for all uses of shell-quote-argument, and have never had a
problem, even though I never run a DOS shell.

Edgar (the original poster on the gmane thread referenced above) was
running bzr from a DOS batch file. That means he was spawning "sh",
which performed the stderr redirection; that spawned "cmd.exe", which
did its own argument parsing, and then that spawned Python, which did
yet more parsing. I'm not clear why that process was confused by the
quotes, but it's not too surprising.

The bzr I have installed today does not need a batch file, nor does it
call Python directly; it has 'bzr.exe', which can be run from 'sh'
directly.

As I discovered, the patch Edgar advocated in fact removes any
quoting. He's just lucky he was not using paths with spaces, or a
backend that requires shell metacharacters in its arguments.

So I believe the correct fix is:

1) Remove Edgar's patch; dvc-build-dvc-command should apply
   shell-quote-argument to all arguments, and not modify the result.

2) Document that DVC users need "sh" on Windows, and point to MinGW
   and Cygwin.

3) Document how to run the supported backends (ie, _not_ thru a DOS
   batch file).

4) Point out the variable explicit-shell-file-name, which may be of
   use for new backends or other problems.

I can add this to dvc.texinfo, if others agree with this approach.

-- 
-- Stephe

_______________________________________________
Dvc-dev mailing list
[email protected]
https://mail.gna.org/listinfo/dvc-dev

Reply via email to