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.

Sign up for GitHub

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

Jump to bottom

refactored binding.gyp #32

Merged
merged 1 commit into from
Mar 24, 2017
Merged

refactored binding.gyp #32

merged 1 commit into from
Mar 24, 2017

Conversation

refack
Copy link
Contributor

@refack refack commented Mar 20, 2017

Will compile on windows with node-gyp:3.6.0

@addaleax
Copy link
Owner

Awesome, thank you a lot! Can you edit the CI files to turn npm install [email protected] into npm install node-gyp@^3.6.0, too? If not I’m happy to just do that myself.

@refack
Copy link
Contributor Author

refack commented Mar 20, 2017

Done. Now I'm curious "will it blend" on 3.5.0...

refack
refack commented Mar 20, 2017
View reviewed changes
appveyor.yml
@@ -19,14 +19,11 @@ environment:


install:
# From https://github.com/nodejs/nan/blob/master/appveyor.yml
- ps: Install-Product node $env:nodejs_version
- ps: Update-NodeJsInstallation (Get-NodeJsLatestBuild $env:nodejs_version) $env:platform
Copy link
Contributor Author

@refack refack Mar 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

AppVeyor now does it on it's own according to nodejs_version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://www.appveyor.com/docs/lang/nodejs-iojs/#testing-under-multiple-versions-of-nodejs-or-iojs

@refack
Copy link
Contributor Author

refack commented Mar 20, 2017

Guess not https://ci.appveyor.com/project/addaleax/lzma-native/build/239/job/sv3uuu30fbocaqyt

@refack refack force-pushed the fix-gyp branch 7 times, most recently from 0291797 to 6c5d518 Compare March 21, 2017 01:47
refack
refack commented Mar 21, 2017
View reviewed changes
binding.gyp
"actions" : [
{
"msvs_quote_cmd": 0,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This kills the regression in node-gyp

@refack
Copy link
Contributor Author

refack commented Mar 21, 2017

I assume the red travis is an encrypted value permission issue:

$ openssl aes-256-cbc -K $encrypted_5d419efedfca_key -iv $encrypted_5d419efedfca_iv -in .travis.id_rsa.enc -out .travis.id_rsa -d
iv undefined
The command "openssl aes-256-cbc -K $encrypted_5d419efedfca_key -iv $encrypted_5d419efedfca_iv -in .travis.id_rsa.enc -out .travis.id_rsa -d" failed and exited with 1 during .

@refack @addaleax
refactored binding.gyp
3fb3736 
added "msvs_quote_cmd": 0
bumped node-gyp in appveyor.yml
@refack
Copy link
Contributor Author

refack commented Mar 24, 2017

@addaleax landing?

@addaleax addaleax merged commit c0491a0 into addaleax:master Mar 24, 2017
@addaleax
Copy link
Owner

Yeah, merging. Thanks for the hard work you’ve been putting into node-gyp & everything! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@refack @addaleax