Tests don't find `bzr` on Windows

[msabramo]

I am not sure yet if this was broken before or was introduced by 8010608, but the tests fail to find bzr on Windows, even though I installed it and can run it from the command prompt.

(pip) C:\Users\Administrator\Development\Python\pip>tox -e py27 -- tests/functional/test_freeze.py -k test_freeze_bazaar_clone --tb=short
GLOB sdist-make: C:\Users\Administrator\Development\Python\pip\setup.py
py27 inst-nodeps: C:\Users\Administrator\Development\Python\pip\.tox\dist\pip-6.1.0.dev0.zip
py27 runtests: PYTHONHASHSEED='762'
py27 runtests: commands[0] | py.test --timeout 300 tests\functional\test_freeze.py -k test_freeze_bazaar_clone --tb=short
============================= test session starts =============================
platform win32 -- Python 2.7.5 -- py-1.4.26 -- pytest-2.6.4
plugins: capturelog, cov, timeout, xdist
collected 8 items

tests/functional/test_freeze.py F

================================== FAILURES ===================================
__________________________ test_freeze_bazaar_clone ___________________________
tests\functional\test_freeze.py:231: in test_freeze_bazaar_clone
    tmpdir.join("cache"),
tests\lib\local_repos.py:82: in local_checkout
    return _get_vcs_and_checkout_url(remote_repo, directory)
tests\lib\local_repos.py:68: in _get_vcs_and_checkout_url
    vcs_class(remote_repository).obtain(destination_path)
pip\vcs\bazaar.py:75: in obtain
    self.run_command(['branch', '-q'] + rev_options + [url, dest])
pip\vcs\__init__.py:318: in run_command
    raise BadCommand('Cannot find command %r' % self.name)
E   BadCommand: Cannot find command 'bzr'
-------------------------------- Captured log ---------------------------------
bazaar.py                   73 INFO     Checking out http://bazaar.launchpad.net/%7Edjango-wikiapp/django-wikiapp/release-0.1 to c:\
users\admini~1\appdata\local\temp\pytest-10\test_freeze_bazaar_clone0\cache\release-0.1
__init__.py                677 DEBUG    Running command bzr branch -q http://bazaar.launchpad.net/%7Edjango-wikiapp/django-wikiapp/r
elease-0.1 c:\users\admini~1\appdata\local\temp\pytest-10\test_freeze_bazaar_clone0\cache\release-0.1
__init__.py                687 CRITICAL Error [Error 2] The system cannot find the file specified while executing command bzr branch
 -q http://bazaar.launchpad.net/%7Edjango-wikiapp/django-wikiapp/release-0.1 c:\users\admini~1\appdata\local\temp\pytest-10\test_fre
eze_bazaar_clone0\cache\release-0.1
============= 7 tests deselected by '-ktest_freeze_bazaar_clone' ==============
=================== 1 failed, 7 deselected in 8.65 seconds ====================
ERROR: InvocationError: 'C:\\Users\\Administrator\\Development\\Python\\pip\\.tox\\py27\\Scripts\\py.test.EXE --timeout 300 tests\\f
unctional\\test_freeze.py -k test_freeze_bazaar_clone --tb=short'
____________________________________________________________ summary _____________________________________________________________
ERROR:   py27: commands failed

(pip) C:\Users\Administrator\Development\Python\pip>bzr
Bazaar 2.5.1 -- a free distributed version-control tool
http://bazaar.canonical.com/

Basic commands:
  bzr init           makes this directory a versioned branch
  bzr branch         make a copy of another branch

  bzr add            make files or directories versioned
  bzr ignore         ignore a file or pattern
  bzr mv             move or rename a versioned file
...

Cc: @pfmoore

Part of the larger effort of getting the tests to work well on Windows (#2520)

[pfmoore]

Hmm, I don't have a Bazaar installation (and I don't have time or inclination to get one and install right now) but can you tell me the exact filename that the bzr command runs? Is it C:\Some\Path\bzr.exe? Or is it a script of some sort? If it's a .py script, Popen won't run it...

