| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
headers tarball has a different structure to the source tarball, no deps or src subdirectories, but this addition makes existing addons work with a headers tarball, a source tarball or a --nodedir
Sorry, something went wrong.
@rvagg only for older versions, right? |
Sorry, something went wrong.
No, also for when you're compiling from the source tarball you can --nodedir and same for when you're doing it directly from the repo—like a lot of us in core have to do; we're stuck with both |
Sorry, something went wrong.
|
comments added to process-release.js |
Sorry, something went wrong.
|
I've given it a quick pass-through but I want to pull it down and run the tests, which is going to take an hour or two. So far: looks good, like the addition of tests, npm will probably just integrate this as-is once it's published. If it weren't for Node 4, I'd probably only land this in npm@3, because there are probably going to be enough knock-on effects from the switch as to cause issues (or at least surprises) for some users upgrading across npm@2 versions. I kind of wish that either npm@3 were the version going into Node 4 or this had a little more time to bake in before Node 4 drops, because I'm not sure I can anticipate all the implications of some of the changes in here. |
Sorry, something went wrong.
|
sorry, it seems that I have process.release branches on both this repo and my own fork .. pushed to the one that's behind this PR now. , bitsre = /\/win-(x86|x64)\//
, bitsreV3 = /\/win-(x86|ia32|x64)\// // io.js v3.x.x shipped with "ia32" but should
// have been "x86"
|
Sorry, something went wrong.
|
No problem and LGTM. |
Sorry, something went wrong.
Yes! 👯 |
Sorry, something went wrong.
semver.satisfies() doesn't play nicely with prerelease tags
|
OK, two more changes. nodejs/node#2719 reminded me that we can't trust semver.satisfies() any more so I've switched to checking major version numbers where needed, see bfaf7df. Also I've removed node_modules from the tree in 44dae28, I hope there are no objections to this. |
Sorry, something went wrong.
|
@rvagg Argh, sorry for this suggestion, I totally forgot most semver ranges ignore pre-releases now. |
Sorry, something went wrong.
Any chance for applying the same patch to pangyp? |
Sorry, something went wrong.
If we could do that elsewhere that would be nice. I'm actually curious if there are any advantages to them being in-tree for us. |
Sorry, something went wrong.
|
@mzgol done and released a patch update, just note that as soon as node-gyp@3 goes out I'm using the deprecate hammer on all those versions |
Sorry, something went wrong.
PR-URL: #711 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: #711 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
semver.satisfies() doesn't play nicely with prerelease tags PR-URL: #711 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: #711 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
merged and published as v3.0.0, thanks for all the help with this one folks! |
Sorry, something went wrong.
* support process.release * support all io.js versions * support node v4+ including new download locations * enable delay-load hook by default by default * download header-only tarballs instead of full source See nodejs/node-gyp#711 for full details PR-URL: #2700 Reviewed-By: Forrest L Norvell <forrest@npmjs.com>
* support process.release * support all io.js versions * support node v4+ including new download locations * enable delay-load hook by default by default * download header-only tarballs instead of full source See nodejs/node-gyp#711 for full details PR-URL: #2700 Reviewed-By: Forrest L Norvell <forrest@npmjs.com>
* support process.release
* support all io.js versions
* support node v4+ including new download locations
* enable delay-load hook by default by default
* download header-only tarballs instead of full source
See nodejs/node-gyp#711 for full details
PR-URL: #2700
Reviewed-By: Forrest L Norvell <forrest@npmjs.com>
* support process.release * support all io.js versions * support node v4+ including new download locations * enable delay-load hook by default by default * download header-only tarballs instead of full source See nodejs/node-gyp#711 for full details PR-URL: #2700 Reviewed-By: Forrest L Norvell <forrest@npmjs.com>
| Back | FazBrowse Home | New Git URL |
Looking for patient reviewers, ASAP please. I'd like to be able to ship v4 with this version patched over the top of npm and upstream it to npm as soon as they are happy with it as well.
What does this buy us?
What's not here?
We need to finally make node-gyp aware of locally installed headers: this has been on multiple people's radars for a very long time but has never got done but it should get done. When you install via a complete tarball, or a make install from the source, you get the header files installed for that version. node-gyp should be able to use those to avoid any downloading. However this is not a panacea because:
So this is a TODO, someone needs to take a bite at this soon, but it's not a top priority for shipping core.
Possible reviewers: @nodejs/build, @nodejs/addon-api, @TooTallNate, @Fishrock123, @othiym23, @bnoordhuis, @piscisaureus