When running "kallithea-cli front-end-build" in Windows' git-bash, cli can't find npm

Issue #332 resolved
Edmund Wong created an issue

Under a Git bash system, when running "kallithea-cli front-end-build", it errors with the following:

$ kallithea-cli front-end-build
Running 'npm install' to install front-end dependencies from package.json
Traceback (most recent call last):
  File "J:\otherprojs\kallithea\venv\Scripts\kallithea-cli-script.py", line 11,
in <module>
    load_entry_point('Kallithea', 'console_scripts', 'kallithea-cli')()
  File "j:\otherprojs\kallithea\venv\lib\site-packages\click\core.py", line 764,  in __call__
    return self.main(*args, **kwargs)
  File "j:\otherprojs\kallithea\venv\lib\site-packages\click\core.py", line 717,  in main
    rv = self.invoke(ctx)
  File "j:\otherprojs\kallithea\venv\lib\site-packages\click\core.py", line 1137, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "j:\otherprojs\kallithea\venv\lib\site-packages\click\core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "j:\otherprojs\kallithea\venv\lib\site-packages\click\core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "j:\otherprojs\kallithea\kallithea\kallithea\bin\kallithea_cli_front_end.py", line 45, in front_end_build
    subprocess.check_call(['npm', 'install'], cwd=front_end_dir)
  File "c:\program files\git\python27\Lib\subprocess.py", line 181, in check_call
    retcode = call(*popenargs, **kwargs)
  File "c:\program files\git\python27\Lib\subprocess.py", line 168, in call
    return Popen(*popenargs, **kwargs).wait()
  File "c:\program files\git\python27\Lib\subprocess.py", line 390, in __init__
    errread, errwrite)
  File "c:\program files\git\python27\Lib\subprocess.py", line 640, in _execute_child
    startupinfo)
WindowsError: [Error 2] The system cannot find the file specified

This error prevents Windows developers to run a dev env using Git bash.

