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

Nodejs 12 support #125

Merged
merged 1 commit into from
Jul 25, 2019
Merged

Nodejs 12 support #125

merged 1 commit into from
Jul 25, 2019

Conversation

chrisdoc
Copy link
Contributor

@chrisdoc chrisdoc commented Jul 9, 2019

This PR aims to make node-dtrace-provider compatible with changes in v8 and allows the usage of the library with Node.JS 12. I ran the tests both for master with node 10.15.2 and on the branch with node 12.5.0 and on both I had 670 passing tests out of 699 total tests. I tested on MacOS 10.14.5 and disable SIP by executing csrutil disable in recovery mode. @chrisa do you have a continuous integration setup where I could test this PR.

@melloc
Copy link
Collaborator

melloc commented Jul 9, 2019

Hi @chrisdoc ,

There's no CI setup currently. I have a FreeBSD, SmartOS and OS X box that I currently run the tests on. I'll test these changes on them for you this week, and put the results here.

@chrisdoc
Copy link
Contributor Author

Thank you @melloc for your help in getting this tested

Reviewed by: Cody Peter Mello <cody.mello@joyent.com>
@melloc
Copy link
Collaborator

melloc commented Jul 25, 2019

I tested the following combinations:

  • SmartOS (joyent_20180813T212436Z), node v0.10.48, v0.12.16, v4.8.1, v6.17.0, and v8.15.1 (tested 32-bit and 64-bit for all versions)
  • SmartOS (joyent_20190523T082605Z), node v10.14.2 and v12.7.0 (64-bit)
  • FreeBSD 10, node v6.9.1 (64-bit)
  • FreeBSD 11, node v9.3.0 (64-bit)
  • macOS (version 10.14.6, Xcode 9.4), node v0.10.48 (32-bit and 64-bit), v4.9.1, v8.16.0, v10.16.0, v12.7.0 (64-it)

dtrace-provider failed to compile with node v0.12.18 on macOS due to nodejs/nan#676, which I can't really do anything about in this library. That's probably fine, since I imagine most people are using newer node versions.

I also had to make some minor tweaks to get the tests running on macOS, due to what seems to be a regression in using -Z to attach to USDT probes that haven't yet been defined. I'll follow up with a separate commit to update the tests to work once I'm convinced that's the problem, and that it's not something that libusdt should be updated to account.

@melloc melloc merged commit 3cbb3c8 into chrisa:master Jul 25, 2019
@melloc
Copy link
Collaborator

melloc commented Jul 25, 2019

Merged as 3cbb3c8. I'll prepare a 0.8.8 release once I figure out whether anything further needs to be done about the macOS issue.

@chrisdoc
Copy link
Contributor Author

Thank you @melloc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants