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

Patch V8 API to be ABI compatible with v5.9. #12878

Closed
wants to merge 1 commit into from

Conversation

MylesBorins
Copy link
Contributor

@MylesBorins MylesBorins commented May 7, 2017

This is a patch to make V8 5.8 ABI compatible with V8 5.9

I did not include a patch to bump the NODE_MODULES version. Do we plan to bump it for each ABI change prior to release?

V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/690/

CI: https://ci.nodejs.org/job/node-test-pull-request/7926/

@nodejs-github-bot nodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label May 7, 2017
@addaleax
Copy link
Member

addaleax commented May 7, 2017

Comparing with my own attempt in #12875, this is missing:

  • c107737 – fixes API, not ABI, breakage, and includes a deprecation that we probably want
  • 01605c0 – ABI breakage, that was just overlooked I guess?
  • 2f6c4bd – as long as we consider deprecations API breakage, this is needed
  • ac5c950 – also ABI breakage that seems to have been overlooked in this patch?

I would say that this patch has a somewhat more elegant solution for the changed value of kJSApiObjectType, but other than that #12875 seems more complete right now.

@addaleax addaleax added this to the 8.0.0 milestone May 7, 2017
@MylesBorins
Copy link
Contributor Author

hey @addaleax didn't see your original patch. Want me to close this?

@addaleax
Copy link
Member

addaleax commented May 7, 2017

@MylesBorins I don’t really care that much… If you want, feel free to cherry-pick the changes listed above (plus 932bd46 and maybe cbf3ecf) to this PR and I’ll close mine. ¯\_(ツ)_/¯

@MylesBorins
Copy link
Contributor Author

Closing in favor of #12875

@MylesBorins MylesBorins closed this May 7, 2017
@Trott Trott removed this from the 8.0.0 milestone May 7, 2017
@targos targos mentioned this pull request Jun 7, 2017
2 tasks
@MylesBorins MylesBorins deleted the 5.8-w-5.9-abi branch November 14, 2017 17:43
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.

4 participants