Shell commands fail on Windows due to popen shell=False in Action._popen

[pytoxbot]

• Bitbucket: https://bitbucket.org/hpk42/tox/issue/257
• Originally reported by: @stefano-m
• Originally created at: 2015-06-16T17:33:31.977

Hello,

I am using tox 2.0.2 and Python 2.7 on Windows 7.

As part of my tox tests, I need to run gulp and other npm-related commands.

To do so, I first run npm install and then e.g. run gulp that has been installed in the node_modules directory.

The relevant part of my tox.ini looks like

[testenv]
whitelist_externals = npm
commands =
                     npm install
                     {toxinidir}/node_modules/.bin/gulp

When I run tox, I get the following error when gulp is run:

py27-win32 runtests: commands[0] | npm install
py27-win32 runtests: commands[1] | F:\my_project\my_project/node_modules/.bin/gulp
ERROR: invocation failed (errno 8), args: ['F:\\my_project\\my_project/node_modules/.bin/gulp'], cwd: F:\my_project\my_project
Traceback (most recent call last):
  File "C:\Python27\lib\runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "C:\Python27\lib\runpy.py", line 72, in _run_code
    exec code in run_globals
  File "C:\Python27\Scripts\tox.exe\__main__.py", line 9, in <module>
  File "C:\Python27\lib\site-packages\tox\session.py", line 39, in main
    retcode = Session(config).runcommand()
  File "C:\Python27\lib\site-packages\tox\session.py", line 367, in runcommand
    return self.subcommand_test()
  File "C:\Python27\lib\site-packages\tox\session.py", line 540, in subcommand_test
    self.runtestenv(venv)
  File "C:\Python27\lib\site-packages\tox\session.py", line 548, in runtestenv
    venv.test(redirect=redirect)
  File "C:\Python27\lib\site-packages\tox\venv.py", line 360, in test
    ignore_ret=ignore_ret)
  File "C:\Python27\lib\site-packages\tox\venv.py", line 384, in _pcall
    return action.popen(args, cwd=cwd, env=env, redirect=redirect, ignore_ret=ignore_ret)
  File "C:\Python27\lib\site-packages\tox\session.py", line 130, in popen
    stdout=stdout, stderr=STDOUT)
  File "C:\Python27\lib\site-packages\tox\session.py", line 218, in _popen
    stdout=stdout, stderr=stderr, env=env)
  File "C:\Python27\lib\subprocess.py", line 710, in __init__
    errread, errwrite)
  File "C:\Python27\lib\subprocess.py", line 958, in _execute_child
    startupinfo)
WindowsError: [Error 193] %1 is not a valid Win32 application

The same configuration works OK on Linux.

I think that the problem lies in Action._popen in the session.py module, where self.session.popen is called (line 216).

popen is run with the shell keyword argument set to False, and causes the command to fall over on Windows.

Since I had already noticed that problem (on Windows only) in other places where I use subprocess, I tried to change the value to True by doing

    return self.session.popen(args, shell=(sys.platform == 'win32'), cwd=str(cwd),
                                             universal_newlines=True,
                                             stdout=stdout, stderr=stderr, env=env)

and that solved the issue (while keeping the default value to False on non-Windows platforms).

[pytoxbot]

Original comment by @hpk42

i guess you need to look at the venv.py and tests/test_venv.py in particular. First writing a test that fails is the hard bit, maybe.

[pytoxbot]

Original comment by @stefano-m

@hpk42 I might have some time to work on this, would you mind to give me pointers to the parts of tox's code I should look at and where I should put the tests?

Thanks.

[pytoxbot]

Original comment by @stefano-m

Reopening to keep this visible.

[pytoxbot]

Original comment by @stefano-m

@The-Compiler, I've verified that on Windows, if shell=False only commands with their extensions will work. I guess that shell=True runs the command through cmd.exe which, in turn expands the extension.

[pytoxbot]

Original comment by @stefano-m

A quick update.

This is an excerpt from my tox.ini that allows me to work around the problem and keep the GNU/Linux and Windows environments separated (karma is basically a JS test runner):

[jscommands:linux]
gulp = {toxinidir}/node_modules/.bin/gulp
karma = {toxinidir}/node_modules/.bin/karma

[jscommands:win32]
gulp = {[javascript:linux]gulp}.cmd
karma = {[javascript:linux]karma}.cmd

[testenv]
whitelist_externals = npm
commands =
         npm install
         py27-linux: {[jscommands:linux]gulp}
         py27-linux: {[jscommands:linux]karma} start --single-run
         py27-win32: {[jscommands:win32]gulp}
         py27-win32: {[jscommands:win32]karma} start --single-run

Of course, if tox could take care of the naming, it would be great.

[pytoxbot]

Original comment by @stefano-m

Ahh! I did not know that!

In fact, I was thinking about why npm install was working on the way home!

In that case, it might be great if tox could be enhanced to do that too.

Still, I feel that the specific issue that I opened is still "invalid". Should we open a new one with a clearer title and goal? Or would it be OK to reopen this one instead?

Thanks.

[pytoxbot]

Original comment by @hpk42

Wait a second -- tox does have machinery to find the correct .cmd/.bat etc. files if a command is on PATH (so e.g. "npm" could actually be "npm.exe" or "npm.cmd" and your tox.ini would work).
However, the {toxinidir}/node_modules/.bin/gulp bit is not using this logic because it's an absolute path. I wonder if we should enhance tox to care for the proper extension on windows also for absolute paths.

[pytoxbot]

Original comment by @stefano-m

Not an issue, see discussion for details.

Maybe some documentation can be added to avoid confusion to Windows users.

[pytoxbot]

Original comment by @stefano-m

Hi,

I've just tried to run the cmd version in my tox.ini and this makes Windows happy.

Oh well, I guess that we can discard my pull request and I will modify my tox.ini so that it'll run the .cmd version if on Windows.

Would it be worth adding something to the documentation about this?

Thank you very much for your super-fast feedback!

[pytoxbot]

Original comment by @The-Compiler

I guess cmd.exe prefers gulp.cmd to the shellscript if it exists, and subprocess (i.e. the Windows APIs) don't. These things are always... funny on Windows.

Have you tried actually invoking gulp.cmd instead of gulp in your tox.ini?

[pytoxbot]

Original comment by @hpk42

i also don't understand why shell is needed for gulp. At this point i don't think we want to change the invocation semantics without very clear reasons and probably even then only by a per-testenv switch or so.

[pytoxbot]

Original comment by @stefano-m

@The-Compiler,

npm install installs a number of node modules, some of which can be executed, e.g. gulp is a sort of make for JavaScript stuff. The way that works is that npm creates a .bin directory inside its node_modules one with scripts that execute node on the actual JavaScript file. npm creates a batch script called <NAME>.cmd and a shell simply script called <NAME>.

So, for Windows, .bin/gulp will actually call .bin/gulp.cmd. The contents of the script are presently:

@IF EXIST "%~dp0\node.exe" (
  "%~dp0\node.exe"  "%~dp0\..\gulp\bin\gulp.js" %*
) ELSE (
  @SETLOCAL
  @SET PATHEXT=%PATHEXT:;.JS;=;%
  node  "%~dp0\..\gulp\bin\gulp.js" %*
)

Honestly, I don't know why subprocess fails on Windows if shell=False though. But that's what I've been observing quite consistently.

[pytoxbot]

Original comment by @stefano-m

I have created pull request #165 that attempts to address the issue.

This is my first PR, so please excuse me if I have done something wrong with the various procedures etc.

Thanks.

[pytoxbot]

Original comment by @The-Compiler

What kind of file is .bin/gulp? Why does it have to be invoked via a shell?

[obestwalter]

@stefano-m As the PR was declined on bitbucket and the general opinion seems to be against changing the invocation semantics I'll close this. Feel free to reopen if you want to have another go at tackling this.

[stefano-m]

@obestwalter thanks for your comment. I agree with you on this and think that it's better to leave the code as is.

[jpmckinney]

When running tox on Windows on GitHub Actions, I inconsistently get the OSError: [WinError 193] %1 is not a valid Win32 application error. However, I haven't determined what specific invocation causes the error. Commenting here in case any old participants has any clues.

Issue tracked here: open-contracting/standard-maintenance-scripts#149