Skip to content

Commit 1691c8a

Browse files
committed
fix: don't attempt negotiation without any refspec-mappings (#1405)
Previously, when a shallow operation was specified, it was possible to skip past the no-change test and attempt to negotiate even though there was nothing to want. This is now 'fixed' by simply aborting early if there is no refspec mapping at all. Further, it aborts as early as possible with a nicer error message, after all, there is no use in having no mapping.
1 parent 61fa125 commit 1691c8a

File tree

2 files changed

+16
-1
lines changed

2 files changed

+16
-1
lines changed

gix/src/remote/connection/fetch/negotiate.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ pub(crate) enum Action {
6161
/// Finally, we also mark tips in the `negotiator` in one go to avoid traversing all refs twice, since we naturally encounter all tips during
6262
/// our own walk.
6363
///
64-
/// Return whether or not we should negotiate, along with a queue for later use.
64+
/// Return whether we should negotiate, along with a queue for later use.
6565
pub(crate) fn mark_complete_and_common_ref(
6666
repo: &crate::Repository,
6767
negotiator: &mut dyn gix_negotiate::Negotiator,
@@ -71,6 +71,9 @@ pub(crate) fn mark_complete_and_common_ref(
7171
mapping_is_ignored: impl Fn(&fetch::Mapping) -> bool,
7272
) -> Result<Action, Error> {
7373
let _span = gix_trace::detail!("mark_complete_and_common_ref", mappings = ref_map.mappings.len());
74+
if ref_map.mappings.is_empty() {
75+
return Ok(Action::NoChange);
76+
}
7477
if let fetch::Shallow::Deepen(0) = shallow {
7578
// Avoid deepening (relative) with zero as it seems to upset the server. Git also doesn't actually
7679
// perform the negotiation for some reason (couldn't find it in code).

gix/src/remote/connection/ref_map.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ pub enum Error {
3232
ConfigureCredentials(#[from] crate::config::credential_helpers::Error),
3333
#[error(transparent)]
3434
MappingValidation(#[from] gix_refspec::match_group::validate::Error),
35+
#[error("None of the refspec(s) {} matched any of the {num_remote_refs} refs on the remote", refspecs.iter().map(|r| r.to_ref().instruction().to_bstring().to_string()).collect::<Vec<_>>().join(", "))]
36+
NoMapping {
37+
refspecs: Vec<gix_refspec::RefSpec>,
38+
num_remote_refs: usize,
39+
},
3540
}
3641

3742
impl gix_protocol::transport::IsSpuriousError for Error {
@@ -145,6 +150,13 @@ where
145150
}
146151
}))
147152
.validated()?;
153+
154+
if res.mappings.is_empty() && !remote.refs.is_empty() {
155+
return Err(Error::NoMapping {
156+
refspecs: specs,
157+
num_remote_refs: remote.refs.len(),
158+
});
159+
}
148160
let mappings = res.mappings;
149161
let mappings = mappings
150162
.into_iter()

0 commit comments

Comments
 (0)