Patch Node 4.8.4 to fix segmetation faults.#9031
Merged
benjamn merged 11 commits intorelease-1.5.2from Aug 23, 2017
Merged
Conversation
Contributor
|
The failures on the Will get that fixed soon! Edit: Same applies to |
This commit revives the script that was removed last year by a4ff6b7, when we switched from building our own version of Node to downloading the prebuilt release. The new implementation comes from @abernix's work on this branch: https://github.com/meteor/meteor/tree/abernix/dev-bundle-from-hash
Thanks to @abernix for identifying this solution to the duplication between dev_bundle/include/node and dev_bundle/.node-gyp/*/node.
To remain the same as official Node.js releases, we need to build with the `small-icu` ICU (International Components for Unicode) package. For Node.js 4.x this means ICU 56.x. As found with `process.config` on an official Node.js release. See https://github.com/nodejs/node/wiki/Intl#configure-node-with-specific-icu-source.
benjamn
force-pushed
the
patch-node-4.8.4-to-fix-segfaults
branch
from
44fd907 to
e2fe03a
Compare
A while back we switched from running `./meteor --get-ready`, a command that takes many minutes and sometimes runs out of memory, to just running `./meteor --help` to prepare for self-tests. The hope was that `./meteor --help` would fail less often, and the work that would have been done by `./meteor --get-ready` would be spread out through the actual tests. This helped, I think, but we've been seeing quite a few self-test failures due to unreliable timing of the actual tests, so I'd like to try shifting the balance back. I'm pushing this to the branch with our patched Node 4.8.4, because that branch should have a lower risk of segmentation faults, which may allow `./meteor --get-ready` to succeed more often. This is a bit of a shot in the dark, admittedly, but I want to see what happens. 🤞
benjamn
commented
Aug 23, 2017
| ./meteor --help | ||
| # shouldn't take longer than 5 minutes | ||
| no_output_timeout: 5m | ||
| ./meteor --get-ready |
Contributor
Author
There was a problem hiding this comment.
This worked amazingly well!
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings nodejs/node#14829 to the version of Node that ships with Meteor, hopefully preventing segmentation faults like those reported in #8648.
These changes will be officially released in Node 4.8.5, but we didn't want to wait until September or October for that release.
All credit for these changes belongs to @abernix. I simply adapted his script to run in a separate Jenkins job, so that we don't have to rebuild Node from source every time we publish a new dev bundle.