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.

Note that it's explicitly implemented such that obtaining such an empty
refmap is fine, but trying to receive it is not. That way, the user can
obtain information about the server without anything being an error.

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

@@ -47,6 +47,11 @@ pub enum Error {
  NegotiationAlgorithmConfig(#[from] config::key::GenericErrorWithValue),
  #[error("Failed to read remaining bytes in stream")]
  ReadRemainingBytes(#[source] std::io::Error),
+ #[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(", "))]
+ NoMapping {
+ refspecs: Vec<gix_refspec::RefSpec>,
+ num_remote_refs: usize,
+ },
 }
 
 impl gix_protocol::transport::IsSpuriousError for Error {

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

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

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

@@ -91,6 +91,15 @@ where
  let _span = gix_trace::coarse!("fetch::Prepare::receive()");
  let mut con = self.con.take().expect("receive() can only be called once");
 
+ if self.ref_map.mappings.is_empty() && !self.ref_map.remote_refs.is_empty() {
+ let mut specs = con.remote.fetch_specs.clone();
+ specs.extend(self.ref_map.extra_refspecs.clone());
+ return Err(Error::NoMapping {
+ refspecs: specs,
+ num_remote_refs: self.ref_map.remote_refs.len(),
+ });
+ }
+
  let handshake = &self.ref_map.handshake;
  let protocol_version = handshake.server_protocol_version;
 

gix/src/remote/connection/ref_map.rs

@@ -145,6 +145,7 @@ where
  }
  }))
  .validated()?;
+
  let mappings = res.mappings;
  let mappings = mappings
  .into_iter()