Conversation
Issue #64 asked a write failure to name which entry was at fault. zip and 7z do. tar has two write paths, and the one an archive with nothing renamed takes, which on Unix is nearly every archive, still answered in the tar crate's own words: `failed to unpack \`/…/out/a.txt/b.txt\``. No entry named, and the absolute destination loose in the sentence. `unpack_in` itself is untouched. It is the traversal guard tar has always used and its canonicalizing containment check is what stops a write from following a symlink already in the output directory; only its error is dressed. Its error needs unwrapping first, because it is a stack of wrappers each adding the destination again: [0] failed to unpack `/…/out/a.txt/b.txt` [1] failed to unpack `a.txt/b.txt` into `/…/out/a.txt/b.txt` [2] Not a directory (os error 20) Only the last says what went wrong, and it is the register the other two formats answer in. Using level 0 inside an `Entry`, which names the destination itself, printed the path three times. before: failed to unpack `/…/out-tar/a.txt/b.txt` after: cannot write entry "a.txt/b.txt" to /…/out-tar/a.txt/b.txt: Not a directory (os error 20) zip: cannot write entry "a.txt/b.txt" to /…/out-zip/a.txt/b.txt: File exists (os error 17) The test that should have caught this passed for the wrong reason. It asserted `message.contains("a.txt/b.txt")`, which tar satisfied because the entry name is a substring of the destination path, so the guarantee was never checked at all. It now requires the name quoted as an entry, and requires the real cause, and it fails against the old code. Closes #93
…icts -o Two issues in `run_compress`, both decidable long before anything is written. **#67.** The source is canonicalized, for good reasons: `.`, `..` and a trailing slash have to resolve to something with a usable name, and an output that aliases the source can only be spotted against a resolved path. That resolved path then leaked into what the user was told. Type six characters, get an absolute path back: $ collapse compress ./sub/a.txt -f zip before: Created /Users/…/scratch/misc.A4Z7/sub/a.txt.zip after: Created ./sub/a.txt.zip Both spellings are kept now. The resolved one still drives the guards, the engine and the arcname, which has to be the file's real name. The typed one is what gets reported. A source with no name of its own falls back to the resolved path, which is also what keeps a directory's archive beside it rather than inside it, where the guards would then refuse it. **#75.** `--format` won over `-o`'s extension in silence, producing a file whose name lies about its contents and that this same CLI then rejects: $ collapse compress notes.txt -f tar -o mixed.zip Created mixed.zip $ collapse extract mixed.zip -o out error: Could not find EOCD Refused now, before anything is read or written. A warning was the other candidate and is worse: it goes to a terminal nobody reads in a script, and what is left behind is still a broken file with a misleading name. An extension that names no known format is deliberately **not** a contradiction. `-o backup.bin -f 7z` is a choice, and a guard that refused it would be overreaching; a test pins that. Every new test was checked against the unfixed code. Reverting #67 fails two, reverting #75 fails one, and making the guard over-refuse fails two more. Closes #67 Closes #75
Name the entry tar could not write, and two CLI answers that were wrong
All eight version strings and the four lockfiles. The release guard checks two of them (issue #77), so the rest are by hand and by eye, and `cargo build --locked` was run afterwards since that is what `make cli/release` uses and a drifted lockfile fails there first. Minor rather than patch. The three fixes since v0.9.0 are fixes, but #75 changes behaviour a user can notice: `collapse compress notes.txt -f tar -o mixed.zip` used to succeed and now refuses, because the file it produced was a tar named `.zip` that this same CLI would not extract.
Release 0.10.0
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
1 participant
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.
Everything on
devsince v0.10.0's predecessor: three fixes and the version bump.The one behaviour change
--formatand-oare no longer allowed to contradict each other:A command that used to succeed now refuses, which is why this is a minor. What it used to produce was a tar named
.zip, and this same CLI then refused to extract it, so the success was worse than the refusal.An extension naming no known format is deliberately not a contradiction:
-o backup.bin -f 7zis a choice, and a test pins that the guard does not overreach.Two things the tool was saying wrong
#67. It answered with a path nobody typed.
collapse compress ./sub/a.txtreported/Users/…/scratch/sub/a.txt.zip, because the source is canonicalized and the resolved spelling leaked into the report. Both are kept now: the resolved one still drives the guards, the engine and the arcname, since that is the name stored inside the archive; the typed one is what gets reported.#93. tar did not say which entry it could not write, so the guarantee #64 asked for reached zip and 7z but not the branch nearly every archive takes:
The dependency's error needed unwrapping first, since it arrives as three wrappers each repeating the destination, and only the innermost says what actually went wrong.
unpack_initself is untouched: it is the traversal guard tar has always used.Worth recording: the test that should have caught #93 passed for the wrong reason, asserting a bare
containsthat tar satisfied because the entry name is a substring of the destination path. It now requires the name quoted as an entry.Verification
620 Rust tests and 116 Vitest, on macOS, Linux and Windows. Every new test was checked against the unfixed code first; the four mutations tried are listed in #109.
Closes #67
Closes #75
Closes #93