From d26b54869525d781c03f2a80c1d1c77e29e6cd2c Mon Sep 17 00:00:00 2001 From: jdx <216188+jdx@users.noreply.github.com> Date: Wed, 29 Apr 2026 09:52:48 -0500 Subject: [PATCH 1/3] fix(task): avoid gix panic when cloning a remote task by commit SHA Cloning a remote git task with `?ref=` (e.g. `git::https://github.com/foo/bar.git//path?ref=`) panicked in `gix::prepare_clone().with_ref_name()` with "we map by name only and have no object-id in refspec". gix-refspec parses any 40- or 64-char hex string as an ObjectId refspec, but `find_custom_refname` only handles name-based matches and crashes on object-id sources. Detect SHA-shaped refs in `Git::clone` and skip `with_ref_name`/`-b` (neither gix nor `git clone -b` accept bare SHAs anyway). Also drop `--depth 1` in the CLI path for SHA refs since shallow clones may not contain the requested object. After the clone completes, fetch and check out the SHA via the existing CLI-backed `update_ref`. Fixes #9472. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/git.rs | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 116 insertions(+), 7 deletions(-) diff --git a/src/git.rs b/src/git.rs index fe9b24877d..f77c0d321d 100644 --- a/src/git.rs +++ b/src/git.rs @@ -119,11 +119,22 @@ impl Git { if let Some(parent) = self.dir.parent() { file::mkdirp(parent)?; } + // gix's `with_ref_name` and git CLI's `-b` only accept branch/tag names. + // If the caller passed a commit SHA, clone without a ref and then + // fetch + checkout the SHA explicitly. gix in particular panics + // ("we map by name only and have no object-id in refspec") if a SHA + // is fed to `with_ref_name`. + let sha_branch = options + .branch + .as_deref() + .filter(|b| looks_like_sha(b)) + .map(str::to_string); + let named_branch = options.branch.as_deref().filter(|b| !looks_like_sha(b)); if Settings::get().libgit2 || Settings::get().gix { debug!("cloning {} to {} with gix", url, self.dir.display()); let mut prepare_clone = gix::prepare_clone(url, &self.dir)?; - if let Some(branch) = &options.branch { + if let Some(branch) = named_branch { prepare_clone = prepare_clone.with_ref_name(Some(branch))?; } @@ -133,6 +144,9 @@ impl Git { prepare_checkout .main_worktree(gix::progress::Discard, &gix::interrupt::IS_INTERRUPTED)?; + if let Some(sha) = sha_branch { + self.update_ref(sha, false)?; + } return Ok(()); } debug!("cloning {} to {} with git", url, self.dir.display()); @@ -154,13 +168,15 @@ impl Git { .arg("-o") .arg("origin") .arg("-c") - .arg("core.autocrlf=false") - .arg("--depth") - .arg("1") - .arg(url) - .arg(&self.dir); + .arg("core.autocrlf=false"); + // `--depth 1` is incompatible with checking out an arbitrary SHA later, + // so do a full clone when the caller passed a SHA. + if sha_branch.is_none() { + cmd = cmd.arg("--depth").arg("1"); + } + cmd = cmd.arg(url).arg(&self.dir); - if let Some(branch) = &options.branch { + if let Some(branch) = named_branch { cmd = cmd.args([ "-b", branch, @@ -171,6 +187,10 @@ impl Git { } cmd.execute()?; + + if let Some(sha) = sha_branch { + self.update_ref(sha, false)?; + } Ok(()) } @@ -314,6 +334,17 @@ fn get_git_version() -> Result { Ok(version.trim().into()) } +/// Heuristic for whether a ref string is a commit SHA (full SHA-1 or SHA-256). +/// +/// Branch and tag names that happen to be all-hex would also match, but git +/// disallows refs that are valid object IDs anyway (see `git check-ref-format`), +/// so the heuristic is safe in practice. Abbreviated SHAs are intentionally not +/// matched — they are ambiguous with short branch names and need server-side +/// resolution before they can be checked out. +fn looks_like_sha(s: &str) -> bool { + matches!(s.len(), 40 | 64) && s.bytes().all(|b| b.is_ascii_hexdigit()) +} + impl Debug for Git { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("Git").field("dir", &self.dir).finish() @@ -337,3 +368,81 @@ impl<'a> CloneOptions<'a> { self } } + +#[cfg(test)] +mod tests { + use super::{CloneOptions, Git, looks_like_sha}; + use std::process::Command; + + #[test] + fn sha_detection() { + assert!(looks_like_sha("0123456789abcdef0123456789abcdef01234567")); + assert!(looks_like_sha( + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + )); + assert!(!looks_like_sha("main")); + assert!(!looks_like_sha("v1.2.3")); + assert!(!looks_like_sha("abcdef1")); // short SHA not supported + assert!(!looks_like_sha("")); + assert!(!looks_like_sha("g123456789abcdef0123456789abcdef01234567")); // non-hex + } + + /// Regression test for https://github.com/jdx/mise/discussions/9472: + /// gix's `with_ref_name` panics ("we map by name only and have no + /// object-id in refspec") when given a commit SHA. Our `clone()` must + /// detect that case and fall back to a plain clone + checkout. + #[test] + fn clone_by_sha_does_not_panic() { + let tmp = tempfile::tempdir().unwrap(); + let src = tmp.path().join("src"); + let dst = tmp.path().join("dst"); + std::fs::create_dir_all(&src).unwrap(); + + let git_in_src = |args: &[&str]| { + let out = Command::new("git") + .args(args) + .current_dir(&src) + .output() + .expect("spawn git"); + assert!( + out.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + out + }; + git_in_src(&["-c", "init.defaultBranch=main", "init", "-q"]); + git_in_src(&[ + "-c", + "user.email=t@t", + "-c", + "user.name=t", + "commit", + "-q", + "--allow-empty", + "-m", + "first", + ]); + let sha = String::from_utf8(git_in_src(&["rev-parse", "HEAD"]).stdout) + .unwrap() + .trim() + .to_string(); + assert_eq!(sha.len(), 40); + + let url = format!("file://{}", src.display()); + Git::new(&dst) + .clone(&url, CloneOptions::default().branch(&sha)) + .expect("clone with SHA must not panic and must succeed"); + + let head = Command::new("git") + .args(["rev-parse", "HEAD"]) + .current_dir(&dst) + .output() + .unwrap(); + assert_eq!( + String::from_utf8(head.stdout).unwrap().trim(), + sha, + "worktree should be checked out at the requested SHA" + ); + } +} From 337eda7b38fe428590328bffd028be7b45adde8b Mon Sep 17 00:00:00 2001 From: jdx <216188+jdx@users.noreply.github.com> Date: Wed, 29 Apr 2026 09:58:08 -0500 Subject: [PATCH 2/3] fix(task): skip redundant fetch when checking out SHA after clone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After clone, the worktree already has every reachable object, so calling update_ref (which fetches `:` from origin first) is both redundant and brittle — servers without `uploadpack.allowReachableSHA1InWant` reject fetching by SHA. Replace the post-clone update_ref call with a plain detached checkout via a new private `Git::checkout` helper. Addresses review feedback on #9473. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/git.rs | 44 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/src/git.rs b/src/git.rs index f77c0d321d..0795308e64 100644 --- a/src/git.rs +++ b/src/git.rs @@ -67,6 +67,38 @@ impl Git { self.update_ref(gitref, true) } + /// Detached `git checkout --force ` with no fetch. Used after `clone` + /// when the caller asked to land on a specific SHA — the clone has already + /// pulled all reachable objects, so a fetch with a `:` refspec + /// would be redundant (and on servers without + /// `uploadpack.allowReachableSHA1InWant`, would fail). + fn checkout(&self, gitref: &str) -> Result<()> { + let cmd = git_cmd!( + &self.dir, + "-c", + "advice.detachedHead=false", + "-c", + "advice.objectNameWarning=false", + "checkout", + "--force", + gitref, + ); + let res = cmd + .stderr_to_stdout() + .stdout_capture() + .unchecked() + .run() + .map_err(|err| eyre!("git failed: {cmd:?} {err:#}"))?; + if !res.status.success() { + return Err(eyre!( + "git failed: {cmd:?} {}", + String::from_utf8_lossy(&res.stdout) + )); + } + touch_dir(&self.dir)?; + Ok(()) + } + fn update_ref(&self, gitref: String, is_tag_ref: bool) -> Result<(String, String)> { debug!("updating {} to {}", self.dir.display(), gitref); let exec = |cmd: Expression| match cmd.stderr_to_stdout().stdout_capture().unchecked().run() @@ -121,14 +153,10 @@ impl Git { } // gix's `with_ref_name` and git CLI's `-b` only accept branch/tag names. // If the caller passed a commit SHA, clone without a ref and then - // fetch + checkout the SHA explicitly. gix in particular panics + // check out the SHA explicitly. gix in particular panics // ("we map by name only and have no object-id in refspec") if a SHA // is fed to `with_ref_name`. - let sha_branch = options - .branch - .as_deref() - .filter(|b| looks_like_sha(b)) - .map(str::to_string); + let sha_branch = options.branch.as_deref().filter(|b| looks_like_sha(b)); let named_branch = options.branch.as_deref().filter(|b| !looks_like_sha(b)); if Settings::get().libgit2 || Settings::get().gix { debug!("cloning {} to {} with gix", url, self.dir.display()); @@ -145,7 +173,7 @@ impl Git { .main_worktree(gix::progress::Discard, &gix::interrupt::IS_INTERRUPTED)?; if let Some(sha) = sha_branch { - self.update_ref(sha, false)?; + self.checkout(sha)?; } return Ok(()); } @@ -189,7 +217,7 @@ impl Git { cmd.execute()?; if let Some(sha) = sha_branch { - self.update_ref(sha, false)?; + self.checkout(sha)?; } Ok(()) } From 94971d539943ca910898245ba3462a26102bf716 Mon Sep 17 00:00:00 2001 From: jdx <216188+jdx@users.noreply.github.com> Date: Wed, 29 Apr 2026 10:01:08 -0500 Subject: [PATCH 3/3] test(git): cover both gix and CLI clone paths with non-default-branch SHA Greptile pointed out that the regression test only exercised the default-branch SHA case and didn't pin the backend, so the gix panic path could go untested. Park the target SHA on a feature branch (so a shallow / single-branch clone would miss it) and run the test against gix=true and gix=false in sequence via Settings::override_with, restoring the prior values afterward. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/git.rs | 103 ++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 74 insertions(+), 29 deletions(-) diff --git a/src/git.rs b/src/git.rs index 0795308e64..20167b4a2b 100644 --- a/src/git.rs +++ b/src/git.rs @@ -400,6 +400,7 @@ impl<'a> CloneOptions<'a> { #[cfg(test)] mod tests { use super::{CloneOptions, Git, looks_like_sha}; + use crate::config::Settings; use std::process::Command; #[test] @@ -419,17 +420,20 @@ mod tests { /// gix's `with_ref_name` panics ("we map by name only and have no /// object-id in refspec") when given a commit SHA. Our `clone()` must /// detect that case and fall back to a plain clone + checkout. + /// + /// Covers both the gix backend (where the panic originates) and a SHA + /// reachable only from a non-default branch (so the clone must be full, + /// not shallow, for the checkout to find the object). #[test] fn clone_by_sha_does_not_panic() { let tmp = tempfile::tempdir().unwrap(); let src = tmp.path().join("src"); - let dst = tmp.path().join("dst"); std::fs::create_dir_all(&src).unwrap(); - let git_in_src = |args: &[&str]| { + let git_in = |dir: &std::path::Path, args: &[&str]| { let out = Command::new("git") .args(args) - .current_dir(&src) + .current_dir(dir) .output() .expect("spawn git"); assert!( @@ -439,38 +443,79 @@ mod tests { ); out }; - git_in_src(&["-c", "init.defaultBranch=main", "init", "-q"]); - git_in_src(&[ - "-c", - "user.email=t@t", - "-c", - "user.name=t", - "commit", - "-q", - "--allow-empty", - "-m", - "first", - ]); - let sha = String::from_utf8(git_in_src(&["rev-parse", "HEAD"]).stdout) + git_in(&src, &["-c", "init.defaultBranch=main", "init", "-q"]); + git_in( + &src, + &[ + "-c", + "user.email=t@t", + "-c", + "user.name=t", + "commit", + "-q", + "--allow-empty", + "-m", + "main", + ], + ); + // Park the SHA we want to check out on a non-default branch, so the + // test would fail if `clone()` did a shallow / single-branch clone. + git_in(&src, &["checkout", "-q", "-b", "feature"]); + git_in( + &src, + &[ + "-c", + "user.email=t@t", + "-c", + "user.name=t", + "commit", + "-q", + "--allow-empty", + "-m", + "feature", + ], + ); + let sha = String::from_utf8(git_in(&src, &["rev-parse", "HEAD"]).stdout) .unwrap() .trim() .to_string(); assert_eq!(sha.len(), 40); + // Move feature off HEAD so the SHA isn't on the default branch. + git_in(&src, &["checkout", "-q", "main"]); let url = format!("file://{}", src.display()); - Git::new(&dst) + + // gix path — the panic site. Settings::gix defaults to true, but make + // it explicit so the test is robust to future default changes. + let backups = (Settings::get().gix, Settings::get().libgit2); + Settings::override_with(|s| { + s.gix = Some(true); + s.libgit2 = Some(false); + }); + let dst_gix = tmp.path().join("dst-gix"); + Git::new(&dst_gix) .clone(&url, CloneOptions::default().branch(&sha)) - .expect("clone with SHA must not panic and must succeed"); - - let head = Command::new("git") - .args(["rev-parse", "HEAD"]) - .current_dir(&dst) - .output() - .unwrap(); - assert_eq!( - String::from_utf8(head.stdout).unwrap().trim(), - sha, - "worktree should be checked out at the requested SHA" - ); + .expect("gix clone with SHA must not panic and must succeed"); + let head = git_in(&dst_gix, &["rev-parse", "HEAD"]); + assert_eq!(String::from_utf8(head.stdout).unwrap().trim(), sha); + + // CLI path — `git clone -b ` is rejected; verify the SHA + // bypass works there too. + Settings::override_with(|s| { + s.gix = Some(false); + s.libgit2 = Some(false); + }); + let dst_cli = tmp.path().join("dst-cli"); + Git::new(&dst_cli) + .clone(&url, CloneOptions::default().branch(&sha)) + .expect("CLI clone with SHA must succeed"); + let head = git_in(&dst_cli, &["rev-parse", "HEAD"]); + assert_eq!(String::from_utf8(head.stdout).unwrap().trim(), sha); + + // Restore so we don't leak settings into other tests. + Settings::override_with(|s| { + s.gix = Some(backups.0); + s.libgit2 = Some(backups.1); + }); } }