refactor(file): rename TarFormat to ArchiveFormat#10275
Conversation
Narrow untar to tar archives, add extract_archive and decompress_file, and move format out of extraction options into per-call arguments.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR replaces a tar-specific extraction API ( ChangesArchive Format API Unification
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR renames
Confidence Score: 5/5Safe to merge — this is a mechanical rename with well-contained behavioral changes that preserve existing semantics throughout the call graph. The rename is consistent across all 18 files. The extension() method switch from Option<&'static str> to Option via strum Display correctly uses the first serializer for every variant, matching the original hand-written match. Single-file decompression is properly migrated to the new decompress_file() helper. The untar() guard against non-tar formats is a correctness improvement. Behavioral changes in aqua.rs were already noted in prior review rounds. No files require special attention beyond what was already flagged in previous review rounds. Important Files Changed
Reviews (2): Last reviewed commit: "refactor(file): rename TarFormat to Arch..." | Re-trigger Greptile |
71772f4 to
d041688
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/backend/aqua.rs (1)
2283-2312: Reassess risk:GithubArchiveskipschmod +x, relying on tar-extracted mode bits
- In
src/backend/aqua.rs(AquaPackageType::GithubArchive),make_executablestaysfalse, so the laterfile::make_executable(...)loop (which doeschmod +x) is not run; it only runs forGithubContent/rawand other archive paths.GithubArchivedownloads source as.../archive/refs/tags/{v}.tar.gz, andfile::extract_archive(...)routesTarGztountar(...), which extracts file permissions from the tar headers (so skippingchmod +xshould still preserve executability when the git repo marks files executable).- Remaining risk is only for cases where the archive contains non-executable binaries but mise expects them to be runnable; the registry has 8 top-level
type: github_archivepackages (b3nj5m1n/xdg-ninja, bats-core/bats-core, iamhsa/pkenv, npryce/adr-tools, shellspec/shellspec, tfutils/tfenv, tgenv/tgenv, tofuutils/tofuenv).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/aqua.rs` around lines 2283 - 2312, The GithubArchive branch currently calls file::extract_archive(...) but leaves the local flag make_executable false so the subsequent file::make_executable(...) loop is skipped; to avoid missing chmod for archives whose tar headers don't mark executables, set make_executable = true in the AquaPackageType::GithubArchive branch (or otherwise ensure the post-extract call to file::make_executable(...) runs) so files copied/extracted by file::extract_archive(...) get the same chmod treatment as the GithubContent/raw/archive paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/backend/aqua.rs`:
- Around line 2283-2312: The GithubArchive branch currently calls
file::extract_archive(...) but leaves the local flag make_executable false so
the subsequent file::make_executable(...) loop is skipped; to avoid missing
chmod for archives whose tar headers don't mark executables, set make_executable
= true in the AquaPackageType::GithubArchive branch (or otherwise ensure the
post-extract call to file::make_executable(...) runs) so files copied/extracted
by file::extract_archive(...) get the same chmod treatment as the
GithubContent/raw/archive paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 70287caa-a398-4dc6-bfbb-0af4b9d9fa17
📒 Files selected for processing (18)
src/backend/aqua.rssrc/backend/asset_matcher.rssrc/backend/github.rssrc/backend/http.rssrc/backend/spm.rssrc/backend/static_helpers.rssrc/cli/generate/tool_stub.rssrc/file.rssrc/plugins/asdf_plugin.rssrc/plugins/core/erlang.rssrc/plugins/core/go.rssrc/plugins/core/java.rssrc/plugins/core/node.rssrc/plugins/core/python.rssrc/plugins/core/ruby.rssrc/plugins/core/swift.rssrc/plugins/core/zig.rssrc/plugins/vfox_plugin.rs
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 1 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 2 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 3 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 4 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
|
This PR currently has failing checks and merge conflicts. If this continues for 7 days, it will be closed automatically. This is warning day 5 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
Summary
TarFormat→ArchiveFormatacross extraction call sitesextension()from strumDisplayinstead of a manual matchDepends on #10274.
Test plan
cargo build --all-featuresmise run lint-fixSummary by CodeRabbit