-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
node-gyp rebuild downloads headers from incorrect location #787
Comments
Actually, just running I reported it also to node-sass in case it's their fault: sass/node-sass#1224 |
note the correct url
|
@rvagg @saper So, it seems unrelated to
|
What does this say?
|
plus some other unrelated packages under |
On Wed, 28 Oct 2015, Michał Gołębiowski wrote:
and all of them are 3.0.3 ? ... I also wonder why npm@3 in its awesomeness |
The global
|
@mzgol all of this is because we are using nvm. |
@mzgol does it work for you with |
Shoot, I thought I unloaded nvm correctly before replying to sass/node-sass#1224 (comment) but it seems I didn't do it correctly... I've just tried again and it works without nvm. Is there any reason to read |
@targos Yup, |
Also, this is just for the RC, when v5.0.0 goes live it'll be fetchable in /dist/ as expected. |
nvm is setting |
argh @ljharb: this is going to have to change if you want to support release candidates, node-gyp is now using |
It's surprising to me that node-gyp reads I initially thought it's a node-sass issue because I had a feeling nvm couldn't break Node in such a way. My intuition was partially right... I just didn't expect node-gyp to rely on nvm config. |
nvm uses This shows to me that node-gyp should use a separate variable, otherwise it implicitly depends on internal nvm logic that might change in the future. It's fragile. |
@rvagg um - yes, reading nvm's internal env variables (instead of just using them to modify I think what would make more sense is a I don't see how this impacts RC support, as I won't at all be changing the mirror logic for it. |
Maybe node-gyp should use somthing like electron-downloader. Maybe we should write a node-downloader? for headers and source. So its official in one place. |
I think we're just going to have to remove @noamokman node-gyp is that downloader, there shouldn't be a need for a separate tool, the logic is all in here. |
I have always thought this feature is in the node-gyp only to address some (arguable) shortcoming in how the node itself is packaged: I always get my current node include and With this issue solved I am always installing necessary header files in |
+1 |
+1 on this. Going through this thread but I'm not understanding the workaround. Could someone explain in |
I propose we introduce It'll be a breaking change so we'll have to bump semver-major and we'll have to communicate the change to users, the README is probably the best we can do on that front. |
👍 |
Sounds good to me.
|
Makes sense. I'm sure someone will request that @rvagg could you add |
yes, good call @ljharb, I need to get the headers download in for 0.12.10 and 0.10.42 in before we bump a major anyway so I'll try and bundle that and hopefully it'll be palatable enough for npm to include in v2 |
solution @ #878, reviewers welcome |
we've moved a step closer with v3.3.0 just out that introduces |
In case anyone still come here because npm install node-sass cannot be executed because he/she is behind a corporate proxy or needs to work offline. The root cause of the problem is the post-script of node-sass cannot download binding.node from github.
We need provide the file before it goes to github. Steps:
|
for me: can work. |
I thought that
node-gyp
3.0.3
should have this all fixed by taking the data fromprocess.release
but it still downloads from the default one for me:Switch to Node 5 & then (notice the incorrect URL https://nodejs.org/dist/v5.0.0-rc.1/node-v5.0.0-rc.1-headers.tar.gz):
The text was updated successfully, but these errors were encountered: