Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows npm.bat pathname bug prevents build with unclear errors (with verified bug fix ) #21576

Closed
cleidigh opened this issue Feb 28, 2017 · 1 comment
Assignees

Comments

@cleidigh
Copy link
Contributor

Development Environment:

OS: Windows 7 Professional SP1

Visual Studio 15 Community \ C++ Tools \ SDK
VS Code: 1.9.1 master release
Node: 6.9.1
NPM: 3.10.8

Description Of Bug:

npm.bat will fail on Windows if the vscode code tree/repository is in a subdirectory
with any directories with spaces in the path. The error does not fatally
stop the script making it easy to miss the "cannot open" output from
the npm script. The error also does not show up in the npm_debug.txt
file after many modules have been processed.

The bug is in line five, in the findstr command -

5 for /f "tokens=2 delims=:, " %%a in ('findstr /R /C:""electronVersion":.*" %~dp0..\package.json') do set npm_config_target=%%~a

The file string: %~dp0..\package.json

Will to evaluate to multiple files/folders for any path with spaces and folder names.
Thus %electronVersion does not get set but no fatal error stops the script.
Since the command output gets overrun from the npm output it's easy to miss the warning.
The actual error building electron comes much later and gives no clue as to the cause
(empty electronVersion value)

Steps to reproduce:

(working example) CheckOut code to: C:\Dev\vscode

(failure example) CheckOut code to: C:\VSC Dev\vscode
(Initial output to terminal NOT npm_debug_out)
FINDSTR: Cannot open C:\VSC
FINDSTR: Cannot open Dev\vscode\scripts..\package.json
...subsequent npm output until electron failure

The Fix: Simply enclose the full file path string in double quotes

5 for /f "tokens=2 delims=:, " %%a in ('findstr /R /C:""electronVersion":.*" "%~dp0..\package.json"') do set npm_config_target=%%~a

I also added an error level output so code.bat could detect an error.
Additionally command output with an explicit fatal error.

Can I submit these fixes? I am embarrassed, but it took me three days
to catch this as all other similar issues I read on github pointed
to a toolchain issue. There our problems with the C++ tools install
that can lead you down a wrong path. I would like to add some notes
to the contribution wiki page.

Looking forward to contributing especially for accessibility
as a disabled\ALS user using voice programming - I really love
the vscode editor !

Cheers Christopher

@joaomoreno
Copy link
Member

joaomoreno commented Mar 1, 2017

Hi Christopher,

Thanks a lot for your fix! I just pushed it 👍

Very glad you like and use VS Code!

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
lemanschik pushed a commit to code-oss-dev/code that referenced this issue Nov 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants