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

Test output instead of guessing by platform #77

Merged
merged 1 commit into from
Feb 14, 2017
Merged

Test output instead of guessing by platform #77

merged 1 commit into from
Feb 14, 2017

Conversation

d3x0r
Copy link
Contributor

@d3x0r d3x0r commented Feb 11, 2017

I have Windows Server 2012
node v7.5.0 (x64)
cmake 3.8.0-rc1 (x64)

build tools installed with 'npm install -g windows-build-tools'

The output received at line 73 in this windows platform has only \n in the buffer.
If you test for 'if stdout.includes('\r\n')' then you can handle it based on what was given, instead of assuming a platform is going to be a certain way.

This was causing
ERR! OMG There is no Visual C++ compiler installed. Install Visual C++ Build Toolset or Visual Studio.
ERR! OMG There is no Visual C++ compiler installed. Install Visual C++ Build Toolset or Visual Studio.

I tracked it back to _getTopSupportedVisualStudioGenerator had a list that was empty ... [] ...
and that's because of the parsing error.

I have Windows Server 2012 
node v7.5.0  (x64)
cmake 3.8.0-rc1  (x64)

build tools installed with 'npm install -g windows-build-tools'

The output received at line 73 in this windows platform has only \n in the buffer.
If you test for 'if stdout.includes('\r\n')' then you can handle it based on what was given, instead of assuming a platform is going to be a certain way.

This was causing 
ERR! OMG There is no Visual C++ compiler installed. Install Visual C++ Build Toolset or Visual Studio.
ERR! OMG There is no Visual C++ compiler installed. Install Visual C++ Build Toolset or Visual Studio.

I tracked it back to _getTopSupportedVisualStudioGenerator had a list that was empty ... [] ... 
and that's because of the parsing error.
Copy link
Member

@unbornchikken unbornchikken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. However, there is a better method for handling this, see: http://stackoverflow.com/questions/10864486/node-js-constant-for-platform-specific-new-line

@d3x0r
Copy link
Contributor Author

d3x0r commented Feb 13, 2017

Nope.... on the system in question....

> require('os').EOL
'\r\n'

it's probably on the output of cmake that's causing it.... which you're not going to be able to control what programs give you. Most of mine will generate only \n no matter what the system.

@unbornchikken unbornchikken merged commit f96f754 into cmake-js:master Feb 14, 2017
@unbornchikken
Copy link
Member

Understood, thanks.

@unbornchikken
Copy link
Member

Publised as of v3.4.1.

@TheGuardianWolf
Copy link

Can confirm same error was resolved for my system as well from upgrading 3.4.0->3.4.1.

I'm on Windows 10 x64.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants