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

Adds iojs support #1

Closed
wants to merge 3 commits into from
Closed

Adds iojs support #1

wants to merge 3 commits into from

Conversation

19h
Copy link

@19h 19h commented Jan 23, 2015

This adds iojs support while maintaining node-legacy support.

Tested & used by us.

Cheers

@19h
Copy link
Author

19h commented Jan 23, 2015

Please publish a native _v42.node alongside TM SDK 1.5.1 or equal, so that it natively works with iojs w/o manual intervention. Selectively using node-legacy while deploying iojs anywhere else disrupts at least a bit :-)

Cheers!

@cb1kenobi
Copy link
Contributor

@KenanSulayman Awesome! I just pulled and ran make, but then things blew up. node-gyp tries to download http://nodejs.org/dist/v1.0.3/node-v1.0.3.tar.gz which obviously doesn't exist. I have iojs installed, but had to restore the symlink for node back to v0.11.15. I'm tinkering with the --tarball and --dist-url options, but can't get node-gyp to download the right file. Any idea?

@cb1kenobi
Copy link
Contributor

Sweet, looks like there's an outstanding PR for node-gyp to add support for io.js! nodejs/node-gyp#564

Perhaps we wait a day or two for that PR to land and the next node-gyp to ship, then merge this so this. Cool?

@19h
Copy link
Author

19h commented Jan 23, 2015

You've got to use the node-gyp packaged with iojs (/deps/npm/bin/node-gyp-bin) .. then it's working.

The node-gyp author didn't respond on merging the changes, so we decided to fork it and distribute our own release with iojs 1.0.2 (1.0.1 had node-gyp from @totallnate).

And besides that: cool!

Thanks 👍

@19h
Copy link
Author

19h commented Feb 7, 2015

Okay, well, damn. I'm not used to this pace of time and then it happened: io.js bumped the ABI to 43. Don't merge this, because I am not sure this is the right model of software delivery for io.js folks.. (having pre-compiled modules, because they're FLYING.).. hm.

I'm at a loss. On the one hand, you need to have this working for releasing the SDK, yet this seems like a never ending problem. Is it really a no-go to compile this for the client on-install?

@cb1kenobi
Copy link
Contributor

@KenanSulayman The problem on our end is node-ios-device is a dependent of ioslib which is baked into the Titanium SDK. Our Titanium SDK release builds are performed on a Linux machine and do not require Xcode. The Objective-C code is actually compiled by the user when they build their app.

So, one thing we could do is extend every public API in ios-device.js to detect if the required module ABI version exists and if not, run node-gyp, then continue to run the requested function. This would be pretty simple to add to the loadIosDeviceModule() function, which would need to be passed a callback. I did a similar thing for windowslib.

The only issue is that node-gyp would have to be a dependency of node-ios-device -or- downloaded on the fly. I'm slightly worried that there may be subtle issues with node-gyp and how we'd be coercing it to work with Node.js and io.js. If we opt to download it on the fly, then it may be a good idea to keep it up-to-date, but then we'd need to support installing it into non-privileged paths since we cannot require the user to be root.

Thoughts?

@corbinu
Copy link

corbinu commented Mar 14, 2015

Curious if there has been any progress here?

It seems to me the easiest thing would be to move node-ios-device to node-pre-gyp and build it on OS X ... release to NPM and then have it pulled by your SDK server when ioslib is pulled in no?

@19h
Copy link
Author

19h commented May 13, 2015

Reiterating this, @cb1kenobi: after carefully watching everything I am quite confident that we won't see any further breaking ABI changes. I spoke to the iojs core people and they acknowledged the mistake that happened which led to me produce the previous comment (#1 (comment)); the ABI will only break in major versions now (1.X.X, 2.X.X.....).

That given, we can safely precompile for ABI-versions 43, 44, ... - deal? :-)

@cb1kenobi
Copy link
Contributor

@KenanSulayman Sorry man, I need a few days before I can visit this. Please bear with me. Thank you!

@cb1kenobi
Copy link
Contributor

Hi @KenanSulayman! Good news! I was able to spend some time and get node-ios-device cleaned up. I've got a new PR that we're testing before publishing: #4. Here's our ticket for it: https://jira.appcelerator.org/browse/TIMOB-18929.

I no longer package built binaries with the release. Instead we ship the gyp file and all of the source files. If you npm install [email protected], it will download and compile using your current environment and things should just work. If you want, you can run make in the node-ios-device directory and it will build all 6 different versions of node-ios-device for the various versions of Node.js and io.js. node-ios-device will then detect if you're running Node or io.js and load the appropriate binary.

Please give that 0.4.1-beta a peek. I hope to have it QA'd and published in the next day or so. Thank you!

P.S. Closing this PR.

@cb1kenobi cb1kenobi closed this May 20, 2015
@cb1kenobi
Copy link
Contributor

@KenanSulayman I found an issue in my release above. I fixed it and now beta2 is the latest!

npm install [email protected]

@19h
Copy link
Author

19h commented May 20, 2015

Awesome, this works extraordinarily well. Great job you did there! Thanks :-)

Will this land in a 3.4.X branch or is it more 4.0-ish?

@cb1kenobi
Copy link
Contributor

@KenanSulayman There will most likely not be any more 3.4.X releases. This fix also missed the 4.0.0 boat. It will be in 4.0.1 though. That won't be too far out.

@jhaynie
Copy link

jhaynie commented May 22, 2015

great job @cb1kenobi !

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.

4 participants