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

build,win: fix python detection script to handle spaces in path #14552

Merged
merged 0 commits into from
Aug 1, 2017

Conversation

jasongin
Copy link
Member

vcbuild.bat silently failed for me. I found the problem was the new find_python.cmd script failed to invoke python.exe because mine was installed at "C:\Program Files\Python27". Adding quotes in a few places fixed it.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build, win

@jasongin jasongin requested a review from refack July 31, 2017 06:43
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. install Issues and PRs related to the installers. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform. labels Jul 31, 2017
@jasongin
Copy link
Member Author

@refack
Copy link
Contributor

refack commented Jul 31, 2017

vcbuild.bat silently failed for me.

Can you think of a good point for it to make noise? That is echo an error?

@jasongin
Copy link
Member Author

Can you think of a good point for it to make noise? That is echo an error?

Well, there is already this code in vcbuild.bat:

call tools\msvs\find_python.cmd
if errorlevel 1 echo Could not find python2 & goto :exit

And then I would have expected that to be triggered by the second line here in find_python.cmd:

%p%python.exe -V 2>&1 | findstr /R "^Python.2.*" > NUL
IF ERRORLEVEL 1 EXIT /B %ERRORLEVEL%

However, when there is a space in the path, the script just dies at the findstr call there. I don't understand it, and I can't figure out any way to actually detect that error.

@refack
Copy link
Contributor

refack commented Jul 31, 2017

However, when there is a space in the path, the script just dies at the findstr call there. I don't understand it, and I can't figure out any way to actually detect that error.

Well... cmd.exe what can you do 🤷‍♂️ ... (I probably should have written it in PS)

@jasongin
Copy link
Member Author

jasongin commented Aug 1, 2017

Landed in cee8d6d

@jasongin jasongin merged commit cee8d6d into nodejs:master Aug 1, 2017
@jasongin jasongin deleted the find_python branch August 1, 2017 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. install Issues and PRs related to the installers. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants