From 7e08219644fb84550b82e4c36cdec2d08a06136d Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 23 Nov 2025 16:33:30 +0100 Subject: [PATCH 01/12] tests: drop unnecessary argument from DelegateBlocking::prepare_ls_refs() --- gix-protocol/tests/protocol/fetch/_impl.rs | 20 +++++--------------- gix-protocol/tests/protocol/fetch/mod.rs | 2 -- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/gix-protocol/tests/protocol/fetch/_impl.rs b/gix-protocol/tests/protocol/fetch/_impl.rs index c4484fda920..68fb90ab813 100644 --- a/gix-protocol/tests/protocol/fetch/_impl.rs +++ b/gix-protocol/tests/protocol/fetch/_impl.rs @@ -95,7 +95,7 @@ mod fetch_fn { gix_protocol::ls_refs( &mut transport, &capabilities, - |a, b| delegate.prepare_ls_refs(a, b), + |a, _b| delegate.prepare_ls_refs(a), &mut progress, trace, ("agent", Some(Cow::Owned(agent.clone()))), @@ -195,7 +195,6 @@ mod delegate { ops::{Deref, DerefMut}, }; - use bstr::BString; use gix_protocol::{ fetch::{Arguments, Response}, handshake::Ref, @@ -234,11 +233,7 @@ mod delegate { /// If the delegate returns [`ls_refs::Action::Skip`], no `ls-refs` command is sent to the server. /// /// Note that this is called only if we are using protocol version 2. - fn prepare_ls_refs( - &mut self, - _server: &Capabilities, - _arguments: &mut Vec, - ) -> std::io::Result { + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> std::io::Result { Ok(ls_refs::Action::Continue) } @@ -303,12 +298,8 @@ mod delegate { self.deref().handshake_extra_parameters() } - fn prepare_ls_refs( - &mut self, - _server: &Capabilities, - _arguments: &mut Vec, - ) -> io::Result { - self.deref_mut().prepare_ls_refs(_server, _arguments) + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { + self.deref_mut().prepare_ls_refs(_server) } fn prepare_fetch( @@ -339,9 +330,8 @@ mod delegate { fn prepare_ls_refs( &mut self, _server: &Capabilities, - _arguments: &mut Vec, ) -> io::Result { - self.deref_mut().prepare_ls_refs(_server, _arguments) + self.deref_mut().prepare_ls_refs(_server) } fn prepare_fetch( diff --git a/gix-protocol/tests/protocol/fetch/mod.rs b/gix-protocol/tests/protocol/fetch/mod.rs index 6725959a58f..d7cae03a9fc 100644 --- a/gix-protocol/tests/protocol/fetch/mod.rs +++ b/gix-protocol/tests/protocol/fetch/mod.rs @@ -105,7 +105,6 @@ impl DelegateBlocking for CloneRefInWantDelegate { fn prepare_ls_refs( &mut self, _server: &Capabilities, - _arguments: &mut Vec, ) -> io::Result { Ok(ls_refs::Action::Skip) } @@ -148,7 +147,6 @@ impl DelegateBlocking for LsRemoteDelegate { fn prepare_ls_refs( &mut self, _server: &Capabilities, - _arguments: &mut Vec, ) -> std::io::Result { match self.abort_with.take() { Some(err) => Err(err), From 149bcfca6a35ebfd7493ee8517905a6b6ca58dd5 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 23 Nov 2025 16:37:57 +0100 Subject: [PATCH 02/12] refactor!: take extra arguments as a separate argument --- gix-protocol/src/fetch/refmap/init.rs | 14 +++++++------- gix-protocol/src/ls_refs.rs | 10 +++++++--- gix-protocol/tests/protocol/fetch/_impl.rs | 8 +++----- gix-protocol/tests/protocol/fetch/mod.rs | 10 ++-------- 4 files changed, 19 insertions(+), 23 deletions(-) diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index abcf027cfb1..1f4f87a8a31 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -80,10 +80,8 @@ impl RefMap { let remote_refs = crate::ls_refs( transport, capabilities, - |_capabilities, arguments| { - push_prefix_arguments(prefix_from_spec_as_filter_on_remote, arguments, &all_refspecs); - Ok(crate::ls_refs::Action::Continue) - }, + |_capabilities| Ok(crate::ls_refs::Action::Continue), + push_prefix_arguments(prefix_from_spec_as_filter_on_remote, &all_refspecs), &mut progress, trace_packetlines, user_agent, @@ -164,13 +162,13 @@ impl RefMap { fn push_prefix_arguments( prefix_from_spec_as_filter_on_remote: bool, - arguments: &mut Vec, all_refspecs: &[gix_refspec::RefSpec], -) { +) -> Vec { if !prefix_from_spec_as_filter_on_remote { - return; + return Vec::new(); } + let mut arguments = Vec::new(); let mut seen = HashSet::new(); for spec in all_refspecs { let spec = spec.to_ref(); @@ -183,4 +181,6 @@ fn push_prefix_arguments( } } } + + arguments } diff --git a/gix-protocol/src/ls_refs.rs b/gix-protocol/src/ls_refs.rs index d9fd8c95c5d..a0ddcc61eaf 100644 --- a/gix-protocol/src/ls_refs.rs +++ b/gix-protocol/src/ls_refs.rs @@ -61,7 +61,8 @@ pub(crate) mod function { }; /// Invoke a ls-refs V2 command on `transport`, which requires a prior handshake that yielded - /// server `capabilities`. `prepare_ls_refs(capabilities, arguments)` can be used to alter the _ls-refs_. + /// server `capabilities`. `prepare_ls_refs(capabilities)` can be used to alter the _ls-refs_. + /// `arguments` are extra arguments to send to the server. /// `progress` is used to provide feedback. /// The `agent` information will be added to the features sent to the server. /// If `trace` is `true`, all packetlines received or sent will be passed to the facilities of the `gix-trace` crate. @@ -69,7 +70,8 @@ pub(crate) mod function { pub async fn ls_refs( mut transport: impl Transport, capabilities: &Capabilities, - prepare_ls_refs: impl FnOnce(&Capabilities, &mut Vec) -> std::io::Result, + prepare_ls_refs: impl FnOnce(&Capabilities) -> std::io::Result, + extra_args: Vec, progress: &mut impl Progress, trace: bool, agent: (&'static str, Option>), @@ -86,7 +88,9 @@ pub(crate) mod function { { ls_args.push("unborn".into()); } - let refs = match prepare_ls_refs(capabilities, &mut ls_args) { + + ls_args.extend(extra_args); + let refs = match prepare_ls_refs(capabilities) { Ok(Action::Skip) => Vec::new(), Ok(Action::Continue) => { ls_refs.validate_argument_prefixes( diff --git a/gix-protocol/tests/protocol/fetch/_impl.rs b/gix-protocol/tests/protocol/fetch/_impl.rs index 68fb90ab813..3429f37b526 100644 --- a/gix-protocol/tests/protocol/fetch/_impl.rs +++ b/gix-protocol/tests/protocol/fetch/_impl.rs @@ -95,7 +95,8 @@ mod fetch_fn { gix_protocol::ls_refs( &mut transport, &capabilities, - |a, _b| delegate.prepare_ls_refs(a), + |a| delegate.prepare_ls_refs(a), + Vec::new(), &mut progress, trace, ("agent", Some(Cow::Owned(agent.clone()))), @@ -327,10 +328,7 @@ mod delegate { self.deref().handshake_extra_parameters() } - fn prepare_ls_refs( - &mut self, - _server: &Capabilities, - ) -> io::Result { + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { self.deref_mut().prepare_ls_refs(_server) } diff --git a/gix-protocol/tests/protocol/fetch/mod.rs b/gix-protocol/tests/protocol/fetch/mod.rs index d7cae03a9fc..a49cd9211ca 100644 --- a/gix-protocol/tests/protocol/fetch/mod.rs +++ b/gix-protocol/tests/protocol/fetch/mod.rs @@ -102,10 +102,7 @@ pub struct CloneRefInWantDelegate { } impl DelegateBlocking for CloneRefInWantDelegate { - fn prepare_ls_refs( - &mut self, - _server: &Capabilities, - ) -> io::Result { + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { Ok(ls_refs::Action::Skip) } @@ -144,10 +141,7 @@ impl DelegateBlocking for LsRemoteDelegate { fn handshake_extra_parameters(&self) -> Vec<(String, Option)> { vec![("value-only".into(), None), ("key".into(), Some("value".into()))] } - fn prepare_ls_refs( - &mut self, - _server: &Capabilities, - ) -> std::io::Result { + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> std::io::Result { match self.abort_with.take() { Some(err) => Err(err), None => Ok(ls_refs::Action::Continue), From c6d76885bdddc2d6a230618b8ba3f4ebae715987 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 1 Dec 2025 10:42:18 +0100 Subject: [PATCH 03/12] refactor!: rename ls_refs::Action to RefsAction --- gix-protocol/src/fetch/refmap/init.rs | 2 +- gix-protocol/src/ls_refs.rs | 10 +++++----- gix-protocol/tests/protocol/fetch/_impl.rs | 8 ++++---- gix-protocol/tests/protocol/fetch/mod.rs | 8 ++++---- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index 1f4f87a8a31..b3c920b832f 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -80,7 +80,7 @@ impl RefMap { let remote_refs = crate::ls_refs( transport, capabilities, - |_capabilities| Ok(crate::ls_refs::Action::Continue), + |_capabilities| Ok(crate::ls_refs::RefsAction::Continue), push_prefix_arguments(prefix_from_spec_as_filter_on_remote, &all_refspecs), &mut progress, trace_packetlines, diff --git a/gix-protocol/src/ls_refs.rs b/gix-protocol/src/ls_refs.rs index a0ddcc61eaf..68dc3068a5b 100644 --- a/gix-protocol/src/ls_refs.rs +++ b/gix-protocol/src/ls_refs.rs @@ -31,7 +31,7 @@ pub use error::Error; /// What to do after preparing ls-refs in [`ls_refs()`][crate::ls_refs()]. #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] -pub enum Action { +pub enum RefsAction { /// Continue by sending a 'ls-refs' command. Continue, /// Skip 'ls-refs' entirely. @@ -50,7 +50,7 @@ pub(crate) mod function { use gix_transport::client::Capabilities; use maybe_async::maybe_async; - use super::{Action, Error}; + use super::{Error, RefsAction}; #[cfg(feature = "async-client")] use crate::transport::client::async_io::{Transport, TransportV2Ext}; #[cfg(feature = "blocking-client")] @@ -70,7 +70,7 @@ pub(crate) mod function { pub async fn ls_refs( mut transport: impl Transport, capabilities: &Capabilities, - prepare_ls_refs: impl FnOnce(&Capabilities) -> std::io::Result, + prepare_ls_refs: impl FnOnce(&Capabilities) -> std::io::Result, extra_args: Vec, progress: &mut impl Progress, trace: bool, @@ -91,8 +91,8 @@ pub(crate) mod function { ls_args.extend(extra_args); let refs = match prepare_ls_refs(capabilities) { - Ok(Action::Skip) => Vec::new(), - Ok(Action::Continue) => { + Ok(RefsAction::Skip) => Vec::new(), + Ok(RefsAction::Continue) => { ls_refs.validate_argument_prefixes( gix_transport::Protocol::V2, capabilities, diff --git a/gix-protocol/tests/protocol/fetch/_impl.rs b/gix-protocol/tests/protocol/fetch/_impl.rs index 3429f37b526..438832decf1 100644 --- a/gix-protocol/tests/protocol/fetch/_impl.rs +++ b/gix-protocol/tests/protocol/fetch/_impl.rs @@ -234,8 +234,8 @@ mod delegate { /// If the delegate returns [`ls_refs::Action::Skip`], no `ls-refs` command is sent to the server. /// /// Note that this is called only if we are using protocol version 2. - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> std::io::Result { - Ok(ls_refs::Action::Continue) + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> std::io::Result { + Ok(ls_refs::RefsAction::Continue) } /// Called before invoking the 'fetch' interaction with `features` pre-filled for typical use @@ -299,7 +299,7 @@ mod delegate { self.deref().handshake_extra_parameters() } - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { self.deref_mut().prepare_ls_refs(_server) } @@ -328,7 +328,7 @@ mod delegate { self.deref().handshake_extra_parameters() } - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { self.deref_mut().prepare_ls_refs(_server) } diff --git a/gix-protocol/tests/protocol/fetch/mod.rs b/gix-protocol/tests/protocol/fetch/mod.rs index a49cd9211ca..31d88880637 100644 --- a/gix-protocol/tests/protocol/fetch/mod.rs +++ b/gix-protocol/tests/protocol/fetch/mod.rs @@ -102,8 +102,8 @@ pub struct CloneRefInWantDelegate { } impl DelegateBlocking for CloneRefInWantDelegate { - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { - Ok(ls_refs::Action::Skip) + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { + Ok(ls_refs::RefsAction::Skip) } fn prepare_fetch( @@ -141,10 +141,10 @@ impl DelegateBlocking for LsRemoteDelegate { fn handshake_extra_parameters(&self) -> Vec<(String, Option)> { vec![("value-only".into(), None), ("key".into(), Some("value".into()))] } - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> std::io::Result { + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> std::io::Result { match self.abort_with.take() { Some(err) => Err(err), - None => Ok(ls_refs::Action::Continue), + None => Ok(ls_refs::RefsAction::Continue), } } fn prepare_fetch( From e5498415a704859fe8385b09e46539372df04d9f Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 1 Dec 2025 10:43:34 +0100 Subject: [PATCH 04/12] tests: import RefsAction directly --- gix-protocol/tests/protocol/fetch/_impl.rs | 10 +++++----- gix-protocol/tests/protocol/fetch/mod.rs | 11 ++++++----- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/gix-protocol/tests/protocol/fetch/_impl.rs b/gix-protocol/tests/protocol/fetch/_impl.rs index 438832decf1..fdbbd70dbdc 100644 --- a/gix-protocol/tests/protocol/fetch/_impl.rs +++ b/gix-protocol/tests/protocol/fetch/_impl.rs @@ -199,7 +199,7 @@ mod delegate { use gix_protocol::{ fetch::{Arguments, Response}, handshake::Ref, - ls_refs, + ls_refs::{self, RefsAction}, }; use gix_transport::client::Capabilities; @@ -234,8 +234,8 @@ mod delegate { /// If the delegate returns [`ls_refs::Action::Skip`], no `ls-refs` command is sent to the server. /// /// Note that this is called only if we are using protocol version 2. - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> std::io::Result { - Ok(ls_refs::RefsAction::Continue) + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> std::io::Result { + Ok(RefsAction::Continue) } /// Called before invoking the 'fetch' interaction with `features` pre-filled for typical use @@ -299,7 +299,7 @@ mod delegate { self.deref().handshake_extra_parameters() } - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { self.deref_mut().prepare_ls_refs(_server) } @@ -328,7 +328,7 @@ mod delegate { self.deref().handshake_extra_parameters() } - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { self.deref_mut().prepare_ls_refs(_server) } diff --git a/gix-protocol/tests/protocol/fetch/mod.rs b/gix-protocol/tests/protocol/fetch/mod.rs index 31d88880637..c54295cd028 100644 --- a/gix-protocol/tests/protocol/fetch/mod.rs +++ b/gix-protocol/tests/protocol/fetch/mod.rs @@ -3,7 +3,8 @@ use std::{borrow::Cow, io}; use bstr::{BString, ByteSlice}; use gix_protocol::{ fetch::{Arguments, Response}, - handshake, ls_refs, + handshake, + ls_refs::{self, RefsAction}, }; use gix_transport::client::Capabilities; @@ -102,8 +103,8 @@ pub struct CloneRefInWantDelegate { } impl DelegateBlocking for CloneRefInWantDelegate { - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { - Ok(ls_refs::RefsAction::Skip) + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { + Ok(RefsAction::Skip) } fn prepare_fetch( @@ -141,10 +142,10 @@ impl DelegateBlocking for LsRemoteDelegate { fn handshake_extra_parameters(&self) -> Vec<(String, Option)> { vec![("value-only".into(), None), ("key".into(), Some("value".into()))] } - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> std::io::Result { + fn prepare_ls_refs(&mut self, _server: &Capabilities) -> std::io::Result { match self.abort_with.take() { Some(err) => Err(err), - None => Ok(ls_refs::RefsAction::Continue), + None => Ok(RefsAction::Continue), } } fn prepare_fetch( From 3bc580173ca8f6e95f0e8996a05a451dc199dfa3 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 23 Nov 2025 16:48:14 +0100 Subject: [PATCH 05/12] refactor!: stop taking a callback for test-only uses --- gix-protocol/src/fetch/refmap/init.rs | 1 - gix-protocol/src/ls_refs.rs | 64 ++++++---------------- gix-protocol/tests/protocol/fetch/_impl.rs | 59 +++++++++++++------- gix-protocol/tests/protocol/fetch/mod.rs | 7 +-- gix-protocol/tests/protocol/fetch/v2.rs | 4 +- 5 files changed, 62 insertions(+), 73 deletions(-) diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index b3c920b832f..01889432e12 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -80,7 +80,6 @@ impl RefMap { let remote_refs = crate::ls_refs( transport, capabilities, - |_capabilities| Ok(crate::ls_refs::RefsAction::Continue), push_prefix_arguments(prefix_from_spec_as_filter_on_remote, &all_refspecs), &mut progress, trace_packetlines, diff --git a/gix-protocol/src/ls_refs.rs b/gix-protocol/src/ls_refs.rs index 68dc3068a5b..79a16545178 100644 --- a/gix-protocol/src/ls_refs.rs +++ b/gix-protocol/src/ls_refs.rs @@ -29,18 +29,6 @@ mod error { #[cfg(any(feature = "blocking-client", feature = "async-client"))] pub use error::Error; -/// What to do after preparing ls-refs in [`ls_refs()`][crate::ls_refs()]. -#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] -pub enum RefsAction { - /// Continue by sending a 'ls-refs' command. - Continue, - /// Skip 'ls-refs' entirely. - /// - /// This is useful if the `ref-in-want` capability is taken advantage of. When fetching, one must must then send - /// `want-ref`s during the negotiation phase. - Skip, -} - #[cfg(any(feature = "blocking-client", feature = "async-client"))] pub(crate) mod function { use std::borrow::Cow; @@ -50,14 +38,14 @@ pub(crate) mod function { use gix_transport::client::Capabilities; use maybe_async::maybe_async; - use super::{Error, RefsAction}; + use super::Error; #[cfg(feature = "async-client")] use crate::transport::client::async_io::{Transport, TransportV2Ext}; #[cfg(feature = "blocking-client")] use crate::transport::client::blocking_io::{Transport, TransportV2Ext}; use crate::{ handshake::{refs::from_v2_refs, Ref}, - indicate_end_of_interaction, Command, + Command, }; /// Invoke a ls-refs V2 command on `transport`, which requires a prior handshake that yielded @@ -70,7 +58,6 @@ pub(crate) mod function { pub async fn ls_refs( mut transport: impl Transport, capabilities: &Capabilities, - prepare_ls_refs: impl FnOnce(&Capabilities) -> std::io::Result, extra_args: Vec, progress: &mut impl Progress, trace: bool, @@ -90,37 +77,22 @@ pub(crate) mod function { } ls_args.extend(extra_args); - let refs = match prepare_ls_refs(capabilities) { - Ok(RefsAction::Skip) => Vec::new(), - Ok(RefsAction::Continue) => { - ls_refs.validate_argument_prefixes( - gix_transport::Protocol::V2, - capabilities, - &ls_args, - &ls_features, - )?; + ls_refs.validate_argument_prefixes(gix_transport::Protocol::V2, capabilities, &ls_args, &ls_features)?; - progress.step(); - progress.set_name("list refs".into()); - let mut remote_refs = transport - .invoke( - ls_refs.as_str(), - ls_features.into_iter(), - if ls_args.is_empty() { - None - } else { - Some(ls_args.into_iter()) - }, - trace, - ) - .await?; - from_v2_refs(&mut remote_refs).await? - } - Err(err) => { - indicate_end_of_interaction(transport, trace).await?; - return Err(err.into()); - } - }; - Ok(refs) + progress.step(); + progress.set_name("list refs".into()); + let mut remote_refs = transport + .invoke( + ls_refs.as_str(), + ls_features.into_iter(), + if ls_args.is_empty() { + None + } else { + Some(ls_args.into_iter()) + }, + trace, + ) + .await?; + Ok(from_v2_refs(&mut remote_refs).await?) } } diff --git a/gix-protocol/tests/protocol/fetch/_impl.rs b/gix-protocol/tests/protocol/fetch/_impl.rs index fdbbd70dbdc..68a22f1cc45 100644 --- a/gix-protocol/tests/protocol/fetch/_impl.rs +++ b/gix-protocol/tests/protocol/fetch/_impl.rs @@ -1,3 +1,15 @@ +/// What to do after preparing ls-refs. +#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] +pub enum RefsAction { + /// Continue by sending a 'ls-refs' command. + Continue, + /// Skip 'ls-refs' entirely. + /// + /// This is useful if the `ref-in-want` capability is taken advantage of. When fetching, one + /// must then send `want-ref`s during the negotiation phase. + Skip, +} + mod fetch_fn { use std::borrow::Cow; @@ -13,7 +25,7 @@ mod fetch_fn { use gix_transport::client::blocking_io::{ExtendedBufRead, HandleProgress, Transport}; use maybe_async::maybe_async; - use super::{Action, Delegate}; + use super::{Action, Delegate, RefsAction}; use crate::fetch::Error; /// A way to indicate how to treat the connection underlying the transport, potentially allowing to reuse it. @@ -91,18 +103,24 @@ mod fetch_fn { let agent = gix_protocol::agent(agent); let refs = match refs { Some(refs) => refs, - None => { - gix_protocol::ls_refs( - &mut transport, - &capabilities, - |a| delegate.prepare_ls_refs(a), - Vec::new(), - &mut progress, - trace, - ("agent", Some(Cow::Owned(agent.clone()))), - ) - .await? - } + None => match delegate.action() { + Ok(RefsAction::Skip) => Vec::new(), + Ok(RefsAction::Continue) => { + gix_protocol::ls_refs( + &mut transport, + &capabilities, + Vec::new(), + &mut progress, + trace, + ("agent", Some(Cow::Owned(agent.clone()))), + ) + .await? + } + Err(err) => { + indicate_end_of_interaction(transport, trace).await?; + return Err(err.into()); + } + }, }; let fetch = Command::Fetch; @@ -199,10 +217,11 @@ mod delegate { use gix_protocol::{ fetch::{Arguments, Response}, handshake::Ref, - ls_refs::{self, RefsAction}, }; use gix_transport::client::Capabilities; + use super::RefsAction; + /// Defines what to do next after certain [`Delegate`] operations. #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] pub enum Action { @@ -231,10 +250,10 @@ mod delegate { /// Note that some arguments are preset based on typical use, and `features` are preset to maximize options. /// The `server` capabilities can be used to see which additional capabilities the server supports as per the handshake which happened prior. /// - /// If the delegate returns [`ls_refs::Action::Skip`], no `ls-refs` command is sent to the server. + /// If the delegate returns [`RefsAction::Skip`], no `ls-refs` command is sent to the server. /// /// Note that this is called only if we are using protocol version 2. - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> std::io::Result { + fn action(&mut self) -> std::io::Result { Ok(RefsAction::Continue) } @@ -299,8 +318,8 @@ mod delegate { self.deref().handshake_extra_parameters() } - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { - self.deref_mut().prepare_ls_refs(_server) + fn action(&mut self) -> io::Result { + self.deref_mut().action() } fn prepare_fetch( @@ -328,8 +347,8 @@ mod delegate { self.deref().handshake_extra_parameters() } - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { - self.deref_mut().prepare_ls_refs(_server) + fn action(&mut self) -> io::Result { + self.deref_mut().action() } fn prepare_fetch( diff --git a/gix-protocol/tests/protocol/fetch/mod.rs b/gix-protocol/tests/protocol/fetch/mod.rs index c54295cd028..b26ffc45007 100644 --- a/gix-protocol/tests/protocol/fetch/mod.rs +++ b/gix-protocol/tests/protocol/fetch/mod.rs @@ -4,14 +4,13 @@ use bstr::{BString, ByteSlice}; use gix_protocol::{ fetch::{Arguments, Response}, handshake, - ls_refs::{self, RefsAction}, }; use gix_transport::client::Capabilities; use crate::fixture_bytes; pub(super) mod _impl; -use _impl::{Action, DelegateBlocking}; +use _impl::{Action, DelegateBlocking, RefsAction}; mod error { use std::io; @@ -103,7 +102,7 @@ pub struct CloneRefInWantDelegate { } impl DelegateBlocking for CloneRefInWantDelegate { - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result { + fn action(&mut self) -> io::Result { Ok(RefsAction::Skip) } @@ -142,7 +141,7 @@ impl DelegateBlocking for LsRemoteDelegate { fn handshake_extra_parameters(&self) -> Vec<(String, Option)> { vec![("value-only".into(), None), ("key".into(), Some("value".into()))] } - fn prepare_ls_refs(&mut self, _server: &Capabilities) -> std::io::Result { + fn action(&mut self) -> std::io::Result { match self.abort_with.take() { Some(err) => Err(err), None => Ok(RefsAction::Continue), diff --git a/gix-protocol/tests/protocol/fetch/v2.rs b/gix-protocol/tests/protocol/fetch/v2.rs index 58ba0fe0d97..97477401cb7 100644 --- a/gix-protocol/tests/protocol/fetch/v2.rs +++ b/gix-protocol/tests/protocol/fetch/v2.rs @@ -1,6 +1,6 @@ use bstr::ByteSlice; use gix_features::progress; -use gix_protocol::{handshake, ls_refs}; +use gix_protocol::handshake; use gix_transport::Protocol; use crate::fetch::{ @@ -143,7 +143,7 @@ async fn ls_remote_abort_in_prep_ls_refs() -> crate::Result { b"0044git-upload-pack does/not/matter\x00\x00version=2\x00value-only\x00key=value\x000000".as_bstr() ); match err { - Error::LsRefs(ls_refs::Error::Io(err)) => { + Error::Io(err) => { assert_eq!(err.kind(), std::io::ErrorKind::Other); assert_eq!(err.get_ref().expect("other error").to_string(), "hello world"); } From 3b4e06ac9f6b600f58c459a034e6cfaac7221514 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 25 Nov 2025 12:37:30 +0100 Subject: [PATCH 06/12] refactor!: extract LsRefsCommand type to split preparation --- gix-protocol/src/fetch/refmap/init.rs | 24 ++--- gix-protocol/src/lib.rs | 4 +- gix-protocol/src/ls_refs.rs | 113 +++++++++++++-------- gix-protocol/tests/protocol/fetch/_impl.rs | 14 +-- 4 files changed, 85 insertions(+), 70 deletions(-) diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index 01889432e12..454851e0432 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -77,16 +77,14 @@ impl RefMap { { let _span = gix_trace::coarse!("gix_protocol::fetch::RefMap::new()"); let all_refspecs = context.aggregate_refspecs(); - let remote_refs = crate::ls_refs( - transport, - capabilities, - push_prefix_arguments(prefix_from_spec_as_filter_on_remote, &all_refspecs), - &mut progress, - trace_packetlines, - user_agent, - ) - .await?; + let mut refs_cmd = crate::LsRefsCommand::new(capabilities, user_agent); + push_prefix_arguments( + prefix_from_spec_as_filter_on_remote, + &all_refspecs, + refs_cmd.arguments(), + ); + let remote_refs = refs_cmd.invoke(transport, &mut progress, trace_packetlines).await?; Self::from_refs(remote_refs, capabilities, context) } @@ -162,12 +160,12 @@ impl RefMap { fn push_prefix_arguments( prefix_from_spec_as_filter_on_remote: bool, all_refspecs: &[gix_refspec::RefSpec], -) -> Vec { + arguments: &mut Vec, +) { if !prefix_from_spec_as_filter_on_remote { - return Vec::new(); + return; } - let mut arguments = Vec::new(); let mut seen = HashSet::new(); for spec in all_refspecs { let spec = spec.to_ref(); @@ -180,6 +178,4 @@ fn push_prefix_arguments( } } } - - arguments } diff --git a/gix-protocol/src/lib.rs b/gix-protocol/src/lib.rs index 9765b102d61..9f0f070e3fa 100644 --- a/gix-protocol/src/lib.rs +++ b/gix-protocol/src/lib.rs @@ -5,7 +5,7 @@ //! * create a `Transport`, either blocking or async //! * perform a [`handshake()`] //! * execute a [`Command`] -//! - [list references](ls_refs()) +//! - [list references](LsRefsCommand) //! - create a mapping between [refspecs and references](fetch::RefMap) //! - [receive a pack](fetch()) //! @@ -67,7 +67,7 @@ pub use handshake::hero::Handshake; /// pub mod ls_refs; #[cfg(any(feature = "blocking-client", feature = "async-client"))] -pub use ls_refs::function::ls_refs; +pub use ls_refs::function::LsRefsCommand; mod util; pub use util::*; diff --git a/gix-protocol/src/ls_refs.rs b/gix-protocol/src/ls_refs.rs index 79a16545178..3761ecaa962 100644 --- a/gix-protocol/src/ls_refs.rs +++ b/gix-protocol/src/ls_refs.rs @@ -2,7 +2,7 @@ mod error { use crate::handshake::refs::parse; - /// The error returned by [`ls_refs()`][crate::ls_refs()]. + /// The error returned by invoking a [`super::function::LsRefsCommand`]. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { @@ -48,51 +48,76 @@ pub(crate) mod function { Command, }; - /// Invoke a ls-refs V2 command on `transport`, which requires a prior handshake that yielded - /// server `capabilities`. `prepare_ls_refs(capabilities)` can be used to alter the _ls-refs_. - /// `arguments` are extra arguments to send to the server. - /// `progress` is used to provide feedback. - /// The `agent` information will be added to the features sent to the server. - /// If `trace` is `true`, all packetlines received or sent will be passed to the facilities of the `gix-trace` crate. - #[maybe_async] - pub async fn ls_refs( - mut transport: impl Transport, - capabilities: &Capabilities, - extra_args: Vec, - progress: &mut impl Progress, - trace: bool, - agent: (&'static str, Option>), - ) -> Result, Error> { - let _span = gix_features::trace::detail!("gix_protocol::ls_refs()", capabilities = ?capabilities); - let ls_refs = Command::LsRefs; - let mut ls_features = ls_refs.default_features(gix_transport::Protocol::V2, capabilities); - ls_features.push(agent); - let mut ls_args = ls_refs.initial_v2_arguments(&ls_features); - if capabilities - .capability("ls-refs") - .and_then(|cap| cap.supports("unborn")) - .unwrap_or_default() - { - ls_args.push("unborn".into()); + /// A command to list references from a remote Git repository. + pub struct LsRefsCommand<'a> { + capabilities: &'a Capabilities, + features: Vec<(&'static str, Option>)>, + arguments: Vec, + } + + impl<'a> LsRefsCommand<'a> { + /// Build a command to list refs from the given server `capabilities`, + /// using `agent` information to identify ourselves. + pub fn new(capabilities: &'a Capabilities, agent: (&'static str, Option>)) -> Self { + let _span = + gix_features::trace::detail!("gix_protocol::LsRefsCommand::new()", capabilities = ?capabilities); + let ls_refs = Command::LsRefs; + let mut features = ls_refs.default_features(gix_transport::Protocol::V2, capabilities); + features.push(agent); + let mut arguments = ls_refs.initial_v2_arguments(&features); + if capabilities + .capability("ls-refs") + .and_then(|cap| cap.supports("unborn")) + .unwrap_or_default() + { + arguments.push("unborn".into()); + } + + Self { + capabilities, + features, + arguments, + } } - ls_args.extend(extra_args); - ls_refs.validate_argument_prefixes(gix_transport::Protocol::V2, capabilities, &ls_args, &ls_features)?; + /// Invoke a ls-refs V2 command on `transport`. + /// + /// `progress` is used to provide feedback. + /// If `trace` is `true`, all packetlines received or sent will be passed to the facilities of the `gix-trace` crate. + #[maybe_async] + pub async fn invoke( + self, + mut transport: impl Transport, + progress: &mut impl Progress, + trace: bool, + ) -> Result, Error> { + Command::LsRefs.validate_argument_prefixes( + gix_transport::Protocol::V2, + self.capabilities, + &self.arguments, + &self.features, + )?; + + progress.step(); + progress.set_name("list refs".into()); + let mut remote_refs = transport + .invoke( + Command::LsRefs.as_str(), + self.features.into_iter(), + if self.arguments.is_empty() { + None + } else { + Some(self.arguments.into_iter()) + }, + trace, + ) + .await?; + Ok(from_v2_refs(&mut remote_refs).await?) + } - progress.step(); - progress.set_name("list refs".into()); - let mut remote_refs = transport - .invoke( - ls_refs.as_str(), - ls_features.into_iter(), - if ls_args.is_empty() { - None - } else { - Some(ls_args.into_iter()) - }, - trace, - ) - .await?; - Ok(from_v2_refs(&mut remote_refs).await?) + /// The arguments that will be sent to the server as part of the ls-refs command. + pub fn arguments(&mut self) -> &mut Vec { + &mut self.arguments + } } } diff --git a/gix-protocol/tests/protocol/fetch/_impl.rs b/gix-protocol/tests/protocol/fetch/_impl.rs index 68a22f1cc45..a6241e8030b 100644 --- a/gix-protocol/tests/protocol/fetch/_impl.rs +++ b/gix-protocol/tests/protocol/fetch/_impl.rs @@ -17,7 +17,7 @@ mod fetch_fn { use gix_protocol::{ credentials, fetch::{Arguments, Response}, - indicate_end_of_interaction, Command, + indicate_end_of_interaction, Command, LsRefsCommand, }; #[cfg(feature = "async-client")] use gix_transport::client::async_io::{ExtendedBufRead, HandleProgress, Transport}; @@ -106,15 +106,9 @@ mod fetch_fn { None => match delegate.action() { Ok(RefsAction::Skip) => Vec::new(), Ok(RefsAction::Continue) => { - gix_protocol::ls_refs( - &mut transport, - &capabilities, - Vec::new(), - &mut progress, - trace, - ("agent", Some(Cow::Owned(agent.clone()))), - ) - .await? + LsRefsCommand::new(&capabilities, ("agent", Some(Cow::Owned(agent.clone())))) + .invoke(&mut transport, &mut progress, trace) + .await? } Err(err) => { indicate_end_of_interaction(transport, trace).await?; From 0f58b0184b34df3b4e530ce4f7ab26369f882f99 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 25 Nov 2025 14:06:06 +0100 Subject: [PATCH 07/12] protocol: extract condition from helper function --- gix-protocol/src/fetch/refmap/init.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index 454851e0432..ecf7035f00d 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -78,11 +78,9 @@ impl RefMap { let _span = gix_trace::coarse!("gix_protocol::fetch::RefMap::new()"); let all_refspecs = context.aggregate_refspecs(); let mut refs_cmd = crate::LsRefsCommand::new(capabilities, user_agent); - push_prefix_arguments( - prefix_from_spec_as_filter_on_remote, - &all_refspecs, - refs_cmd.arguments(), - ); + if prefix_from_spec_as_filter_on_remote { + push_prefix_arguments(&all_refspecs, refs_cmd.arguments()); + } let remote_refs = refs_cmd.invoke(transport, &mut progress, trace_packetlines).await?; Self::from_refs(remote_refs, capabilities, context) @@ -157,15 +155,7 @@ impl RefMap { } } -fn push_prefix_arguments( - prefix_from_spec_as_filter_on_remote: bool, - all_refspecs: &[gix_refspec::RefSpec], - arguments: &mut Vec, -) { - if !prefix_from_spec_as_filter_on_remote { - return; - } - +fn push_prefix_arguments(all_refspecs: &[gix_refspec::RefSpec], arguments: &mut Vec) { let mut seen = HashSet::new(); for spec in all_refspecs { let spec = spec.to_ref(); From 37a42bde69d1a80080f91a78514565508b278876 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 25 Nov 2025 14:07:27 +0100 Subject: [PATCH 08/12] protocol: attach push_prefix_arguments() to LsRefsCommand --- gix-protocol/src/fetch/refmap/init.rs | 21 +++------------------ gix-protocol/src/ls_refs.rs | 20 +++++++++++++++----- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index ecf7035f00d..738d7b611f2 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -1,6 +1,6 @@ -use std::{borrow::Cow, collections::HashSet}; +use std::borrow::Cow; -use bstr::{BString, ByteSlice, ByteVec}; +use bstr::{BString, ByteSlice}; use gix_features::progress::Progress; use gix_transport::client::Capabilities; @@ -79,7 +79,7 @@ impl RefMap { let all_refspecs = context.aggregate_refspecs(); let mut refs_cmd = crate::LsRefsCommand::new(capabilities, user_agent); if prefix_from_spec_as_filter_on_remote { - push_prefix_arguments(&all_refspecs, refs_cmd.arguments()); + refs_cmd.push_prefix_arguments(&all_refspecs); } let remote_refs = refs_cmd.invoke(transport, &mut progress, trace_packetlines).await?; @@ -154,18 +154,3 @@ impl RefMap { }) } } - -fn push_prefix_arguments(all_refspecs: &[gix_refspec::RefSpec], arguments: &mut Vec) { - let mut seen = HashSet::new(); - for spec in all_refspecs { - let spec = spec.to_ref(); - if seen.insert(spec.instruction()) { - let mut prefixes = Vec::with_capacity(1); - spec.expand_prefixes(&mut prefixes); - for mut prefix in prefixes { - prefix.insert_str(0, "ref-prefix "); - arguments.push(prefix); - } - } - } -} diff --git a/gix-protocol/src/ls_refs.rs b/gix-protocol/src/ls_refs.rs index 3761ecaa962..d806d59aa40 100644 --- a/gix-protocol/src/ls_refs.rs +++ b/gix-protocol/src/ls_refs.rs @@ -31,9 +31,9 @@ pub use error::Error; #[cfg(any(feature = "blocking-client", feature = "async-client"))] pub(crate) mod function { - use std::borrow::Cow; + use std::{borrow::Cow, collections::HashSet}; - use bstr::BString; + use bstr::{BString, ByteVec}; use gix_features::progress::Progress; use gix_transport::client::Capabilities; use maybe_async::maybe_async; @@ -115,9 +115,19 @@ pub(crate) mod function { Ok(from_v2_refs(&mut remote_refs).await?) } - /// The arguments that will be sent to the server as part of the ls-refs command. - pub fn arguments(&mut self) -> &mut Vec { - &mut self.arguments + pub(crate) fn push_prefix_arguments(&mut self, all_refspecs: &[gix_refspec::RefSpec]) { + let mut seen = HashSet::new(); + for spec in all_refspecs { + let spec = spec.to_ref(); + if seen.insert(spec.instruction()) { + let mut prefixes = Vec::with_capacity(1); + spec.expand_prefixes(&mut prefixes); + for mut prefix in prefixes { + prefix.insert_str(0, "ref-prefix "); + self.arguments.push(prefix); + } + } + } } } } From 7a60e7846a0e077a1c7f6d560461ee81ae0d4e85 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 25 Nov 2025 14:16:24 +0100 Subject: [PATCH 09/12] refactor!: move prefix argument handling into LsRefsCommand::new() --- gix-protocol/src/fetch/refmap/init.rs | 10 +++--- gix-protocol/src/ls_refs.rs | 36 ++++++++++++---------- gix-protocol/tests/protocol/fetch/_impl.rs | 2 +- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index 738d7b611f2..2a56a625637 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -77,12 +77,10 @@ impl RefMap { { let _span = gix_trace::coarse!("gix_protocol::fetch::RefMap::new()"); let all_refspecs = context.aggregate_refspecs(); - let mut refs_cmd = crate::LsRefsCommand::new(capabilities, user_agent); - if prefix_from_spec_as_filter_on_remote { - refs_cmd.push_prefix_arguments(&all_refspecs); - } - - let remote_refs = refs_cmd.invoke(transport, &mut progress, trace_packetlines).await?; + let prefix_refspecs = prefix_from_spec_as_filter_on_remote.then_some(&all_refspecs[..]); + let remote_refs = crate::LsRefsCommand::new(prefix_refspecs, capabilities, user_agent) + .invoke(transport, &mut progress, trace_packetlines) + .await?; Self::from_refs(remote_refs, capabilities, context) } diff --git a/gix-protocol/src/ls_refs.rs b/gix-protocol/src/ls_refs.rs index d806d59aa40..4bebf72b9c1 100644 --- a/gix-protocol/src/ls_refs.rs +++ b/gix-protocol/src/ls_refs.rs @@ -58,7 +58,11 @@ pub(crate) mod function { impl<'a> LsRefsCommand<'a> { /// Build a command to list refs from the given server `capabilities`, /// using `agent` information to identify ourselves. - pub fn new(capabilities: &'a Capabilities, agent: (&'static str, Option>)) -> Self { + pub fn new( + prefix_refspecs: Option<&[gix_refspec::RefSpec]>, + capabilities: &'a Capabilities, + agent: (&'static str, Option>), + ) -> Self { let _span = gix_features::trace::detail!("gix_protocol::LsRefsCommand::new()", capabilities = ?capabilities); let ls_refs = Command::LsRefs; @@ -73,6 +77,21 @@ pub(crate) mod function { arguments.push("unborn".into()); } + if let Some(refspecs) = prefix_refspecs { + let mut seen = HashSet::new(); + for spec in refspecs { + let spec = spec.to_ref(); + if seen.insert(spec.instruction()) { + let mut prefixes = Vec::with_capacity(1); + spec.expand_prefixes(&mut prefixes); + for mut prefix in prefixes { + prefix.insert_str(0, "ref-prefix "); + arguments.push(prefix); + } + } + } + } + Self { capabilities, features, @@ -114,20 +133,5 @@ pub(crate) mod function { .await?; Ok(from_v2_refs(&mut remote_refs).await?) } - - pub(crate) fn push_prefix_arguments(&mut self, all_refspecs: &[gix_refspec::RefSpec]) { - let mut seen = HashSet::new(); - for spec in all_refspecs { - let spec = spec.to_ref(); - if seen.insert(spec.instruction()) { - let mut prefixes = Vec::with_capacity(1); - spec.expand_prefixes(&mut prefixes); - for mut prefix in prefixes { - prefix.insert_str(0, "ref-prefix "); - self.arguments.push(prefix); - } - } - } - } } } diff --git a/gix-protocol/tests/protocol/fetch/_impl.rs b/gix-protocol/tests/protocol/fetch/_impl.rs index a6241e8030b..67bc8a7f8fe 100644 --- a/gix-protocol/tests/protocol/fetch/_impl.rs +++ b/gix-protocol/tests/protocol/fetch/_impl.rs @@ -106,7 +106,7 @@ mod fetch_fn { None => match delegate.action() { Ok(RefsAction::Skip) => Vec::new(), Ok(RefsAction::Continue) => { - LsRefsCommand::new(&capabilities, ("agent", Some(Cow::Owned(agent.clone())))) + LsRefsCommand::new(None, &capabilities, ("agent", Some(Cow::Owned(agent.clone())))) .invoke(&mut transport, &mut progress, trace) .await? } From 0021f9daec773f381c6f10bd310cd0f8c5077d54 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 25 Nov 2025 14:21:46 +0100 Subject: [PATCH 10/12] refactor!: inline RefMap::fetch() into Handshake::fetch_or_extract_refmap() --- gix-protocol/src/fetch/negotiate.rs | 2 +- gix-protocol/src/fetch/refmap/init.rs | 48 ++------------------------- gix-protocol/src/fetch/types.rs | 4 +-- gix-protocol/src/handshake/mod.rs | 28 +++++++--------- 4 files changed, 18 insertions(+), 64 deletions(-) diff --git a/gix-protocol/src/fetch/negotiate.rs b/gix-protocol/src/fetch/negotiate.rs index 7480aea9960..f0017d132d1 100644 --- a/gix-protocol/src/fetch/negotiate.rs +++ b/gix-protocol/src/fetch/negotiate.rs @@ -111,7 +111,7 @@ pub struct Round { /// * `graph` /// - The commit-graph for use by the `negotiator` - we populate it with tips to initialize the graph traversal. /// * `ref_map` -/// - The references known on the remote, as previously obtained with [`RefMap::fetch()`]. +/// - The references known on the remote, as previously obtained with [`crate::Handshake::fetch_or_extract_refmap()`]. /// * `shallow` /// - How to deal with shallow repositories. It does affect how negotiations are performed. /// * `mapping_is_ignored` diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index 2a56a625637..da670c1618c 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -1,13 +1,6 @@ -use std::borrow::Cow; - use bstr::{BString, ByteSlice}; -use gix_features::progress::Progress; use gix_transport::client::Capabilities; -#[cfg(feature = "async-client")] -use crate::transport::client::async_io::Transport; -#[cfg(feature = "blocking-client")] -use crate::transport::client::blocking_io::Transport; use crate::{ fetch::{ refmap::{Mapping, Source, SpecIndex}, @@ -16,7 +9,7 @@ use crate::{ handshake::Ref, }; -/// The error returned by [`RefMap::fetch()`]. +/// The error returned by [`crate::Handshake::fetch_or_extract_refmap()`]. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { @@ -28,7 +21,7 @@ pub enum Error { ListRefs(#[from] crate::ls_refs::Error), } -/// For use in [`RefMap::fetch()`]. +/// For use in [`RefMap::from_refs()`]. #[derive(Debug, Clone)] pub struct Context { /// All explicit refspecs to identify references on the remote that you are interested in. @@ -41,7 +34,7 @@ pub struct Context { } impl Context { - fn aggregate_refspecs(&self) -> Vec { + pub(crate) fn aggregate_refspecs(&self) -> Vec { let mut all_refspecs = self.fetch_refspecs.clone(); all_refspecs.extend(self.extra_refspecs.iter().cloned()); all_refspecs @@ -49,41 +42,6 @@ impl Context { } impl RefMap { - /// Create a new instance by obtaining all references on the remote that have been filtered through our remote's specs - /// for _fetching_. - /// - /// * `progress` is used if `ls-refs` is invoked on the remote. Always the case when V2 is used. - /// * `capabilities` are the capabilities of the server, obtained by a [handshake](crate::handshake()). - /// * `transport` is a way to communicate with the server to obtain the reference listing. - /// * `user_agent` is passed to the server. - /// * `trace_packetlines` traces all packet lines if `true`, for debugging primarily. - /// * `prefix_from_spec_as_filter_on_remote` - /// - Use a two-component prefix derived from the ref-spec's source, like `refs/heads/` to let the server pre-filter refs - /// with great potential for savings in traffic and local CPU time. - /// * `context` to provide more [configuration](Context). - #[allow(clippy::result_large_err)] - #[maybe_async::maybe_async] - pub async fn fetch( - mut progress: impl Progress, - capabilities: &Capabilities, - transport: &mut T, - user_agent: (&'static str, Option>), - trace_packetlines: bool, - prefix_from_spec_as_filter_on_remote: bool, - context: Context, - ) -> Result - where - T: Transport, - { - let _span = gix_trace::coarse!("gix_protocol::fetch::RefMap::new()"); - let all_refspecs = context.aggregate_refspecs(); - let prefix_refspecs = prefix_from_spec_as_filter_on_remote.then_some(&all_refspecs[..]); - let remote_refs = crate::LsRefsCommand::new(prefix_refspecs, capabilities, user_agent) - .invoke(transport, &mut progress, trace_packetlines) - .await?; - Self::from_refs(remote_refs, capabilities, context) - } - /// Create a ref-map from already obtained `remote_refs`. Use `context` to pass in refspecs. /// `capabilities` are used to determine the object format. pub fn from_refs(remote_refs: Vec, capabilities: &Capabilities, context: Context) -> Result { diff --git a/gix-protocol/src/fetch/types.rs b/gix-protocol/src/fetch/types.rs index 549451f4703..99d3107abb7 100644 --- a/gix-protocol/src/fetch/types.rs +++ b/gix-protocol/src/fetch/types.rs @@ -18,7 +18,7 @@ pub struct Options<'a> { pub reject_shallow_remote: bool, } -/// For use in [`RefMap::fetch()`] and [`fetch`](crate::fetch()). +/// For use in [`crate::Handshake::fetch_or_extract_refmap()`] and [`fetch`](crate::fetch()). #[cfg(feature = "handshake")] pub struct Context<'a, T> { /// The outcome of the handshake performed with the remote. @@ -29,7 +29,7 @@ pub struct Context<'a, T> { /// /// This is always done if the underlying protocol is V2, which is implied by the absence of refs in the `handshake` outcome. pub transport: &'a mut T, - /// How to self-identify during the `ls-refs` call in [`RefMap::fetch()`] or the `fetch` call in [`fetch()`](crate::fetch()). + /// How to self-identify during the `ls-refs` call in [`crate::Handshake::fetch_or_extract_refmap()`] or the `fetch` call in [`fetch()`](crate::fetch()). /// /// This could be read from the `gitoxide.userAgent` configuration variable. pub user_agent: (&'static str, Option>), diff --git a/gix-protocol/src/handshake/mod.rs b/gix-protocol/src/handshake/mod.rs index 31128063a52..42686e638e0 100644 --- a/gix-protocol/src/handshake/mod.rs +++ b/gix-protocol/src/handshake/mod.rs @@ -75,7 +75,7 @@ pub(crate) mod hero { use crate::transport::client::async_io::Transport; #[cfg(feature = "blocking-client")] use crate::transport::client::blocking_io::Transport; - use crate::Handshake; + use crate::{fetch::RefMap, Handshake}; use gix_features::progress::Progress; use std::borrow::Cow; @@ -96,21 +96,17 @@ pub(crate) mod hero { where T: Transport, { - Ok(match self.refs.take() { - Some(refs) => crate::fetch::RefMap::from_refs(refs, &self.capabilities, refmap_context)?, - None => { - crate::fetch::RefMap::fetch( - &mut progress, - &self.capabilities, - transport, - user_agent, - trace_packetlines, - prefix_from_spec_as_filter_on_remote, - refmap_context, - ) - .await? - } - }) + if let Some(refs) = self.refs.take() { + return crate::fetch::RefMap::from_refs(refs, &self.capabilities, refmap_context); + } + + let _span = gix_trace::coarse!("gix_protocol::handshake::fetch_or_extract_refmap()"); + let all_refspecs = refmap_context.aggregate_refspecs(); + let prefix_refspecs = prefix_from_spec_as_filter_on_remote.then_some(&all_refspecs[..]); + let remote_refs = crate::LsRefsCommand::new(prefix_refspecs, &self.capabilities, user_agent) + .invoke(transport, &mut progress, trace_packetlines) + .await?; + RefMap::from_refs(remote_refs, &self.capabilities, refmap_context) } } } From c26efb9e733cb33a38ef885eded915e17e61bd9c Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 25 Nov 2025 20:06:33 +0100 Subject: [PATCH 11/12] refactor!: split Handshake::fetch_or_extract_refmap() --- gitoxide-core/src/pack/receive.rs | 10 +---- gix-protocol/src/handshake/mod.rs | 58 ++++++++++++++++++++-------- gix-protocol/src/ls_refs.rs | 2 +- gix/src/remote/connection/ref_map.rs | 8 ++-- 4 files changed, 49 insertions(+), 29 deletions(-) diff --git a/gitoxide-core/src/pack/receive.rs b/gitoxide-core/src/pack/receive.rs index 0be29d7afd0..f3fd0fda179 100644 --- a/gitoxide-core/src/pack/receive.rs +++ b/gitoxide-core/src/pack/receive.rs @@ -88,14 +88,8 @@ where extra_refspecs: vec![], }; let refmap = handshake - .fetch_or_extract_refmap( - &mut progress, - &mut transport.inner, - user_agent.clone(), - trace_packetlines, - true, - context, - ) + .fetch_or_extract_refmap(user_agent.clone(), true, context)? + .fetch(&mut progress, &mut transport.inner, trace_packetlines) .await?; if refmap.mappings.is_empty() && !refmap.remote_refs.is_empty() { diff --git a/gix-protocol/src/handshake/mod.rs b/gix-protocol/src/handshake/mod.rs index 42686e638e0..b3ec551ba6a 100644 --- a/gix-protocol/src/handshake/mod.rs +++ b/gix-protocol/src/handshake/mod.rs @@ -79,34 +79,59 @@ pub(crate) mod hero { use gix_features::progress::Progress; use std::borrow::Cow; - impl Handshake { - /// Obtain a [refmap](crate::fetch::RefMap) either from this instance, taking it out in the process, if the handshake was - /// created from a V1 connection, or use `transport` to fetch the refmap as a separate command invocation. + /// Intermediate state while potentially fetching a refmap after the handshake. + pub enum FetchRefMap<'a> { + /// We already got a refmap to use from the handshake. + Map(RefMap), + /// We need to invoke another command to get the refmap. + Command(crate::LsRefsCommand<'a>, crate::fetch::refmap::init::Context), + } + + impl FetchRefMap<'_> { + /// Fetch the refmap, either by returning the existing one or invoking the command. #[allow(clippy::result_large_err)] #[maybe_async::maybe_async] - pub async fn fetch_or_extract_refmap( - &mut self, + pub async fn fetch( + self, mut progress: impl Progress, - transport: &mut T, - user_agent: (&'static str, Option>), + transport: &mut impl Transport, trace_packetlines: bool, + ) -> Result { + let (cmd, cx) = match self { + FetchRefMap::Map(map) => return Ok(map), + FetchRefMap::Command(cmd, cx) => (cmd, cx), + }; + + let capabilities = cmd.capabilities; + let remote_refs = cmd.invoke(transport, &mut progress, trace_packetlines).await?; + RefMap::from_refs(remote_refs, capabilities, cx) + } + } + + impl Handshake { + /// Prepare fetching a [refmap](crate::fetch::RefMap) if not present in the handshake. + #[allow(clippy::result_large_err)] + pub fn fetch_or_extract_refmap( + &mut self, + user_agent: (&'static str, Option>), prefix_from_spec_as_filter_on_remote: bool, refmap_context: crate::fetch::refmap::init::Context, - ) -> Result - where - T: Transport, - { + ) -> Result, crate::fetch::refmap::init::Error> { if let Some(refs) = self.refs.take() { - return crate::fetch::RefMap::from_refs(refs, &self.capabilities, refmap_context); + return Ok(FetchRefMap::Map(crate::fetch::RefMap::from_refs( + refs, + &self.capabilities, + refmap_context, + )?)); } let _span = gix_trace::coarse!("gix_protocol::handshake::fetch_or_extract_refmap()"); let all_refspecs = refmap_context.aggregate_refspecs(); let prefix_refspecs = prefix_from_spec_as_filter_on_remote.then_some(&all_refspecs[..]); - let remote_refs = crate::LsRefsCommand::new(prefix_refspecs, &self.capabilities, user_agent) - .invoke(transport, &mut progress, trace_packetlines) - .await?; - RefMap::from_refs(remote_refs, &self.capabilities, refmap_context) + Ok(FetchRefMap::Command( + crate::LsRefsCommand::new(prefix_refspecs, &self.capabilities, user_agent), + refmap_context, + )) } } } @@ -146,6 +171,7 @@ mod error { } } } + #[cfg(feature = "handshake")] pub use error::Error; diff --git a/gix-protocol/src/ls_refs.rs b/gix-protocol/src/ls_refs.rs index 4bebf72b9c1..f2f408d5fa8 100644 --- a/gix-protocol/src/ls_refs.rs +++ b/gix-protocol/src/ls_refs.rs @@ -50,7 +50,7 @@ pub(crate) mod function { /// A command to list references from a remote Git repository. pub struct LsRefsCommand<'a> { - capabilities: &'a Capabilities, + pub(crate) capabilities: &'a Capabilities, features: Vec<(&'static str, Option>)>, arguments: Vec, } diff --git a/gix/src/remote/connection/ref_map.rs b/gix/src/remote/connection/ref_map.rs index e149705dc6b..da04ef880c8 100644 --- a/gix/src/remote/connection/ref_map.rs +++ b/gix/src/remote/connection/ref_map.rs @@ -156,16 +156,16 @@ where fetch_refspecs: self.remote.fetch_specs.clone(), extra_refspecs, }; + let ref_map = handshake .fetch_or_extract_refmap( - progress, - &mut self.transport.inner, self.remote.repo.config.user_agent_tuple(), - self.trace, prefix_from_spec_as_filter_on_remote, context, - ) + )? + .fetch(progress, &mut self.transport.inner, self.trace) .await?; + self.handshake = Some(handshake); Ok(ref_map) } From ab3cce472efc4f6469d99a0d3a425d58ac2140b9 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 25 Nov 2025 14:29:51 +0100 Subject: [PATCH 12/12] refactor!: split async and blocking implementations for Handshake::fetch_or_extract_refmap() --- gitoxide-core/src/pack/receive.rs | 12 ++++-- gix-protocol/src/handshake/mod.rs | 31 ++++++++++++--- gix-protocol/src/ls_refs.rs | 44 +++++++++++++++++++--- gix-protocol/tests/protocol/fetch/_impl.rs | 14 +++++-- gix/src/remote/connection/ref_map.rs | 19 ++++++---- 5 files changed, 95 insertions(+), 25 deletions(-) diff --git a/gitoxide-core/src/pack/receive.rs b/gitoxide-core/src/pack/receive.rs index f3fd0fda179..4d354eef9c3 100644 --- a/gitoxide-core/src/pack/receive.rs +++ b/gitoxide-core/src/pack/receive.rs @@ -87,11 +87,17 @@ where fetch_refspecs: fetch_refspecs.clone(), extra_refspecs: vec![], }; - let refmap = handshake - .fetch_or_extract_refmap(user_agent.clone(), true, context)? - .fetch(&mut progress, &mut transport.inner, trace_packetlines) + + let fetch_refmap = handshake.fetch_or_extract_refmap(user_agent.clone(), true, context)?; + + #[cfg(feature = "async-client")] + let refmap = fetch_refmap + .fetch_async(&mut progress, &mut transport.inner, trace_packetlines) .await?; + #[cfg(feature = "blocking-client")] + let refmap = fetch_refmap.fetch_blocking(&mut progress, &mut transport.inner, trace_packetlines)?; + if refmap.mappings.is_empty() && !refmap.remote_refs.is_empty() { return Err(Error::NoMapping { refspecs: refmap.refspecs.clone(), diff --git a/gix-protocol/src/handshake/mod.rs b/gix-protocol/src/handshake/mod.rs index b3ec551ba6a..6f3ddfe0842 100644 --- a/gix-protocol/src/handshake/mod.rs +++ b/gix-protocol/src/handshake/mod.rs @@ -72,9 +72,9 @@ pub(crate) mod hero { #[cfg(feature = "fetch")] mod fetch { #[cfg(feature = "async-client")] - use crate::transport::client::async_io::Transport; + use crate::transport::client::async_io; #[cfg(feature = "blocking-client")] - use crate::transport::client::blocking_io::Transport; + use crate::transport::client::blocking_io; use crate::{fetch::RefMap, Handshake}; use gix_features::progress::Progress; use std::borrow::Cow; @@ -89,12 +89,12 @@ pub(crate) mod hero { impl FetchRefMap<'_> { /// Fetch the refmap, either by returning the existing one or invoking the command. + #[cfg(feature = "async-client")] #[allow(clippy::result_large_err)] - #[maybe_async::maybe_async] - pub async fn fetch( + pub async fn fetch_async( self, mut progress: impl Progress, - transport: &mut impl Transport, + transport: &mut impl async_io::Transport, trace_packetlines: bool, ) -> Result { let (cmd, cx) = match self { @@ -103,7 +103,26 @@ pub(crate) mod hero { }; let capabilities = cmd.capabilities; - let remote_refs = cmd.invoke(transport, &mut progress, trace_packetlines).await?; + let remote_refs = cmd.invoke_async(transport, &mut progress, trace_packetlines).await?; + RefMap::from_refs(remote_refs, capabilities, cx) + } + + /// Fetch the refmap, either by returning the existing one or invoking the command. + #[cfg(feature = "blocking-client")] + #[allow(clippy::result_large_err)] + pub fn fetch_blocking( + self, + mut progress: impl Progress, + transport: &mut impl blocking_io::Transport, + trace_packetlines: bool, + ) -> Result { + let (cmd, cx) = match self { + FetchRefMap::Map(map) => return Ok(map), + FetchRefMap::Command(cmd, cx) => (cmd, cx), + }; + + let capabilities = cmd.capabilities; + let remote_refs = cmd.invoke_blocking(transport, &mut progress, trace_packetlines)?; RefMap::from_refs(remote_refs, capabilities, cx) } } diff --git a/gix-protocol/src/ls_refs.rs b/gix-protocol/src/ls_refs.rs index f2f408d5fa8..f994e11346d 100644 --- a/gix-protocol/src/ls_refs.rs +++ b/gix-protocol/src/ls_refs.rs @@ -36,13 +36,12 @@ pub(crate) mod function { use bstr::{BString, ByteVec}; use gix_features::progress::Progress; use gix_transport::client::Capabilities; - use maybe_async::maybe_async; use super::Error; #[cfg(feature = "async-client")] - use crate::transport::client::async_io::{Transport, TransportV2Ext}; + use crate::transport::client::async_io::{self, TransportV2Ext as _}; #[cfg(feature = "blocking-client")] - use crate::transport::client::blocking_io::{Transport, TransportV2Ext}; + use crate::transport::client::blocking_io::{self, TransportV2Ext as _}; use crate::{ handshake::{refs::from_v2_refs, Ref}, Command, @@ -103,10 +102,10 @@ pub(crate) mod function { /// /// `progress` is used to provide feedback. /// If `trace` is `true`, all packetlines received or sent will be passed to the facilities of the `gix-trace` crate. - #[maybe_async] - pub async fn invoke( + #[cfg(feature = "async-client")] + pub async fn invoke_async( self, - mut transport: impl Transport, + mut transport: impl async_io::Transport, progress: &mut impl Progress, trace: bool, ) -> Result, Error> { @@ -133,5 +132,38 @@ pub(crate) mod function { .await?; Ok(from_v2_refs(&mut remote_refs).await?) } + + /// Invoke a ls-refs V2 command on `transport`. + /// + /// `progress` is used to provide feedback. + /// If `trace` is `true`, all packetlines received or sent will be passed to the facilities of the `gix-trace` crate. + #[cfg(feature = "blocking-client")] + pub fn invoke_blocking( + self, + mut transport: impl blocking_io::Transport, + progress: &mut impl Progress, + trace: bool, + ) -> Result, Error> { + Command::LsRefs.validate_argument_prefixes( + gix_transport::Protocol::V2, + self.capabilities, + &self.arguments, + &self.features, + )?; + + progress.step(); + progress.set_name("list refs".into()); + let mut remote_refs = transport.invoke( + Command::LsRefs.as_str(), + self.features.into_iter(), + if self.arguments.is_empty() { + None + } else { + Some(self.arguments.into_iter()) + }, + trace, + )?; + Ok(from_v2_refs(&mut remote_refs)?) + } } } diff --git a/gix-protocol/tests/protocol/fetch/_impl.rs b/gix-protocol/tests/protocol/fetch/_impl.rs index 67bc8a7f8fe..12e8e98beb1 100644 --- a/gix-protocol/tests/protocol/fetch/_impl.rs +++ b/gix-protocol/tests/protocol/fetch/_impl.rs @@ -106,9 +106,17 @@ mod fetch_fn { None => match delegate.action() { Ok(RefsAction::Skip) => Vec::new(), Ok(RefsAction::Continue) => { - LsRefsCommand::new(None, &capabilities, ("agent", Some(Cow::Owned(agent.clone())))) - .invoke(&mut transport, &mut progress, trace) - .await? + #[cfg(feature = "async-client")] + { + LsRefsCommand::new(None, &capabilities, ("agent", Some(Cow::Owned(agent.clone())))) + .invoke_async(&mut transport, &mut progress, trace) + .await? + } + #[cfg(feature = "blocking-client")] + { + LsRefsCommand::new(None, &capabilities, ("agent", Some(Cow::Owned(agent.clone())))) + .invoke_blocking(&mut transport, &mut progress, trace)? + } } Err(err) => { indicate_end_of_interaction(transport, trace).await?; diff --git a/gix/src/remote/connection/ref_map.rs b/gix/src/remote/connection/ref_map.rs index da04ef880c8..c67d834a075 100644 --- a/gix/src/remote/connection/ref_map.rs +++ b/gix/src/remote/connection/ref_map.rs @@ -157,15 +157,20 @@ where extra_refspecs, }; - let ref_map = handshake - .fetch_or_extract_refmap( - self.remote.repo.config.user_agent_tuple(), - prefix_from_spec_as_filter_on_remote, - context, - )? - .fetch(progress, &mut self.transport.inner, self.trace) + let fetch_refmap = handshake.fetch_or_extract_refmap( + self.remote.repo.config.user_agent_tuple(), + prefix_from_spec_as_filter_on_remote, + context, + )?; + + #[cfg(feature = "async-network-client")] + let ref_map = fetch_refmap + .fetch_async(progress, &mut self.transport.inner, self.trace) .await?; + #[cfg(feature = "blocking-network-client")] + let ref_map = fetch_refmap.fetch_blocking(progress, &mut self.transport.inner, self.trace)?; + self.handshake = Some(handshake); Ok(ref_map) }