fix!: assure that empty fetches due to no objects wanted are terminated early

Author: Byron

gix-protocol/src/fetch/error.rs

@@ -21,8 +21,8 @@ pub enum Error {
     LockShallowFile(#[from] gix_lock::acquire::Error),
     #[error("Receiving objects from shallow remotes is prohibited due to the value of `clone.rejectShallow`")]
     RejectShallowRemote,
-    #[error("Failed to consume the pack sent by the remove")]
-    ConsumePack(Box<dyn std::error::Error + Send + Sync + 'static>),
+    #[error("Failed to consume the pack sent by the remote")]
+    ConsumePack(#[source] Box<dyn std::error::Error + Send + Sync + 'static>),
     #[error("Failed to read remaining bytes in stream")]
     ReadRemainingBytes(#[source] std::io::Error),
 }

gix-protocol/src/fetch/function.rs

@@ -22,12 +22,12 @@ use std::sync::atomic::{AtomicBool, Ordering};
 /// * …update local refs
 /// * …end the interaction after the fetch
 ///
-/// Note that the interaction will never be ended, even on error or failure, leaving it up to the caller to do that, maybe
+/// **Note that the interaction will never be ended**, even on error or failure, leaving it up to the caller to do that, maybe
 /// with the help of [`SendFlushOnDrop`](crate::SendFlushOnDrop) which can wrap `transport`.
 /// Generally, the `transport` is left in a state that allows for more commands to be run.
 ///
-/// Return `Ok(None)` if there was nothing to do because all remote refs are at the same state as they are locally, or `Ok(Some(outcome))`
-/// to inform about all the changes that were made.
+/// Return `Ok(None)` if there was nothing to do because all remote refs are at the same state as they are locally,
+/// or there was nothing wanted, or `Ok(Some(outcome))` to inform about all the changes that were made.
 #[maybe_async::maybe_async]
 pub async fn fetch<P, T, E>(
     negotiate: &mut impl Negotiate,
@@ -91,7 +91,9 @@ where
         negotiate::Action::MustNegotiate {
             remote_ref_target_known,
         } => {
-            negotiate.add_wants(&mut arguments, remote_ref_target_known);
+            if !negotiate.add_wants(&mut arguments, remote_ref_target_known) {
+                return Ok(None);
+            }
             let mut rounds = Vec::new();
             let is_stateless = arguments.is_stateless(!transport.connection_persists_across_multiple_requests());
             let mut state = negotiate::one_round::State::new(is_stateless);

gix-protocol/src/fetch/negotiate.rs

@@ -284,17 +284,21 @@ pub fn make_refmapping_ignore_predicate(fetch_tags: Tags, ref_map: &RefMap) -> i
 /// * `ref_map` is the state of refs as known on the remote.
 /// * `shallow` defines if the history should be shallow.
 /// * `mapping_is_ignored` is typically initialized with [`make_refmapping_ignore_predicate`].
+///
+/// Returns `true` if at least one [want](crate::fetch::Arguments::want()) was added, or `false` otherwise.
+/// Note that not adding a single want can make the remote hang, so it's avoided on the client side by ending the fetch operation.
 pub fn add_wants(
     objects: &impl gix_object::FindHeader,
     arguments: &mut crate::fetch::Arguments,
     ref_map: &RefMap,
     remote_ref_target_known: &[bool],
     shallow: &Shallow,
     mapping_is_ignored: impl Fn(&refmap::Mapping) -> bool,
-) {
+) -> bool {
     // When using shallow, we can't exclude `wants` as the remote won't send anything then. Thus, we have to resend everything
     // we have as want instead to get exactly the same graph, but possibly deepened.
     let is_shallow = !matches!(shallow, Shallow::NoChange);
+    let mut has_want = false;
     let wants = ref_map
         .mappings
         .iter()
@@ -306,13 +310,15 @@ pub fn add_wants(
         if !arguments.can_use_ref_in_want() || matches!(want.remote, refmap::Source::ObjectId(_)) {
             if let Some(id) = id_on_remote {
                 arguments.want(id);
+                has_want = true;
             }
         } else {
             arguments.want_ref(
                 want.remote
                     .as_name()
                     .expect("name available if this isn't an object id"),
             );
+            has_want = true;
         }
         let id_is_annotated_tag_we_have = id_on_remote
             .and_then(|id| objects.try_header(id).ok().flatten().map(|h| (id, h)))
@@ -324,6 +330,7 @@ pub fn add_wants(
             arguments.have(tag_on_remote);
         }
     }
+    has_want
 }
 
 /// Remove all commits that are more recent than the cut-off, which is the commit time of the oldest common commit we have with the server.

gix-protocol/src/fetch/types.rs

@@ -70,7 +70,9 @@ mod with_fetch {
         /// Typically invokes [`negotiate::mark_complete_and_common_ref()`].
         fn mark_complete_and_common_ref(&mut self) -> Result<negotiate::Action, negotiate::Error>;
         /// Typically invokes [`negotiate::add_wants()`].
-        fn add_wants(&mut self, arguments: &mut fetch::Arguments, remote_ref_target_known: &[bool]);
+        /// Returns `true` if wants were added, or `false` if the negotiation should be aborted.
+        #[must_use]
+        fn add_wants(&mut self, arguments: &mut fetch::Arguments, remote_ref_target_known: &[bool]) -> bool;
         /// Typically invokes [`negotiate::one_round()`].
         fn one_round(
             &mut self,