Repository navigation
feat(brew): add support for font artifact types from git urls - #11781
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHomebrew casks now support Git sources with optional branch selection and ChangesHomebrew cask Git staging
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CaskInstall
participant GitRepository
participant StagingDirectory
CaskInstall->>GitRepository: Clone .git source at selected branch or HEAD
GitRepository->>StagingDirectory: Copy selected source contents
StagingDirectory->>CaskInstall: Return clean staged source
CaskInstall->>CaskInstall: Install from staged source
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/system/packages/brew/cask.rs (1)
677-683: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd focused tests for the new archive contracts.
Add tests for Git branch URL construction and
only_pathrelocation. Include a missing path and a parent-directory path. These cases prevent regressions that can empty or corrupt the cask extraction cache.Also applies to: 756-787
🤖 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/system/packages/brew/cask.rs` around lines 677 - 683, Add focused tests covering the new archive contracts around git_tarball_url and the only_path relocation logic: verify branch-based Git archive URL construction, missing-path handling, and relocation to a parent-directory path. Keep the tests targeted to these behaviors and assert outcomes that prevent empty or corrupted cask extraction caches.
🤖 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.
Inline comments:
In `@src/system/packages/brew/cask.rs`:
- Around line 758-768: Validate cask.url_specs.only_path before constructing
nested: reject empty, absolute, current-directory, and parent-directory
components, and return an error if nested is not a directory. In the
find_extracted_top_dir relocation flow, relocate entries only after validation
succeeds, then remove top_dir only after the relocation completes successfully.
---
Nitpick comments:
In `@src/system/packages/brew/cask.rs`:
- Around line 677-683: Add focused tests covering the new archive contracts
around git_tarball_url and the only_path relocation logic: verify branch-based
Git archive URL construction, missing-path handling, and relocation to a
parent-directory path. Keep the tests targeted to these behaviors and assert
outcomes that prevent empty or corrupted cask extraction caches.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 7fb56f4c-fdec-492f-8c0e-a842f40299e7
📒 Files selected for processing (1)
src/system/packages/brew/cask.rs
Greptile SummaryThis PR replaces archive conversion for
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (4): Last reviewed commit: "feat(brew): add support for font artifac..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR extends the built-in Homebrew cask package manager to support casks whose url points at a Git repository (e.g. font casks like powerline/fonts). It teaches mise to translate a .git URL plus a branch into a GitHub archive tarball, download/cache it consistently, and—when an only_path is configured—hoist the contents of a nested subdirectory to the extraction root so the existing artifact-matching logic can find them.
Changes:
- Adds a
CaskUrlSpecsstruct (branch,only_path) deserialized from cask metadata and wires it intoCask. - Adds
git_tarball_urlto build a GitHubarchive/refs/heads/{branch}.tar.gzURL, used for download, cache filename hashing, and the raw-binary copy path. - Adds
only_pathrestructuring inextract_archive(with path-traversal validation) plus afind_extracted_top_dirhelper, and unit tests.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/system/packages/brew/cask.rs`:
- Around line 7306-7351: Update the git fixture around the repository setup and
Cask URL specs to explicitly create and select a dedicated non-default branch.
After committing the baseline files in the initial branch, create the test
branch, modify the font content, commit the change there, and set
CaskUrlSpecs.branch to that branch so the test verifies branch selection
independently of Git’s configured default.
- Around line 695-708: The clone setup around CloneOptions must not default
url_specs.branch to "HEAD". Preserve the absent-branch case without calling
CloneOptions::branch, and apply branch only when a concrete branch is present;
continue applying the PR ref through the existing pr handling.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 3122acc7-adff-472e-813b-e0dcc788e10d
📒 Files selected for processing (1)
src/system/packages/brew/cask.rs
Summary by CodeRabbit
New Features
Bug Fixes