Open
Conversation
Author
|
Seems the rest of clippy errors are on unrelated code. The UI tests really tripped me. 😅 Seems a single |
Merged
Member
|
Hey, I apologize for the delay, I haven't been very active with Ouch, but I just wanted to let you know we'll be reviewing this properly, thanks for the patience! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #830
The code mostly follows Zip implementations, using internal buffers for
Seekrequirement.Unresolved questions:
Which extension to mention in docs?
.squashfsor.sqfs?I'm currently picking
sqfs.There is not canonical extension for squashfs, but there are two semi-official ones:
.squashfs: The most popular one from a quick search on GitHub. Also used by some distros.https://github.com/NixOS/nixpkgs/blob/6576d979e9a64d870b1f298a5c598116891a6c25/nixos/modules/installer/cd-dvd/iso-image.nix#L797
.sqfs: Mentioned inman mksquashfsby squashfs-tools, the reference implementation.https://github.com/plougher/squashfs-tools/blob/9f9bbd79016ff04967800f4301c7a9d0024c0c91/Documentation/manpages/mksquashfs.1#L494
backhanddependsliblzma, a maintainedxz2fork, which conflicts with our dependencyxz2. I disabled the Xz support ofbackhandto mitigate this. How can we fix this?How to pass compression options for squashfs compressor? It is done inside squashfs format (per-chunk compression) thus does not use chained formats. We probably need a new CLI argument for it.
Currently, during compression, it buffers the whole squashfs in memory but we can write directly into file (which supports
Seek). We did a special case in decompression, but it causes some code duplication and we does not do this for Zip compression either. Is there a better way to do so, or should we also add a special case in compression?Do we want streaming compression support
.sqfs.zstd(archive-then-compress instead or compress-per-chunk)? It could be done using some tricks like reserving spaces and emit ZSTD Literal Frame. Not sure about other compression formats. I don't think we want to support this, at least for now, given that decompression cannot be streamed anyway.How should we support multiple input paths? Typically squashfs has a single "root directory". Currently, it reports an error if the input is not a single path to a directory. It would be weird to either "merge" all paths into one tree, or put multiple directory under the root directory (What mode/permission should the root use? Surprising behavior difference between one and more than one paths.).