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.

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

Create vee-eight-5.0 branch #5592

Closed
wants to merge 2 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Mar 7, 2016

Pull Request check-list

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to [CONTRIBUTING.md][0]?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Affected core subsystem(s)

deps

Description of change

Picked up latest commit of 5.0-lkgr.
There is only one test to update this time.

R= @bnoordhuis, @ofrobots
cc @nodejs/v8

@targos targos added the v8 engine Issues and PRs related to the V8 dependency. label Mar 7, 2016
@targos
Copy link
Member Author

targos commented Mar 7, 2016

@targos
Copy link
Member Author

targos commented Mar 9, 2016

V8 5.0 introduced a small modification for the unexpected end of input
error.
@targos
Copy link
Member Author

targos commented Mar 17, 2016

Updated to 5.0.71.19.
CI: https://ci.nodejs.org/job/node-test-commit/2569/

@bnoordhuis
Copy link
Member

LGTM and sorry for the delay. Snapshotting on the armv8 bot still fails with a segmentation fault. I can probably figure out why if someone from @nodejs/build gives me access.

@indutny
Copy link
Member

indutny commented Mar 18, 2016

Rubber-stamp LGTM

@jbergstroem
Copy link
Member

@bnoordhuis I'll be slightly afk today (saturday) but ping me on irc and tell me where you need access. I'll hook you up.

@jbergstroem
Copy link
Member

@bnoordhuis ping re access

@bnoordhuis
Copy link
Member

@jbergstroem Sorry, must've missed your previous reply. I'll ping you on IRC later today.

@targos
Copy link
Member Author

targos commented Mar 26, 2016

I will be on a Greek island for the next two weeks and I don't think I'll have time to play with a computer, so anyone feel free to take over this PR !

@ofrobots
Copy link
Contributor

I can help with this. Enjoy your trip!

@jbergstroem I will ping you on Monday to get access to an ARM8 machine to diagnose the snapshot issue.

@ofrobots
Copy link
Contributor

I updated my copy of vee-eight-5.0 (https://github.com/ofrobots/node/tree/vee-eight-5.0) to the latest 5.0-lkgr and launched a new CI: https://ci.nodejs.org/job/node-test-commit/2715/.

The snapshot generation segfault is gone (probably because of this fix). There is different failure on arm that I need to investigate.

@ofrobots
Copy link
Contributor

Second CI run: https://ci.nodejs.org/job/node-test-commit/2717/. Failure seems to be a flake.

@trevnorris
Copy link
Contributor

I just tried the build on the latest of this branch and am seeing the following:

$ ./node -p "process.versions"
{ http_parser: '2.6.2
  node: '6.0.0-pre',
  v8: '4.9.385.27',
  uv: '1.8.0',
  zlib: '1.2.8',
  ares: '1.10.1-DEV',
  modules: '47',
  openssl: '1.0.2g' }

Can anyone else confirm that v8: '4.9.385.27' is showing up for them?

EDIT: nm me. Confused my branches.

@ofrobots
Copy link
Contributor

@trevnorris A blank vee-eight-5.0 branch was created by @targos so that he could target this PR to that branch. Nothing has landed on that branch yet. If you want to play with V8 5.0, you'll have to add-in commits from this PR, or better, use my branch here: https://github.com/ofrobots/node/tree/vee-eight-5.0.

@ofrobots
Copy link
Contributor

Taking this PR over in #5945.

@ofrobots ofrobots closed this Mar 29, 2016
sam-github pushed a commit to sam-github/node that referenced this pull request Dec 14, 2016
Since install is per machine only, installation path should be stored
in local machine instead of current user. The registry stores HKLM in
different places for 32 and 64 bit applications, so the installer will
not suggest the old path when upgrading from 32 to 64 bit version.

Fixes nodejs#5592
Fixes nodejs#25087

PR-URL: nodejs/node-v0.x-archive#25640
Reviewed-By: Alexis Campailla <alexis@janeasystems.com>
Reviewed-By: Bert Belder <bertbelder@gmail.com>
@targos targos deleted the vee-eight-5.0-proposal branch June 1, 2017 09:06
joaocgreis added a commit to JaneaSystems/node that referenced this pull request Aug 24, 2017
Since install is per machine only, installation path should be stored
in local machine instead of current user. The registry stores HKLM in
different places for 32 and 64 bit applications, so the installer will
not suggest the old path when upgrading from 32 to 64 bit version.

Fixes nodejs#5592
Fixes nodejs#25087
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants