improve fetchspec handling to be closer to what git does.

The current implementation was a bit hacky to mimick observed
behaviour. These improvements make it easier to understand
and bring it closer to what git actually seems to do.

Author: Byron

Diff for: gix/src/remote/connection/fetch/negotiate.rs

@@ -1,5 +1,4 @@
 use crate::remote::fetch;
-use gix_refspec::RefSpec;
 
 /// The way the negotiation is performed
 #[derive(Copy, Clone)]
@@ -28,19 +27,15 @@ pub(crate) fn one_round(
     fetch_tags: crate::remote::fetch::Tags,
     arguments: &mut gix_protocol::fetch::Arguments,
     _previous_response: Option<&gix_protocol::fetch::Response>,
-    wants_shallow_change: Option<(&fetch::Shallow, &[RefSpec])>,
+    shallow: Option<&fetch::Shallow>,
 ) -> Result<bool, Error> {
     let tag_refspec_to_ignore = fetch_tags
         .to_refspec()
         .filter(|_| matches!(fetch_tags, crate::remote::fetch::Tags::Included));
-    let (shallow, non_wildcard_specs_only) = wants_shallow_change
-        .map(|(a, b)| (Some(a), Some(b)))
-        .unwrap_or_default();
-
-    if let Some(shallow) = shallow {
-        if *shallow == fetch::Shallow::Deepen(0) {
-            return Ok(true);
-        }
+    if let Some(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).
+        return Ok(true);
     }
 
     match algo {
@@ -57,14 +52,6 @@ pub(crate) fn one_round(
                 }) {
                     continue;
                 }
-                if non_wildcard_specs_only
-                    .and_then(|refspecs| mapping.spec_index.get(refspecs, &ref_map.extra_refspecs))
-                    .map_or(false, |spec| {
-                        spec.to_ref().local().map_or(false, |ref_| ref_.contains(&b'*'))
-                    })
-                {
-                    continue;
-                }
                 let have_id = mapping.local.as_ref().and_then(|name| {
                     repo.find_reference(name)
                         .ok()
@@ -73,7 +60,7 @@ pub(crate) fn one_round(
                 match have_id {
                     Some(have_id) => {
                         if let Some(want_id) = mapping.remote.as_id() {
-                            if want_id != have_id || wants_shallow_change.is_some() {
+                            if want_id != have_id {
                                 arguments.want(want_id);
                                 arguments.have(have_id);
                             }
@@ -88,7 +75,7 @@ pub(crate) fn one_round(
                 }
             }
 
-            if has_missing_tracking_branch || (wants_shallow_change.is_some() && arguments.is_empty()) {
+            if has_missing_tracking_branch || (shallow.is_some() && arguments.is_empty()) {
                 if let Ok(Some(r)) = repo.head_ref() {
                     if let Some(id) = r.target().try_id() {
                         arguments.have(id);

Diff for: gix/src/remote/connection/fetch/receive_pack.rs

@@ -118,8 +118,7 @@ where
                 con.remote.fetch_tags,
                 &mut arguments,
                 previous_response.as_ref(),
-                (self.shallow != Shallow::NoChange)
-                    .then(|| (&self.shallow, con.remote.refspecs(remote::Direction::Fetch))),
+                (self.shallow != Shallow::NoChange).then_some(&self.shallow),
             ) {
                 Ok(_) if arguments.is_empty() => {
                     gix_protocol::indicate_end_of_interaction(&mut con.transport).await.ok();

Diff for: gix/tests/clone/mod.rs

@@ -2,6 +2,7 @@ use crate::{remote, util::restricted};
 
 #[cfg(feature = "blocking-network-client")]
 mod blocking_io {
+    use gix::bstr::BString;
     use gix::config::tree::{Clone, Core, Init, Key};
     use gix::remote::Direction;
     use gix_object::bstr::ByteSlice;
@@ -25,9 +26,14 @@ mod blocking_io {
                 let called_configure_remote = called_configure_remote;
                 move |r| {
                     called_configure_remote.store(true, std::sync::atomic::Ordering::Relaxed);
-                    let r = r
-                        .with_refspecs(Some("+refs/tags/b-tag:refs/tags/b-tag"), gix::remote::Direction::Fetch)?
-                        .with_fetch_tags(desired_fetch_tags);
+                    let mut r = r.with_fetch_tags(desired_fetch_tags);
+                    r.replace_refspecs(
+                        [
+                            BString::from(format!("refs/heads/main:refs/remotes/{remote_name}/main")),
+                            "+refs/tags/b-tag:refs/tags/b-tag".to_owned().into(),
+                        ],
+                        Direction::Fetch,
+                    )?;
                     Ok(r)
                 }
             })
@@ -111,6 +117,10 @@ mod blocking_io {
         let tmp = gix_testtools::tempfile::TempDir::new()?;
         let (repo, _change) = gix::prepare_clone_bare(remote::repo("base").path(), tmp.path())?
             .with_shallow(Shallow::DepthAtRemote(2.try_into()?))
+            .configure_remote(|mut r| {
+                r.replace_refspecs(Some("refs/heads/main:refs/remotes/origin/main"), Direction::Fetch)?;
+                Ok(r)
+            })
             .fetch_only(gix::progress::Discard, &std::sync::atomic::AtomicBool::default())?;
 
         assert!(repo.is_shallow());