The issue is because nodejs (as installed from the nodejs Windows package from nodejs' site), installs npm and npm.cmd to the nodejs directory as specified. (In my case, d:\nodejs). However, under the Git Bash, it doesn't detect 'npm' as a run-able executable. The 'npm.cmd' is ignored.

Comments (28)

  1. Mads Kiilerich

    I thought "git bash" was msys which has a nice abstraction of only providing a shell but running programs natively ... but from the description, it sounds more like it is cygwin which introduce a different calling convention?

    Can you confirm it works when using "plain" windows command prompt and shell?

    Does a change like this make any difference?:

    --- a/kallithea/bin/kallithea_cli_front_end.py
    +++ b/kallithea/bin/kallithea_cli_front_end.py
    @@ -42,7 +42,7 @@ def front_end_build(install_deps, genera
    
         if install_deps:
             click.echo("Running 'npm install' to install front-end dependencies from package.json")
    -        subprocess.check_call(['npm', 'install'], cwd=front_end_dir)
    +        subprocess.check_call('npm install', cwd=front_end_dir, shell=True)
    
         if generate:
             tmp_dir = os.path.join(front_end_dir, 'tmp')
    
  2. Edmund Wong reporter

    In plain windows command, npm works, though it's because it runs the npm.cmd version and not npm. I tested this out by renaming npm.cmd to npmx.cmd, and then running npm. It can't find that command.

    And no, changing ['npm', 'install'] to 'npm install' doesn't work either.

  3. Mads Kiilerich

    Our use of npm on Windows is probably a bit untested and rough. Thanks for helping out.

    Let's be explicit and make sure we agree on the back story.

    A bit of googling suggest that it should be downloaded from https://nodejs.org/en/download/ . That should perhaps be documented. (A bit surprising for me that the platform (nodejs) also include a package manager (npm) that runs on/inside the platform.)

    In usual Windows style, the installation binary path must be added to PATH (where Unix systems generally install to a /bin directory that already is in PATH).

    Looking at https://nodejs.org/dist/v10.15.1/node-v10.15.1-win-x64.zip , I see it contains 'npm.cmd' which is a plain cmd script, "Created by npm", and launches node.exe with the right npm-cli.js. It also contains a plain 'npm' which is a mingw (msys) / cygwin sh script.

    Cygwin do evil things, changing semantics of other programs by redefining well-known windows-isms to be more unix-like, thus leaving it as something neither is one or the other. I don't think it is worth it to try to support that.

    Msys/mingw are fine. Once programs have been launched, they run fully native.

    So you confirm that on plain windows, this works, using npm.cmd: python -c "import subprocess; subprocess.check_call(['npm', '-v'])"

    But you found that inside git bash it fails, as Python cannot find npm.cmd? It seems like this must be a cygwin-ish environment. And if using shell=True, it still doesn't use the proper shell for the OS but a special git bash shell? The proper way to support that would be at the Python level. It seems wrong if we have to work around that.

    Should we really support git bash, even it it require adding hacks and layering violations that we cannot guarantee actually works? How common and essential is it to use git bash? How do other programs deal with it?

  4. Mads Kiilerich

    But ... while I don't like the idea of adding "random" and supposedly unnecessary workarounds we don't understand and can't maintain, I guess it would be ok to be explicit about windows npm actually being npm.cmd ... even though the whole idea of using .cmd was that it should be invisible and irrelevant to us users of it ...

    Thoughts? @Thomas De Schampheleire ?

  5. Thomas De Schampheleire

    Thanks @Mads Kiilerich for the details you found, that was very helpful.

    @Edmund Wong Would it be possible to dump the value of the 'PATHEXT' environment variable from Kallithea?

    Could you also try to run 'npm' directly from git bash (not via Kallithea)? Does that work? And what is the value of PATHEXT there?

    Depending on the above we could consider asking for help from the Git Bash developers, which seem to be active: https://github.com/git-for-windows/git/issues

    Also, I just found this: https://github.com/msys2/MINGW-packages/issues/1039 which claims that the problem is actually in the 'npm' wrapper which should do some 'cygpath' basedir conversion even if not on cygwin. This leads to following patch: https://github.com/msys2/MINGW-packages/pull/1096/commits/241409d8de3fbed0171446acbe272c6bdbb8dc20

    Could you try this change manually on the 'npm' script? i.e.:

    --- npm 2018-08-02 16:56:34.000000000 +0200
    +++ npm.new 2019-02-23 20:35:28.698031593 +0100
    @@ -3,9 +3,9 @@
    
     basedir=`dirname "$0"`
    
    -case `uname` in
    -    *CYGWIN*) basedir=`cygpath -w "$basedir"`;;
    -esac
    +if [ -x "$(command -v cygpath)" ]; then
    +  basedir=`cygpath -w "$basedir"`
    +fi
    
     NODE_EXE="$basedir/node.exe"
     if ! [ -x "$NODE_EXE" ]; then
    
  6. Mads Kiilerich

    About the problem being in the 'npm' wrapper: That one must be for running directly from bash. It should never be used when a windows program invoke a sub program or a windows shell run 'npm' - then it should look for 'npm.cmd' and other extensions in PATHEXT. The described workaround should thus not be relevant?

  7. Thomas De Schampheleire

    "It should never be used" because it does not have an extension in PATHEXT? (I'm not familiar with Windows development, and don't use it anymore either). That probably sounds reasonable. I fail to really understand what this 'git bash' thing is and how it influences execution of programs started inside of it.

  8. Mads Kiilerich

    Yes, I would expect everything to work as usual and use the usual PATHEXT when not directly in a bash shell or bash script (or program compiled to work inside this environment).

    git bash (and msys and cygwin) are all about making windows look more posix. But still, it is windows, and we have to insist on treating it as "normal" windows - we can't have it as another "platform" that we have to consider.

  9. Edmund Wong reporter

    I apologize for the confusion. Git bash is the mingw64 bash command window that is included in the Git Windows installation.

    @Thomas De Schampheleire 1) Regarding PATHEXT, prior to the execution of the npm install command, the PATHEXT is: .COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH;.MSC

    2) Changing the npm script doesn't do anything as it doesn't even find the npm script.

    3) git bash uses mingw64.

    4) digging deeply into front_end_build(), and following the code, check_call() which calls call() which calls Popen(...).. which finally runs CreateProcess(). Came across this tidbit: https://bugs.python.org/issue8557.

    So under windows (if I understand that correctly), explicit inclusion of the extension is required. (Please do correct me if I'm wrong.) Which is probably why nodejs includes *.cmd in the nodejs dir for Windows installation. (still looking for the reason..)

    sorry for the mess of a patch..

  10. Edmund Wong reporter

    Also, forgot to mention. Under the Git Bash command prompt, running npm (without the .cmd) works as well as the fact that it doesn't depend on the .cmd existing (as I had renamed the npm.cmd to npmx.cmd).

  11. Mads Kiilerich

    Yes, for some reason npm added a special bash launcher that can be used under mingw/cygwin instead of native windows. It shouldn't be necessary (as any bash on windows also must obey PATHEXT and be able to run .com / .exe / .cmd files), but it might be convenient in some cases.

    @Edmund Wong can you confirm that on plain windows, this works, using npm.cmd:

    python -c "import subprocess; subprocess.check_call(['npm', '-v'])"
    

    ?

    If so, does it fail when run inside a bash prompt?

    That seems to be a minimal case that is close to the root cause, not involving anything Kallithea, and can help us understand what is going on.

    Are you perhaps using a special mingw Python when launching from bash? Can you try to explicit use the python.exe used from a cmd prompt? But you found that inside git bash it fails, as Python cannot find npm.cmd? It seems like this must be a cygwin-ish environment. And if using shell=True, it still doesn't use the proper shell for the OS but a special git bash shell? The proper way to support that would be at the Python level. It seems wrong if we have to work around that.

  12. Edmund Wong reporter

    @Mads Killerich

    In a windows command prompt (with d:\nodejs in the PATH):

    C:\projects\personal>python -c "import subprocess ;
    subprocess.check_call(['npm', '-v'])"
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "C:\Program Files (x86)\Git\python36\lib\subprocess.py", line 286, in check_call
        retcode = call(*popenargs, **kwargs)
      File "C:\Program Files (x86)\Git\python36\lib\subprocess.py", line 267, in call
        with Popen(*popenargs, **kwargs) as p:
      File "C:\Program Files (x86)\Git\python36\lib\subprocess.py", line 709, in __init__
        restore_signals, start_new_session)
      File "C:\Program Files (x86)\Git\python36\lib\subprocess.py", line 997, in _execute_child
        startupinfo)
    FileNotFoundError: [WinError 2] The system cannot find the file specified
    
    C:\projects\personal>
    

    If I use 'npm.cmd' instead of 'npm':

    C:\projects\personal>python -c "import subprocess ;
    subprocess.check_call(['npm.cmd', '-v'])"
    6.4.1
    

    in a git bash:

    $ python -c "import subprocess; subprocess.check_call(['npm', '-v'])"
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "C:\Program Files\Git\python27\lib\subprocess.py", line 181, in check_call
        retcode = call(*popenargs, **kwargs)
      File "C:\Program Files\Git\python27\lib\subprocess.py", line 168, in call
        return Popen(*popenargs, **kwargs).wait()
      File "C:\Program Files\Git\python27\lib\subprocess.py", line 390, in __init__
        errread, errwrite)
      File "C:\Program Files\Git\python27\lib\subprocess.py", line 640, in _execute_child
        startupinfo)
    WindowsError: [Error 2] The system cannot find the file specified
    

    but using npm.cmd works:

    $ python -c "import subprocess; subprocess.check_call(['npm.cmd', '-v'])"
    6.4.1
    

    To note though that Python was installed via python.org's msi and installed within the git path (i.e. c:\program files\Git\python27) as the windows git bash installation doesn't include python.

    As far as I understand from the above, in a windows command prompt, "npm -v" runs the "npm.cmd -v" command. However, under python's subprocess, it totally ignores the PATHEXT file.

    And I think you're right.. this is a Python issue and not Kallithea.
    judging from https://bugs.python.org/issue15533 or https://bugs.python.org/issue8557.

  13. Mads Kiilerich

    Thanks for helping clarifying this.

    Now we can completely rule out that it has anything to do with git bash. Do you agree? It would almost make sense to start over from scratch, only including information we now know is relevant ...

    But ok, right, it makes sense that pure subprocess doesn't do like the shell does. But then I would expect plain 'npm' to work if using shell=True. (But even if so, I guess we might conclude that it is better to not use shell. Especially if pure subprocess use PATH anyway ... and I guess it does.)

    Right now, I tend to prefer the solution of having some kind of "npm command" function or platform dependent "const". It could perhaps also be extra configurable with a .ini setting, but that might come close to giving a chicken-egg problem.

  14. Brandon Jones

    Just adding my experience with this issue here - once I modified all of the subprocess calls to include .cmd on the end, I was able to successfully get the front end to build. Here is what I modified in kallithea_cli_front_end.py

    • 45: subprocess.check_call(['npm.cmd', 'install'], cwd=front_end_dir)
    • 59: lesscpath = os.path.join(front_end_dir, 'node_modules', '.bin', 'lessc.cmd')
    • 96: os.path.join(front_end_dir, 'node_modules', '.bin', 'license-checker.cmd'),
  15. Mads Kiilerich

    Hmm. Thanks for clarifying that these additional workarounds are needed too ...

    But I wonder why Edmund didn't need it ... or didn't report it.

  16. Thomas De Schampheleire

    I went into Windows to see things for myself. I made a simple foobar.cmd file with 'echo hello' inside of it, to investigate away from npm.

    c:\Users\tdescham>C:\python27\python -c "import subprocess; subprocess.call('foobar')"
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "C:\python27\lib\subprocess.py", line 172, in call
        return Popen(*popenargs, **kwargs).wait()
      File "C:\python27\lib\subprocess.py", line 394, in __init__
        errread, errwrite)
      File "C:\python27\lib\subprocess.py", line 644, in _execute_child
        startupinfo)
    WindowsError: [Error 2] The system cannot find the file specified
    
    
    c:\Users\tdescham>C:\python27\python -c "import subprocess; subprocess.call('foobar.cmd')"
    c:\Users\tdescham>echo hello
    hello
    
    
    c:\Users\tdescham>C:\python27\python -c "import subprocess; subprocess.call('foobar', shell=True)"
    c:\Users\tdescham>echo hello
    hello
    
    
    c:\Users\tdescham>C:\python27\python -c "import subprocess; subprocess.call('foobar.cmd', shell=True)"
    c:\Users\tdescham>echo hello
    hello
    

    So, based on the above, my conclusion is that 'shell=True' does actually fix the problem.

  17. Thomas De Schampheleire

    Also confirmed with npm, from a Windows command prompt:

    C:\Users\tdescham>C:\python27\python -c "import subprocess; subprocess.call('npm -v', shell=True)"
    6.4.1
    
  18. Thomas De Schampheleire

    For real executables, e.g. node.exe, there is no need to do anything special. subprocess.call('node') works fine, without shell=True or explicit extension. The problem is only relevant for scripts like foo.cmd. This is also hinted by comment https://bugs.python.org/issue8557#msg262399 :

    CreateProcess executes PE executables and batch files (run via the %ComSpec% interpreter).
    It automatically appends the .exe extension when searching for an executable. It does this via the lpExtension parameter of SearchPath [3]. 
    
    Some .com files are PE executables (e.g. chcp.com). Otherwise it's not really usefully to
    loop over the PATHEXT extensions unless you're using shell=True, since most are filetypes that CreateProcess doesn't support [4]. 
    
    [4]: If Microsoft's Windows team cared at all about cross-platform
         idioms they'd add shebang support to CreateProcess, which
         would make all scripts, not just batch files, directly
         executable without requiring ShellExecuteEx and registered
         filetypes. ShellExecuteEx doesn't support a lot of useful
         creation flags that are only available by calling
         CreateProcess directly, and it also doesn't check ACLs to
         prevent executing a file. So scripts are second class
         citizens in Windows, which is why Python has to embed 
         scripts in .exe wrappers.
    

    So, we need to know what kind of thing we are executing in order to know how to call it. Or, as suggested in some of the referenced links, do a lookup before calling.

    @Mads Kiilerich Based on this info, what is your final preference to solve this issue?

  19. Mads Kiilerich

    @Thomas De Schampheleire I agree, that is what I thought too. Thus, my initial solution should work too. But apparently it didn't? (Or ... Edmund's response could perhaps suggest that he didn't use shell=True?) Did you try my initial patch?

  20. Edmund Wong reporter

    @Mads Killerich I didn't use shell=True as I've always avoided using it. I'll try it with shell=True.

  21. Edmund Wong reporter

    Ok.. I can confirm.. using shell=True does work. My question is "is it ok to do that?" I certainly don't want to introduce any security issues.. or screw something up

  22. Edmund Wong reporter

    Would something like the following work?:

    diff --git a/kallithea/bin/kallithea_cli_front_end.py b/kallithea/bin/kallithea_cli_front_end.py
    --- a/kallithea/bin/kallithea_cli_front_end.py
    +++ b/kallithea/bin/kallithea_cli_front_end.py
    @@ -21,19 +21,21 @@ import subprocess
     import json
     import sys
    
     import kallithea
    
    
     def make_win_cmd(in_name):
         retval = in_name
    +    ret_use_shell = False
         if sys.platform.startswith('win'):
             retval = '%s.cmd' % in_name
    -    return retval
    +        ret_use_shell = True
    +    return retval, ret_use_shell
    
    
     @cli_base.register_command()
     @click.option('--install-deps/--no-install-deps', default=True,
             help='Skip installation of dependencies, via "npm".')
     @click.option('--generate/--no-generate', default=True,
             help='Skip generation of front-end files.')
     def front_end_build(install_deps, generate):
    @@ -44,41 +46,41 @@ def front_end_build(install_deps, genera
         covers Python dependencies.
    
         The installation of front-end dependencies happens via the tool 'npm' which
         is expected to be installed already.
         """
         front_end_dir = os.path.abspath(os.path.join(kallithea.__file__, '..', 'front-end'))
         public_dir = os.path.abspath(os.path.join(kallithea.__file__, '..', 'public'))
    
    -    npm_exe = make_win_cmd('npm')
    +    npm_exe, use_shell = make_win_cmd('npm')
    
         if install_deps:
             click.echo("Running 'npm install' to install front-end dependencies from package.json")
    -        subprocess.check_call([npm_exe, 'install'], cwd=front_end_dir)
    +        subprocess.check_call([npm_exe, 'install'], cwd=front_end_dir, shell=use_shell)
    
         if generate:
             tmp_dir = os.path.join(front_end_dir, 'tmp')
             if not os.path.isdir(tmp_dir):
                 os.mkdir(tmp_dir)
    
             click.echo("Building CSS styling based on Bootstrap")
             with open(os.path.join(tmp_dir, 'pygments.css'), 'w') as f:
                 subprocess.check_call(['pygmentize',
                         '-S', 'default',
                         '-f', 'html',
                         '-a', '.code-highlight'],
                         stdout=f)
    -        lesscpath = make_win_cmd(
    +        lesscpath, use_shell = make_win_cmd(
                 os.path.join(front_end_dir, 'node_modules', '.bin', 'lessc'))
             lesspath = os.path.join(front_end_dir, 'main.less')
             csspath = os.path.join(public_dir, 'css', 'style.css')
             subprocess.check_call([lesscpath, '--source-map',
                     '--source-map-less-inline', lesspath, csspath],
    -                cwd=front_end_dir)
    +                cwd=front_end_dir, shell=use_shell)
    
             click.echo("Preparing Bootstrap JS")
             shutil.copy(os.path.join(front_end_dir, 'node_modules', 'bootstrap', 'dist', 'js', 'bootstrap.js'), os.path.join(public_dir, 'js', 'bootstrap.js'))
    
             click.echo("Preparing jQuery JS with Flot, Caret and Atwho")
             shutil.copy(os.path.join(front_end_dir, 'node_modules', 'jquery', 'dist', 'jquery.min.js'), os.path.join(public_dir, 'js', 'jquery.min.js'))
             shutil.copy(os.path.join(front_end_dir, 'node_modules', 'jquery.flot', 'jquery.flot.js'), os.path.join(public_dir, 'js', 'jquery.flot.js'))
             shutil.copy(os.path.join(front_end_dir, 'node_modules', 'jquery.flot', 'jquery.flot.selection.js'), os.path.join(public_dir, 'js', 'jquery.flot.selection.js'))
    

    The rationale for the ret_use_shell is to return to the calling code the fact that it is being run on a Windows system and should use shell=True. If it isn't a Windows system, set shell as false.

  23. Mads Kiilerich

    Ok. We could have saved a lot of time and confusion if my question had been answered ... or if it had been more clear that it wasn't an answered.

    shell=True is evidently the right way to make plain "npm" work on Windows. It is annoying it doesn't work with argv on posix, and shell=True thus force us to use a command line snippet and thus have concerns about quoting. That is reliability and "not entirely right" concerns, but I don't see any serious security concerns.

    The question is if we want to hardcode the assumption that these commands are .cmd on windows, or if we just want to be supportive of the idea that they might be .cmd . I think I prefer to just be supportive.

    But how about keep using an argv, but use shell=kallithea.is_windows? Posix will keep using the native argv handling without shell and quoting, and windows subprocess will use its internal and secure list2cmdline. That will also be self-documenting that while testing might show that shell isn't needed on posix, it is needed on windows, and annotate will point to the more detailed discussion in the commit message. @Thomas De Schampheleire right now I think that seems like the best solution ;-)

  24. Log in to comment