search

brendan-duncan / archive

Dart library to encode and decode various archive and compression formats, such as Zip, Tar, GZip, ZLib, and BZip2.
MIT License
395 stars 130 forks source link

Remove the dep on package:pointycastle? #331

Closed devoncarew closed 2 months ago

devoncarew commented 3 months ago

Hi - it looks like this package took a dep on package:pointycastle a little while ago:

https://github.com/brendan-duncan/archive/commit/3dd6a0534cbc26c20ab0e6e541c87772836a2c06

It looks like this was for the purpose of supporting encrypted zip files / zip entries? Adding that dep does limit some of the places where we can consume this package (package:pointycastle is a larger dep and is crypto related).

It would be nice to be able to decouple using package:archive with also having to bring in package:pointycastle. Is the usage of pointycastle trivial (something that could be written around)? Or, could we move encrypted zip file support to something like package:archive_decrypt (sitting on top of package:archive, and adding functionality that requires the use of pointycastle)?

(somewhat related, I see that you're in the middle of a large re-write - https://github.com/brendan-duncan/archive/discussions/237)

brendan-duncan commented 3 months ago

I was asked about this earlier, related to building for WASM. My Image package uses Archive for PNG compression and other things, so that also picks up the "now you can't build it for WASM" limitation.

Encrypted zips is something that was very much in demand, so I can't exactly remove support for that, and putting it into a separate package would significantly complicate things. I can barely manage my two Dart packages. But I agree having such a big dependency, and one that limits things, is sucky. I'll see if I can extract the required functions out of pointycastle (it is MIT, I'll attribute them if I do), or maybe re-implement the algorithms, to get rid of that dependency.

I am in the middle of that 4.0 rewrite. I had started it a year ago, but...life. We moved to a new state, among other things. I picked it up again a couple weeks ago. It's going well, still not happy with the interfaces so I'm still playing around with the API, it'll still be a little bit before I think it's ok enough to publish. I'll do another pass to try and minimize backwards incompatible changes, but there will be some. The focus has been on making sure file IO streaming performs well with better memory management...decoding a multi-gig zip will consume relatively little memory because it will just store file handles, and use a single buffer for caching disk IO. This library has accumulated a bunch of baggage over the years, so it's a good chance to make any breaking changes needed to clean up all that stuff. No more 'dynamic', things like that. I can't believe this library is over 11 years old now! It's almost a teenager!

brendan-duncan commented 3 months ago

I removed pointycastle from the 4.0 branch by copying the relevant code from the pointycasle package locally. https://github.com/brendan-duncan/archive/commit/f7b9263b2484b0acf3e48804c6577181fb4f63c0

I can trim down down that code quite a bit, maybe eventually rewrite it to not be so convoluted and sloppy like the original pointycastle code. But at least the dependency is gone now.

I'll clean it up a bit more, then make a main branch version too, since it'll probably still be a little bit before 4.0 is ready.

brendan-duncan commented 3 months ago

I removed pointycastle from the main branch, and published 3.5.0.

devoncarew commented 2 months ago

Encrypted zips is something that was very much in demand, so I can't exactly remove support for that

ack.

and putting it into a separate package would significantly complicate things

We often use mono-repos to organize groups of related packages. The packages go in a sub-dir (like pkgs/, packages/, ...). There's pretty good economies of scale w/ that type of setup, so it may be a little less work to manage a 2nd package than you'd expect.

I'll see if I can extract the required functions out of pointycastle ...

I removed pointycastle from the main branch, and published 3.5.0.

Thanks! I was going to offer to split the package into two if you were open to going that route. Happy to contribute that in the future if the in-lined portions of pointycastle end up being a maintenance burden.