search

Rantanen / node-mumble

Mumble client in Node.js
MIT License
155 stars 48 forks source link

Nan 2, iojs 3 support #63

Closed justinmchase closed 9 years ago

justinmchase commented 9 years ago

I have forked the various mumble projects to add electron build support. I just wanted to let you know that I have upgraded them all to build under nan-2, iojs-3. I plan to come back here and send proper pull requests so we can be as in sync as possible but if you decide to upgrade sooner than i can do that, you can reference electron-opus as a guide.

Most of the changes are pretty straight forward replacements (e.g. NanNew -> Nan::New) but there was one breaking change you should know about that was fairly difficult for me to figure out, which was the semantic change of Nan::NewBuffer being changed to not copy and also take ownership of the provided memory. This caused a crash in the GC as it was attempting to free memory that had already been freed. Changing it instead to be Nan::CopyBuffer fixes the bug without memory leak as well.

Thanks again for all the good work on this project.

Rantanen commented 9 years ago

Thanks for the tips! I'm most likely going to be quite busy with work this week and some of the next one. Might find some times during the weekend.

If not, I'll look forward to your pull requests. :)

tjhorner commented 9 years ago

electron build support

Sounds fun :D

mattvperry commented 9 years ago

I was able to integrate @justinmchase 's changes to node-mumble's dependencies such that node-mumble will now build on node4 using nan2.

Rantanen commented 9 years ago

Yeah. Just tested this.

Travis had trouble with Node 4 builds thanks to GCC 4.8 requirements, but sorted those out so now we got working Travis-builds again as well. Thanks!