Adding Zip64 support to Zip::Writer #7103
Comments
|
Nice! If the performance is similar or faster and it provides more features, I can't see why we wouldn't want to replace the existing implementation with that one. |
|
I'd suggest moving the current zip library out of the stdlib to a repo on the Moving logic out of |
|
So what would be the preferred course of action? Should I make a repo with only the ZIP library from stdlib and change/add/remove things there, and then it becomes a |
|
As long as ZIP module is in stdlib I'd suggest contributing there and extracting it to separate shard once final decision for stdlib "diet" has been made. No need to fragment the ecosystem before anything is decided (if at all). |
|
Great idea @RX14 . @julik could have more rights on the repo, therefore freeing time for core maintainers. To sum-up:
I strongly think this type of splits worth it, PR are stacking and depends only on core members for merging. People mastering specific fields like crypto, system, http etc got slowed down. Not really motivating to actively contribute if our work become staled |
|
@j8r It seems too early for "summing-up" since it's up to the core team members to decide and this is still unresolved. Personally I'm against fragmentation stdlib like that - ZIP is sth quite widely used which doesn't changes very often and I see no good reason to keep it out of stdlib. |
|
@Sija It will still be in the stdlib, this isn't exclusive. Look at submodules in https://github.com/rust-lang/rust/tree/master/src and https://github.com/rust-lang/rust/blob/master/Cargo.lock . This approach is clearly possible, the only remaining thing is a go from core members. Personally, I don't think Crystal has the ressource, i.e core members, to support properly the whole stdlib only by themselves. Especially since there are people who are willing to help them. |
|
I may be wrong, they probably can support the whole language without delegating work to additional maintainers... For sure, that's up to them to see. |
|
@julik the best course would be to create a Anybody will be able to just use it, without waiting for core team reviews, an eventual merge, wait again for the next crystal release, and so on. We can decide later whether we merge it back, distribute the shard with the stdlib (like ruby does for some gems), or drop it from stdlib and keep it as a shard only. |
|
@ysbaddaden that sounds error prone. What's the benefit of that for @julik over using the shard he already wrote then? If all the changes to stdlib zip are made in a shard its less likely the changes will be merged since there's no core team member reviewing the changes. Anyway, http depends on |
|
Do we really want that any change in the stdlib require a review of a core member? Wouldn't be better to have teams responsible to different part of the language, instead of one for the whole? The core member will be able to focus on the compiler itself. That's already more or less the case without knowing it: PCRE, OpenSSL, bdwgc are done externally, why not HTTP, Zip or Zlib then? |
@j8r Yes, Crystal is the language and the standard library. The code in the standard library has to have a base level of quality, readablity and test coverage. If the ones that contribute to the standard library disappear and the code is a mess, nobody will be able to maintain it. That's why I'm more and more inclined to split out things outside of the standard library, because it frees the core developers from this task. Also, not all core developers know everything (for example I implemented the current zip implementation and I had to learn it first, but it would have been faster if someone else knew zip beforehand and implemented it as a shard, as needed). |
|
I would rather mean, having shards developing in parallel, and then asking to integrate X version in the stdlib. A bit like what it's done with LLVM when supporting a new version, or OpenSSL. Core member, maintainer or an other name, there are likely people out there capable of doing a great job in the stdlib without knowing Crystal's compiler internals. At the end, it's all about trust: do we trust people X to do a quality Y shard for the stdlib? |
|
@j8r Not all the current core developers are compiler developers (although they do need a robust understanding of how the compiler is architectured and it's externally visible warts). Removing things for the core team to do does not magically mean the compiler will get developed faster. Recruitment to the core team has always been about aligning design and review sensibilities. Since the stdlib cannot be changed after 1.0 it means the stdlib needs a lot higher standard of design and review than external shards. Removing stuff from the stdlib is fine, but it's a balance between making crystal useful for basic tasks without requiring shards and the reduced workload. I argue that removing zip is probably not worth it on balance because the |
|
Ok. I am going to figure out how to test the stdlib parts in isolation (still very new to the language) and then gradually open patches to add Zip64 support to the parts that exist. I might have to rework the writing of the entries as stated previously but that is IMO a good compromise - and won't change the API surface. The only concern I have is that the API allows passing I will keep cr_zip_tricks for the time being though since it was purpose-made and we need it to do stuff. |
|
@julik you'll want to check out the crystal repo, use I'm fine with small breaking changes to make stuff like |
We have ported our zip_tricks library to Crystal at least in as far as writing ZIPs is concerned. What we needed was Zip64 support and the ability to manually "simulate" writes for keeping offsets. The low-level implementation differs slightly from Crystal's ZIP module, but the high-level API is very, very similar (a streaming Writer object that handles any IO within a block). The low-level difference is that there is a single Writer object that handles low-level byte packing and writing, and that is the object that handles the tricky logic with switching Zip64 on and off, setting the correct masked permission bits on things and so forth. The shard is in https://github.com/WeTransfer/cr_zip_tricks
So, in light of #5215 etc would it be something that belongs in the Crystal stdlib? I could potentially port the writing to the standard library, but I would prefer to then remove the "self writing" feature in
Entryso that the serializer we already have can be reused and stay in one module. The Entry object then gets a much smaller API surface as well. @asterite would love to know your thoughts on thisThe text was updated successfully, but these errors were encountered: