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

libuv changes #97

Closed
kkoopa opened this issue May 5, 2014 · 6 comments
Closed

libuv changes #97

kkoopa opened this issue May 5, 2014 · 6 comments

Comments

@kkoopa
Copy link
Collaborator

kkoopa commented May 5, 2014

There have been some changes to libuv as well. uv_idle_cb and others have had the status argument removed. Might be other breaking changes as well. Should we address this somehow?

@rvagg
Copy link
Member

rvagg commented May 5, 2014

yeah I think so, they come under the whole banner of what we're doing, do we try and stick with C-style naming in this case though? could get ugly having the mix

@bnoordhuis
Copy link
Member

As the main instigator of the libuv API changes I can tell you you're going to be in a world of pain if you want to cover all API changes.

The uv_idle_cb prototype change is a fairly minor thing but for example error handling has been completely overhauled, see nodejs/node-v0.x-archive@3ee4d3f. If you want to shim that, you have to basically shim the whole API.

That's just one example, there are plenty more. I would suggest taking a good look at git diff origin/v0.10..origin/master -- include/uv.h before deciding to go down that path. :-)

@kkoopa
Copy link
Collaborator Author

kkoopa commented May 5, 2014

I was afraid of something like that. Maybe not cover all changes then, but the most common cases. Limiting it to usage of libuv within node addons should reduce the scope somewhat. All I really need is the idle callback with the unused status argument, so I don't have to ifdef its declaration and definition. Might do the rest of the bunch with removed status args in that regard (I think it was less than 10 functions). After that I don't know what more is in use and needed in a typical node module.

@rvagg
Copy link
Member

rvagg commented May 5, 2014

hm, that sucks then

Perhaps we're better off focusing on higher-level utilities that can alleviate some of the pain, like NanAsyncWorker. We could split these into a separate header file if we come up with more.

@bnoordhuis
Copy link
Member

All I really need is the idle callback with the unused status argument, so I don't have to ifdef its declaration and definition.

For that particular case, you could just uv_idle_start(handle, (uv_idle_cb) cb). The status argument in v0.10 is uninteresting anyway because it's always zero.

@kkoopa
Copy link
Collaborator Author

kkoopa commented May 5, 2014

Yeah, that should work. Didn't think of using a typecast although I had gleaned that the status argument was unused. However, I think I'll just stick with conditional compilation, it is messier but obscures less and I only need a single use (that I know of). Hopefully there won't be a need for a nan-uv. (Or is that nauv?)

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

No branches or pull requests

3 participants