[msabramo]

I didn't have bzr in my Windows VM, but it was trivial to download and install using the installer from http://wiki.bazaar.canonical.com/WindowsDownloads

That said:

(pip) C:\Users\Administrator\Development\Python\pip>where bzr
C:\Python27\Scripts\bzr
C:\Python27\Scripts\bzr.bat

I am guessing that the PATH environment variable is not getting passed through. Or will Popen not open a .bat file?

[msabramo]

Hmmm, if I make this change then it works:

diff --git a/pip/vcs/bazaar.py b/pip/vcs/bazaar.py
index 708efe4..e649e54 100644
--- a/pip/vcs/bazaar.py
+++ b/pip/vcs/bazaar.py
@@ -20,7 +20,7 @@ logger = logging.getLogger(__name__)


 class Bazaar(VersionControl):
-    name = 'bzr'
+    name = 'bzr.bat'
     dirname = '.bzr'
     repo_name = 'branch'
     schemes = (

See:

(pip) C:\Users\Administrator\Development\Python\pip>tox -e py27 -- -k test_freeze_bazaar_clone --tb=short --pdb
GLOB sdist-make: C:\Users\Administrator\Development\Python\pip\setup.py
py27 inst-nodeps: C:\Users\Administrator\Development\Python\pip\.tox\dist\pip-6.1.0.dev0.zip
py27 runtests: PYTHONHASHSEED='442'
py27 runtests: commands[0] | py.test --timeout 300 -k test_freeze_bazaar_clone --tb=short --pdb
============================= test session starts =============================
platform win32 -- Python 2.7.5 -- py-1.4.26 -- pytest-2.6.4
plugins: capturelog, cov, timeout, xdist
collected 492 items

tests/functional/test_freeze.py .

============ 491 tests deselected by '-ktest_freeze_bazaar_clone' =============
============ 1 passed, 491 deselected, 1 warnings in 13.08 seconds ============
____________________________________________________________ summary _____________________________________________________________
  py27: commands succeeded
  congratulations :)

My fear is that we just discovered the reason for the find_command stuff that you removed in 8010608 (584b2d0)...

[msabramo]

Reverting that change also fixes the problem.

(pip) C:\Users\Administrator\Development\Python\pip>git revert 584b2d0f820b58fc052866c3bfa208e463b04682
[test_freeze_mercurial_clone_remove_dep_on_bitbucket 9ee4f8d] Revert "Remove find_command - let subprocess.Popen search PATH for the
 VCS command to run"
 8 files changed, 221 insertions(+), 94 deletions(-)

(pip) C:\Users\Administrator\Development\Python\pip>tox -e py27 -- -k test_freeze_bazaar_clone --tb=short --pdb
GLOB sdist-make: C:\Users\Administrator\Development\Python\pip\setup.py
py27 inst-nodeps: C:\Users\Administrator\Development\Python\pip\.tox\dist\pip-6.1.0.dev0.zip
py27 runtests: PYTHONHASHSEED='306'
py27 runtests: commands[0] | py.test --timeout 300 -k test_freeze_bazaar_clone --tb=short --pdb
============================= test session starts =============================
platform win32 -- Python 2.7.5 -- py-1.4.26 -- pytest-2.6.4
plugins: capturelog, cov, timeout, xdist
collected 496 items

tests/functional/test_freeze.py .

============ 495 tests deselected by '-ktest_freeze_bazaar_clone' =============
============ 1 passed, 495 deselected, 1 warnings in 12.31 seconds ============
____________________________________________________________ summary _____________________________________________________________
  py27: commands succeeded
  congratulations :)

[pfmoore]

That seems to me like a serious bug in Popen:

>dir progs                                                                                                                      


    Directory: C:\Work\Scratch\x\progs                                                                                          


