-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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
npm install/node-gyp rebuild fails on 8.1.1 (wrong dist-url) #13667
Comments
|
@theverything are you seeing the same as @bhj? could you paste the whole |
See nodejs/node-gyp#1230, the embedded download URL in v8.1.1 is wrong:
As a workaround, you can install with:
cc @addaleax - might be worth doing a new release for. I'll remove the npm and macos labels. |
Yeah, maybe … @nodejs/release @jasnell I’m not quite sure how this happened? The default url base in the source code is |
I think this is an issue in the ci-release job configure script. There's this code: RELEASE_URLBASE=https://nodejs.org/download/${DISTTYPE}/
if [ "X${disttype}" == "Xtest" ]; then
disttype=custom
CUSTOMTAG="test${datestring}${commit}"
RELEASE_URLBASE=https://nodejs.org/download/test/
elif [ "X${disttype}" == "Xv8-canary" ]; then
disttype=custom
CUSTOMTAG="v8-canary${datestring}${commit}"
RELEASE_URLBASE=https://nodejs.org/download/v8-canary/
elif [ "X${disttype}" == "Xrc" ]; then
disttype=custom
CUSTOMTAG="rc.$rc"
RELEASE_URLBASE=https://nodejs.org/download/rc/
fi
# ...
make -j 2 binary-upload \
DESTCPU="$DESTCPU" \
ARCH="$ARCH" \
DISTTYPE="$disttype" \
DATESTRING="$datestring" \
COMMIT="$commit" \
CUSTOMTAG="$CUSTOMTAG" \
RELEASE_URLBASE="$RELEASE_URLBASE" \
CONFIG_FLAGS="$CONFIG_FLAGS" but the env var we pass in is I think the best thing to do is just to uppercase cc/ @nodejs/build |
The Windows release is unaffected:
|
Yep, that's a different script (and windows env vars aren't case-sensitive). |
This is bad guys 😞 Basically breaks all native modules for non windows since node-gyp will fail to download headers for |
Is an urgent bug fix release planned for this? |
No idea how this happened either. A change to the ci-release script is the only thing I can imagine. That's a bit troubling. |
@refack yeah I'm seeing the same thing |
Workaround if you are deploying to heroku:
|
Changed title to (in theory) improve discoverability. |
@fiznool nice find on the env var for the workaround. Just want to mention that folks on Heroku should be sure to unset the var on next node release to avoid another difficult to diagnose build breakage in the future if the release host changes. |
In the meantime, can the web server be configured to handle this incorrect URL? |
I'm going to have to take the blame for this. It's a result of my refactoring of the main Jenkins build job to accommodate the new v8-canary builds. I'll push a speedy 8.1.2 release with only a version bump to take care of this unless anyone objects to that course of action. |
@rvagg Thank you, that would be great. :) Doing that has been suggested here before and nobody objected, so I think it’s safe to say you have our blessings. |
Building, I'll test the binaries before promoting them to make sure I have it right! |
Release to fix broken `process.release` properties Ref: #13667
Out in a redirect for https://nodejs.org/download//v8.1.1/node-v8.1.1-headers.tar.gz and https://nodejs.org/download//v8.1.1/SHASUMS256.txt as has been suggested in this thread already. Confirmed working (thankfully node-gyp uses request which follows redirects automatically). So the problem should be solved for now for everyone. But, as is being discussed over in #13667 we probably won't leave this redirect in place permanently, just considering it as a band-aid solution for now. Still working on an 8.1.2, should be out shortly. |
Release to fix broken `process.release` properties Ref: #13667
8.1.2 out @ https://nodejs.org/en/blog/release/v8.1.2/ armv6 still building, will promote later |
@ilkkao that was a good idea, very Web 2.0 (that was a thing). I'm feeling old that I was stuck in the filesystem metaphor 👴 |
Release to fix broken process.release properties Ref: nodejs/node#13667
run
npm --build-from-source install fsevents
Edit (by @gibfahn):
Workaround is (see @addaleax's tweet):
npm config set dist-url https://nodejs.org/download/release/
This will be fixed in the next Node 8.x release.
The text was updated successfully, but these errors were encountered: