This afternoon, Stefan Reichör wrote:
> Stephen Leake <[EMAIL PROTECTED]> writes:
>
>> 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.
>
> Stephen, thanks for the comprehensive analysis!
>
> I have put Edgar as cc receiver.
>
> Edgar do you still use dvc on windows?
> If yes, could you please try the instructions from Stephen.
>
> If it works this way, we should remove the (only half working) patch.

Hi! I'm still using DVC, but I stopped following its development for a
while. I've just retrieved the latest dev version, removed my patch, as you
sugested and yes, I confirm that bzr.exe now runs without problems. I remember
my patch to be needed in bzr's very early stages! My only configuration is now
the following:

(require 'dvc-autoloads)
(setq bzr-executable "bzr")


And all the magic is done properly. So yes, you should have the patch in
question removed!



>
>
> Stefan.

-- 
Edgar Gonçalves

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

Reply via email to