Mode                LastWriteTime     Length Name                                                                               
----                -------------     ------ ----                                                                               
-a---        12/03/2015     08:03         42 foo                                                                                
-a---        12/03/2015     08:03         33 foo.bat                                                                            

>$env:PATH += ";C:\Work\Scratch\x\progs"                                                                                        
>foo                                                                                                                            
Hello, world from foo.bat                                                                                                       
>python -c "import subprocess; subprocess.Popen(['foo'])"                                                                       
Traceback (most recent call last):                                                                                              
  File "<string>", line 1, in <module>                                                                                          
  File "C:\Apps\Python34\lib\subprocess.py", line 859, in __init__                                                              
    restore_signals, start_new_session)                                                                                         
  File "C:\Apps\Python34\lib\subprocess.py", line 1112, in _execute_child                                                       
    startupinfo)                                                                                                                
FileNotFoundError: [WinError 2] The system cannot find the file specified                                                       

I'll need to look into this some more. Reverting the change to remove find_command will solve this, but at the expense of causing issues if there were a bzr.py file on PATH. While breaking a known case is more of an issue than breaking a theoretical case, neither is good :-(

[piotr-dobrogost]

My fear is that we just discovered the reason for the find_command stuff that you removed in 8010608

:)

[pfmoore]

This looks like a bug in the Windows CreateProcess API. I've reproduced it in C code :-(

What I'll probably do if I can't find a better fix is introduce a very limited search for the executable, only on Windows and only looking for exactly what CreateProcess accepts (i.e. no complex PATHEXT checking which won't work anyway...). But I'm still investigating at the moment.

[pfmoore]

BTW, I don't think that's the reason for find_command being in there. It may have resulted in us not triggering this bug, but I doubt someone found the bug and created find_command as a fix...

(Not that I'm making excuses at all, of course ;-))

[msabramo]

We can ask @jezdez what the motivation was for find_command, but I wouldn't be surprised if he doesn't remember, since it was back in 2009:

5b49777#diff-abb7fb849a00297ea347b89aa9fa79acR4130

[pfmoore]

Wow, at that point pip was a single file, pip.py...

[pfmoore]

... (Still investigating)

Sigh. Actually, this is just yet another reason why people need to use proper .exe script wrappers for their Python commands. CreateProcess (and hence Popen) won't add any extension other than .exe when searching for a command. If you specify an extension, CreateProcess will use it, but as far as I've been able to determine, anything other than .exe, .com, .bat and .cmd fail with the error "OSError: [WinError 193] %1 is not a valid Win32 application".

So I think what I'll probably do is a path search for those four specific extensions only (no PATHEXT search, and only if the command has no extension). But only on Windows.

I'd argue that this is a bug in bzr as well, though. Using anything other than an actual .exe as a command has all sorts of odd corner case failures, this is only one of them. (Try using the "bzr" command in a .bat file if you want to see another...)

[msabramo]

Sounds like a reasonable plan to me.

[pfmoore]

Sadly, it's not :-( The docs for CreateProcess (https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425%28v=vs.85%29.aspx) say:

To run a batch file, you must start the command interpreter; set lpApplicationName to cmd.exe and set lpCommandLine to the following arguments: /c plus the name of the batch file.

But if we do that we hit all sorts of problems because we have to convert the command from a list of args to a quoted string that cmd.exe will parse correctly without completely screwing the whole flaming thing up again :-(

My tests seem to show that you can actually supply the name of a .bat file with the extension given, and it will be searched for on PATH and executed "properly". But that seems to be completely undocumented behaviour.

I think the least wrong thing to do is probably just to add a hack into the ENOENT processing - if we're on Windows, try again using (self.name + '.bat') and then if that still fails, one more time with (self.name + '.cmd'). I'll have to cache the variation that works to make sure this isn't horrendously slow, but it's the best I can think of without introducing a load more potential issues.

I'll also have to raise a bug report against Bazaar, but that needs a Launchpad account and I don't have time to get that sorted right now...