Fix panic when --policy uses a commit SHA refspec#1414
Conversation
Detect full-length hex commit SHAs (SHA-1: 40 chars, SHA-256: 64 chars) and skip `with_ref_name` which only accepts symbolic refs. Instead, clone the repo fully and run `git checkout <sha>` after checkout to switch to the requested commit. Agent-Logs-Url: https://github.com/open-telemetry/weaver/sessions/80002501-91c1-453a-990b-88b6664f0edf Co-authored-by: lmolkova <2347409+lmolkova@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1414 +/- ##
=======================================
- Coverage 82.4% 82.3% -0.2%
=======================================
Files 120 120
Lines 10092 10108 +16
=======================================
+ Hits 8320 8323 +3
- Misses 1772 1785 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // checked out the default branch. Now switch to the requested commit. | ||
| if is_sha { | ||
| let sha = refspec.as_ref().expect("is_sha implies Some"); | ||
| let output = std::process::Command::new("git") |
There was a problem hiding this comment.
@copilot use gix.
every other git op in this crate goes through it, and Cargo.toml:34 already enables worktree-mutation, so the checkout API is on the dep tree for free. Roughly:
repo.rev_parse_single(sha)?, update HEAD, write the worktree. Also lets you
skip main_worktree above on the SHA path and avoid the double checkout.
There was a problem hiding this comment.
Replaced std::process::Command with pure gix APIs in dcf3e8e. The SHA path now skips main_worktree() entirely and uses ObjectId::from_hex → find_object → peel_to_tree → index_from_tree → worktree::state::checkout to write the worktree directly, avoiding the double checkout.
Use gix's ObjectId::from_hex, find_object, peel_to_tree, index_from_tree, and worktree::state::checkout instead of shelling out to `git checkout`. For the SHA path, skip main_worktree() entirely and checkout the requested commit directly, avoiding a double checkout. Agent-Logs-Url: https://github.com/open-telemetry/weaver/sessions/3f847f32-e10a-4518-8deb-b4f2d0d99f88 Co-authored-by: lmolkova <2347409+lmolkova@users.noreply.github.com>
| let repo = checkout.persist(); | ||
| Self::checkout_sha(&repo, sha, url)?; | ||
| } else { | ||
| let _repo = checkout |
There was a problem hiding this comment.
Why is the result unused? Please add a comment or rationale behind what this is doing - is it for an effect? if so comment on what we expect to have happen and why we're not returning a value.
gix::clone::PrepareFetch::with_ref_nameonly accepts symbolic refs (branches/tags) and panics on raw object IDs. Passing a 40-char commit SHA as a refspec causes an unrecoverable panic in gix internals.Changes
is_commit_shahelper — strict detection of full-length hex SHAs (40-char SHA-1, 64-char SHA-256). Intentionally rejects short hashes to avoid false positives on branch/tag names likedeadbeef.try_from_git_url— when refspec is a commit SHA, skipwith_ref_name(which would panic) and clone all refs. Instead of callingmain_worktree()(which would checkout the default branch), use a dedicatedcheckout_sha()helper that checks out the requested commit directly via gix APIs (ObjectId::from_hex→find_object→peel_to_tree→index_from_tree→worktree::state::checkout), avoiding a double checkout. Non-existent SHAs produce a properGitErrorinstead of a panic.is_commit_shacovering valid/invalid inputs.