fix: shallow clone with tag ref_name now uses correct refspec#2553
fix: shallow clone with tag ref_name now uses correct refspec#2553sapatrjv wants to merge 1 commit into
Conversation
sapatrjv
force-pushed
the
fix-shallow-clone-tag-refspec
branch
from
f85cbbe to
653d073
Compare
When performing a shallow clone with a tag-based ref_name (e.g., \�1.30.0\), the code unconditionally assumed the ref was a branch, generating \ efs/heads/v1.30.0\ even when the remote only had \ efs/tags/v1.30.0\. This caused the refspec to match nothing, failing with a 'None of the refspec(s) matched' error. Now the shallow clone path connects to the remote to discover what the ref actually is before constructing the refspec. For tags, the refspec maps as \+refs/tags/X:refs/tags/X\ (matching git's behavior). For branches, the existing \+refs/heads/X:refs/remotes/origin/X\ mapping is preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sapatrjv
force-pushed
the
fix-shallow-clone-tag-refspec
branch
from
653d073 to
57cf3ed
Compare
Sebastian Thiel (Byron)
requested review from
Copilot
and removed request for
Copilot
|
Codex (@codex) review |
Sebastian Thiel (Byron)
left a comment
Sebastian Thiel (Byron)
left a comment
There was a problem hiding this comment.
Needs a test.
Added followng test case: shallow_clone_with_tag_ref_name_uses_tag_refspec |
There was a problem hiding this comment.
Pull request overview
Fixes shallow-clone behavior in gix when the user provides a tag name via with_ref_name(), ensuring the generated persisted fetch refspec matches the remote’s actual ref type (tag vs branch), addressing #2554.
Changes:
- For shallow clones, connect to the remote to resolve
ref_name(orHEAD) to a concrete remote full ref name before constructing the single-ref refspec. - Persist a tag-style refspec (
+refs/tags/X:refs/tags/X) when the resolved target is a tag; otherwise keep branch-style mapping torefs/remotes/<remote>/.... - Add a regression test asserting a tag-based
ref_nameresults in a tag-style persisted fetch refspec.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| gix/src/clone/fetch/mod.rs | Resolve shallow-clone target ref via remote ref discovery, then build tag-vs-branch appropriate single-ref refspec. |
| gix/tests/gix/clone.rs | Add regression test for shallow clone with tag ref_name producing a tag-style persisted refspec. |
| let extra_refspec = if let Some(ref_name) = &self.ref_name { | ||
| // Use the user-specified ref_name as the refspec to discover what it resolves to. | ||
| gix_refspec::parse(ref_name.as_ref().as_bstr(), gix_refspec::parse::Operation::Fetch) | ||
| .expect("partial names are valid refspecs") | ||
| .to_owned() | ||
| } else { | ||
| // For shallow clones without a specified ref, we need to determine the ref to clone. | ||
| // Just fetch HEAD for that. | ||
| let prev_tags = std::mem::replace(&mut remote.fetch_tags, remote::fetch::Tags::None); | ||
| let mut connection = remote.connect(remote::Direction::Fetch).await?; | ||
| if let Some(f) = self.configure_connection.as_mut() { | ||
| f(&mut connection).map_err(Error::RemoteConnection)?; | ||
| } | ||
| let refmap = connection | ||
| .ref_map_by_ref( | ||
| &mut progress, | ||
| remote::ref_map::Options { | ||
| extra_refspecs: vec![gix_refspec::parse( | ||
| "HEAD".into(), | ||
| gix_refspec::parse::Operation::Fetch, | ||
| ) | ||
| .expect("valid") | ||
| .to_owned()], | ||
| ..Default::default() | ||
| }, | ||
| ) | ||
| .await?; | ||
| gix_refspec::parse("HEAD".into(), gix_refspec::parse::Operation::Fetch) | ||
| .expect("valid") | ||
| .to_owned() |
There was a problem hiding this comment.
Avoid expect() when parsing the user-provided ref_name into a refspec. Even though with_ref_name() validates PartialName, a parse failure here would still panic inside fetch_only() (production code). Prefer propagating the parse error (e.g., add an Error::RefSpecParse variant and use map_err(...) + ?) so invalid inputs never crash the process.
|
Closing in favor of #2556. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57cf3ededd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| Some(Category::Tag) => { | ||
| format!("+{target_ref}:{target_ref}") | ||
| } |
There was a problem hiding this comment.
When target_ref is a tag, this new tag-style refspec now allows shallow --ref <tag> clones to proceed, but the flow still appends the implicit HEAD:refs/remotes/<remote>/HEAD extra refspec later, so the fetch pulls the default-branch tip in addition to the tag. In practice, a --depth 1 --ref <tag> clone can end up with multiple commits/shallow boundaries (unlike git clone, which keeps only the tagged commit), increasing transfer size and breaking expected shallow semantics for tag-based clones.
Useful? React with 👍 / 👎.
Summary
Fixes a bug where shallow clones with a tag-based fail with:
\
None of the refspec(s) +refs/heads/v1.30.0:refs/remotes/origin/v1.30.0, ... matched any of the 23 refs on the remote
\\
Fix for issue: #2554
Root Cause
The shallow clone path unconditionally assumed ref_name was a branch via Category::LocalBranch.to_full_name(), generating refs/heads/* even when the remote only had refs/tags/*.
Fix
The shallow clone path now always connects to the remote to discover the actual ref type before constructing the refspec:
Testing
Related Issues
gix clone --depth 1is larger thangit clone --depth 1#2227) to handle tag refs correctly