search

chbrown / gzbz

A streaming compression / gzip library for Node.js
MIT License
0 stars 1 forks source link

bzip2 doesn't seem to work #2

Open zingaburga opened 10 years ago

zingaburga commented 10 years ago

Adding '-lbz2' next to '-lz' in binding.gyp seems to fix it for me.

Thanks for the module by the way.

atheken commented 10 years ago

@zingaburga -- I don't know much about node-gyp, but if this is required to get bzip working, then perhaps you can fork, update and do a pull request?

zingaburga commented 10 years ago

It's pretty easy to do, just find '-lz' in binding.gyp and replace it with '-lz','-lbz2' ...

You'll obviously need the bzip2 dev library installed for it to work (package "libbz2-dev" on Debian)

atheken commented 10 years ago

@zingaburga Yes, I understand that, but like I said, I don't know the implications of changing the flags, but it seems you do.

If changing the flags fixes it (and doesn't break gzip), why not contributed it back to the project?

Otherwise, people using the package need to come here and find this thread or hack around in the node_modules folder (not really a good thing). Little fixes help keep these projects going.

I don't own or have any pull on this project, it was simply a suggestion that you have some valuable knowledge that could make the package just a little bit better.

zingaburga commented 10 years ago

I have no power to update this package, and I don't intend to maintain a fork over such a trivial change.
chbrown needs to update the package for it to be effective (which also affects npm), but if you or anyone else wishes to maintain a fork, then you have my blessing.

Adding the flag (not changing it) won't break anything. The -l* flags tell the linker to link to a library. -lz basically says, find libz.a in the lib directory and link the executable to it. -lbz2 tells the linker to link to libbz2.a. The code contains references to libz (aka zlib) and libbz2, but only libz is linked in, hence only the gzip functions work. Linking to both libraries allows both to work.

why not contributed it back to the project?

Also laziness, not that it really matters.
I'd also assert that anyone working with NodeJS at the moment should get comfortable with 'hacking around in the node_modules folder'. Packages, in general, seem rather immature at this stage, and bugs should be expected, meaning that you should be prepared to identify/fix them.

atheken commented 10 years ago

OK, so altering it to -lbz2 effectively breaks zlib? instead should this be -lz -lbz2?

My point of saying "hack around node_modules" was not to suggest people not know how NPM works, but rather that that altering it locally in a way that is not version controlled is inviting issues when sharing a project with others. Unless you commit node_modules to git, this will leave you with "works on my machine" problems that are easily avoided.

Regardless, you've made your position clear.

zingaburga commented 10 years ago

Yes, changing -lz means that you're no longer linking to libz, which means that bzip will work, but zlib will stop working. You need to link to both libraries.