Added stream-like read API and write API to node-zipfile #16

wants to merge 9 commits into


None yet

3 participants



As discussed previously, I have performed some changes in node-zipfile. They are the following:

  • Made it impossible to have multiple concurrent calls to libzip functions by referencing opened objects
    (file, source) and throwing an exception when they are used.

  • Added a stream-like API for file read. I figured this out because

    • I prefer that an asynchronous read does not necessarily read all the file at once.
    • libzip actually has an API that, on purpose, imitates usual file system IO.

    Thus, the code for this API in zipfile.js is actually taken from node's code..

  • Added write API. For now, one can only add/replace local files and local directories. Adding buffers should
    be fairly trivial to add.

    The trick here is that adding/replacing in libzip returns immediately, Actual write is done when closing
    (saving in the current API). Thus, it is not possible to stream-line write operations in a stream-oriented

I am aware that those changes represent a pretty huge modification of you codebase. I am submitting a pull-request in the hope that you could accept them. However, feel free to refuse them as well, I would have no problem maintaing it myself -- it is just that I believe that there are already way to many node-zipfile bindings floating around to add yet another one..


Mapbox member

great, thanks very much for the contribution. Looks good. I am just headed offline for 2 weeks however, so I'll review when I'm back and look into applying the merge then. thanks for the patience.

Mapbox member

hey there. now that node v6 is out, and things have changed a bit around async stuff. This ideally could be revised so that it automatically merges and works with node v6. If you are on to new things, no worries I will try to take a look once node v6 settles out a bit.


+1 to this..

I tried to merge myself with current head but it appears there is some reworking going on with 'node_zipfile.cpp' for the 0.6.x update - and i'm not a C++/node extension expert :)
I'll look at it again later to see if I can get it working (am working on something that could using this streaming interface right now) but might be better if someone how know wtf they were doing did it :) (doesn't otherwise seem to be a huge merge (assuming it works))

Mapbox member

ya, wish I had of merged it long ago, apologies. It will require more wrangling time than I have to update to node v6. Currently I'm pushing on trying to get node-zipfile compiled on windows.



I've had a try at this. A lot of C headers have been changed since 0.4.x, which I use here. Thus, I cannot compile the new code.

It would need to work around the new uv_work_t type, using the NODE_VERSION_AT_LEAST macro..

Mapbox member

this patch should ideally be revised to work with node v0.6.x, which, as you note uses libuv now and uv_work_t. Current node-zipfile master code is already using these libuv types. Thoughts?


It's feasible but I want to use node 0.4.x here for production reasons. I'll see if I find time to have another 0.6.x install and update the patch to make it compile with both versions.

Mapbox member

curious, what is holding you back from 0.6.x?


I'm planning on using on heroku at some point...

Mapbox member

closing, this is stale. sorry we missed getting it in. would review an updated pull.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment