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

Updated nan to 1.8.4 to work with io.js v2.3.1 #7

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kieut
Copy link

@kieut kieut commented Jun 30, 2015

Updated nan to 1.8.4 to work with newest versions of Node.

@bobrik
Copy link
Contributor

bobrik commented Jul 1, 2015

Can you tell me which version of node breaks? v0.12.5 works fine for me.

Any reason to change src/lzf/lzfP.h? Current versions seems to be the latest.

@kieut
Copy link
Author

kieut commented Jul 1, 2015

It breaks with Node/io.js v2.3.1. I changed src/lzf/lzfP.h originally because Clang Mavericks OSX couldn't find cstdint. On further inspection, the file can be found under the tr1 directory. I've pushed the changes. Reference: http://stackoverflow.com/questions/10116724/clang-os-x-lion-cannot-find-cstdint

@kieut kieut changed the title Updated nan to 1.8.4 Updated nan to 1.8.4 to work with io.js v2.3.1 Jul 1, 2015
@bobrik
Copy link
Contributor

bobrik commented Jul 2, 2015

Can you give me the link how you installed node? Looks like iojs installed from homebrew doesn't work with npm. I want to replicate your issue.

I also recall that node-lzf worked for me on mavericks and it works for me on yosemite with v0.12.5.

@kieut
Copy link
Author

kieut commented Jul 6, 2015

I did the manual install from https://github.com/creationix/nvm, and used nvm to install iojs.

@bobrik
Copy link
Contributor

bobrik commented Jul 7, 2015

Okay, looks like npm that comes with iojs is causing the troubles. I suggest you to raise an issue for npm or iojs since it works with nodejs.

I understand that this is bad to have a broken module, but changing vendored sources for lzf sounds like a bad idea for me. It can only be pulled from upstream, not changed locally.

Does that make sense?

@kieut
Copy link
Author

kieut commented Jul 8, 2015

Thanks for responding, I really appreciate it.To clarify, did you use homebrew or nvm to install iojs? We're not using home homebrew to install iojs, because we've run into issues with it. When I run brew info iojs, it says that iojs currently requires a patched npm which you pointed out as the reason for the troubles. This is why we use nvm to install iojs, it downloads the most recent version of npm.

As for the dependencies, lzf is using nan 1.3.0. I looked at their changelog and I see that iojs isn't supported until version 1.4.2/1.5.0. The changelog says that version 1.8.4(the most current) supports both current Node 12: 0.12.2, Node 10: 0.10.38, and io.js: 1.8. Since it should support both, I don't see why updating nan would be a problem. Perhaps you could elaborate on your reasoning?

I don't really understand what you mean by 'changing vendored sources for lzf', and that it can only be pulled from upstream and not locally. Can you please explain?

@bobrik
Copy link
Contributor

bobrik commented Jul 9, 2015

I used nvm to install iojs and it's failing, I can confirm that. I am also not denying that nan should be upgraded.

By changing vendored sources for lzf I mean that I didn't write those sources, they come from liblzf. I don't want to change the stuff I don't own for no good reason. Since it all works with nodejs, but doesn't work with iojs, I assume that iojs should be fixed. If it isn't the case, then liblzf should be fixed. Or even nan should be fixed to deal with this tr1 thing. It'd be super-cool if you can fix it by hacking building.gyp.

Does that make sense?

@bobrik bobrik mentioned this pull request Sep 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants