From 0105b352235287de4e756b975335ebe219639d1b Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 1 Nov 2025 17:09:45 +0100 Subject: [PATCH 01/36] fix: silence warning for unused client impl --- gix-transport/src/client/blocking_io/connect.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gix-transport/src/client/blocking_io/connect.rs b/gix-transport/src/client/blocking_io/connect.rs index d96f3e4e7bd..ffc4c5b2a53 100644 --- a/gix-transport/src/client/blocking_io/connect.rs +++ b/gix-transport/src/client/blocking_io/connect.rs @@ -3,7 +3,7 @@ pub use crate::client::non_io_types::connect::{Error, Options}; pub(crate) mod function { #[cfg(feature = "http-client-curl")] use crate::client::blocking_io::http::curl::Curl; - #[cfg(feature = "http-client-reqwest")] + #[cfg(all(feature = "http-client-reqwest", not(feature = "http-client-curl")))] use crate::client::blocking_io::http::reqwest::Remote as Reqwest; use crate::client::{blocking_io::Transport, non_io_types::connect::Error}; From 9de190ee76a8f0af2bd7a40d09719c2b09a37a24 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 1 Nov 2025 17:12:00 +0100 Subject: [PATCH 02/36] fix: forward serde feature to bstr in gix-transport --- gix-transport/Cargo.toml | 2 +- justfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gix-transport/Cargo.toml b/gix-transport/Cargo.toml index 80f3f2a4ccb..28983270981 100644 --- a/gix-transport/Cargo.toml +++ b/gix-transport/Cargo.toml @@ -59,7 +59,7 @@ async-client = [ #! ### Other ## Data structures implement `serde::Serialize` and `serde::Deserialize`. -serde = ["dep:serde"] +serde = ["dep:serde", "bstr/serde"] [[test]] name = "blocking-transport" diff --git a/justfile b/justfile index 81ab31ac8d9..eeb7a76229e 100755 --- a/justfile +++ b/justfile @@ -45,10 +45,10 @@ check: cargo check --workspace cargo check --no-default-features --features small cargo check -p gix-packetline --all-features 2>/dev/null + cargo check -p gix-transport --all-features 2>/dev/null # assure compile error occurs ! cargo check --features lean-async 2>/dev/null ! cargo check -p gitoxide-core --all-features 2>/dev/null - ! cargo check -p gix-transport --all-features 2>/dev/null ! cargo check -p gix-protocol --all-features 2>/dev/null # warning happens if nothing found, no exit code :/ cargo --color=never tree -p gix --no-default-features -e normal -i imara-diff \ From 87d5419a35298d6627a5695790f91ead698b6fb0 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 1 Nov 2025 22:13:47 +0100 Subject: [PATCH 03/36] fix!: enable async I/O code independent of blocking_io --- .../tests/protocol/fetch/arguments.rs | 11 +++++----- gix-protocol/tests/protocol/fetch/mod.rs | 8 ++++---- gix-transport/src/client/async_io/connect.rs | 4 ++-- .../src/client/blocking_io/http/mod.rs | 7 ++++--- gix-transport/src/client/capabilities.rs | 14 ++++++------- gix-transport/src/client/git/async_io.rs | 20 ++++++++++--------- gix-transport/src/client/git/blocking_io.rs | 7 +++---- gix-transport/src/client/git/mod.rs | 12 +++++------ gix-transport/src/client/mod.rs | 2 +- gix-transport/tests/client/capabilities.rs | 6 +++++- gix-transport/tests/client/git.rs | 20 ++++++++++++------- 11 files changed, 61 insertions(+), 50 deletions(-) diff --git a/gix-protocol/tests/protocol/fetch/arguments.rs b/gix-protocol/tests/protocol/fetch/arguments.rs index da5e37d50ac..52b82075c67 100644 --- a/gix-protocol/tests/protocol/fetch/arguments.rs +++ b/gix-protocol/tests/protocol/fetch/arguments.rs @@ -2,6 +2,10 @@ use bstr::ByteSlice; use gix_transport::Protocol; use crate::fetch; +#[cfg(feature = "async-client")] +use gix_transport::client::git::async_io::Connection; +#[cfg(feature = "blocking-client")] +use gix_transport::client::git::blocking_io::Connection; fn arguments_v1(features: impl IntoIterator) -> fetch::Arguments { fetch::Arguments::new(Protocol::V1, features.into_iter().map(|n| (n, None)).collect(), false) @@ -140,12 +144,9 @@ mod impls { } } -fn transport( - out: &mut Vec, - stateful: bool, -) -> Transport>> { +fn transport(out: &mut Vec, stateful: bool) -> Transport>> { Transport { - inner: gix_transport::client::git::Connection::new( + inner: Connection::new( &[], out, Protocol::V1, // does not matter diff --git a/gix-protocol/tests/protocol/fetch/mod.rs b/gix-protocol/tests/protocol/fetch/mod.rs index 355afbb2850..03f69bed8d8 100644 --- a/gix-protocol/tests/protocol/fetch/mod.rs +++ b/gix-protocol/tests/protocol/fetch/mod.rs @@ -302,9 +302,9 @@ pub fn transport( path: &str, desired_version: gix_transport::Protocol, mode: gix_transport::client::git::ConnectMode, -) -> gix_transport::client::git::Connection { +) -> gix_transport::client::git::async_io::Connection { let response = fixture_bytes(path); - gix_transport::client::git::Connection::new( + gix_transport::client::git::async_io::Connection::new( Cursor::new(response), out, desired_version, @@ -321,9 +321,9 @@ pub fn transport( path: &str, version: gix_transport::Protocol, mode: gix_transport::client::git::ConnectMode, -) -> gix_transport::client::git::Connection { +) -> gix_transport::client::git::blocking_io::Connection { let response = fixture_bytes(path); - gix_transport::client::git::Connection::new( + gix_transport::client::git::blocking_io::Connection::new( Cursor::new(response), out, version, diff --git a/gix-transport/src/client/async_io/connect.rs b/gix-transport/src/client/async_io/connect.rs index 6b094328e4b..cd5a3a7fda1 100644 --- a/gix-transport/src/client/async_io/connect.rs +++ b/gix-transport/src/client/async_io/connect.rs @@ -2,7 +2,7 @@ pub use crate::client::non_io_types::connect::{Error, Options}; #[cfg(feature = "async-std")] pub(crate) mod function { - use crate::client::{async_io::Transport, git, non_io_types::connect::Error}; + use crate::client::{async_io::Transport, git::async_io::Connection, non_io_types::connect::Error}; /// A general purpose connector connecting to a repository identified by the given `url`. /// @@ -26,7 +26,7 @@ pub(crate) mod function { } let path = std::mem::take(&mut url.path); Box::new( - git::Connection::new_tcp( + Connection::new_tcp( url.host().expect("host is present in url"), url.port, path, diff --git a/gix-transport/src/client/blocking_io/http/mod.rs b/gix-transport/src/client/blocking_io/http/mod.rs index 454af724a55..864fe3809f7 100644 --- a/gix-transport/src/client/blocking_io/http/mod.rs +++ b/gix-transport/src/client/blocking_io/http/mod.rs @@ -19,7 +19,8 @@ use crate::{ http::options::{HttpVersion, SslVersionRangeInclusive}, ExtendedBufRead, HandleProgress, RequestWriter, SetServiceResponse, }, - capabilities, Capabilities, MessageKind, + capabilities::blocking_recv::Outcome, + MessageKind, }, packetline::{blocking_io::StreamingPeekableIter, PacketLineRef}, Protocol, Service, @@ -402,11 +403,11 @@ impl blocking_io::Transport for Transport { line_reader.as_read().read_to_end(&mut Vec::new())?; } - let capabilities::recv::Outcome { + let Outcome { capabilities, refs, protocol: actual_protocol, - } = Capabilities::from_lines_with_version_detection(line_reader)?; + } = Outcome::from_lines_with_version_detection(line_reader)?; self.actual_version = actual_protocol; self.service = Some(service); Ok(SetServiceResponse { diff --git a/gix-transport/src/client/capabilities.rs b/gix-transport/src/client/capabilities.rs index ddb3682d070..dc0fd2bd296 100644 --- a/gix-transport/src/client/capabilities.rs +++ b/gix-transport/src/client/capabilities.rs @@ -167,7 +167,7 @@ impl Capabilities { #[cfg(feature = "blocking-client")] /// -pub mod recv { +pub mod blocking_recv { use std::io; use bstr::ByteVec; @@ -178,7 +178,7 @@ pub mod recv { Protocol, }; - /// Success outcome of [`Capabilities::from_lines_with_version_detection`]. + /// Success outcome of [`Outcome::from_lines_with_version_detection`]. pub struct Outcome<'a> { /// The [`Capabilities`] the remote advertised. pub capabilities: Capabilities, @@ -191,7 +191,7 @@ pub mod recv { pub protocol: Protocol, } - impl Capabilities { + impl Outcome<'_> { /// Read the capabilities and version advertisement from the given packetline reader. /// /// If [`Protocol::V1`] was requested, or the remote decided to downgrade, the remote refs @@ -253,10 +253,10 @@ pub mod recv { } } -#[cfg(all(feature = "async-client", not(feature = "blocking-client")))] +#[cfg(feature = "async-client")] #[allow(missing_docs)] /// -pub mod recv { +pub mod async_recv { use bstr::ByteVec; use futures_io::AsyncRead; @@ -266,7 +266,7 @@ pub mod recv { Protocol, }; - /// Success outcome of [`Capabilities::from_lines_with_version_detection`]. + /// Success outcome of [`Outcome::from_lines_with_version_detection`]. pub struct Outcome<'a> { /// The [`Capabilities`] the remote advertised. pub capabilities: Capabilities, @@ -279,7 +279,7 @@ pub mod recv { pub protocol: Protocol, } - impl Capabilities { + impl Outcome<'_> { /// Read the capabilities and version advertisement from the given packetline reader. /// /// If [`Protocol::V1`] was requested, or the remote decided to downgrade, the remote refs diff --git a/gix-transport/src/client/git/async_io.rs b/gix-transport/src/client/git/async_io.rs index 9ecbfd84976..b8bfc6539bc 100644 --- a/gix-transport/src/client/git/async_io.rs +++ b/gix-transport/src/client/git/async_io.rs @@ -9,9 +9,8 @@ use crate::{ client::{ self, async_io::{RequestWriter, SetServiceResponse}, - capabilities, + capabilities::async_recv::Outcome, git::{self, ConnectionState}, - Capabilities, }, packetline::{ async_io::{StreamingPeekableIter, Writer}, @@ -97,11 +96,11 @@ where line_writer.flush().await?; } - let capabilities::recv::Outcome { + let Outcome { capabilities, refs, protocol: actual_protocol, - } = Capabilities::from_lines_with_version_detection(&mut self.line_provider).await?; + } = Outcome::from_lines_with_version_detection(&mut self.line_provider).await?; Ok(SetServiceResponse { actual_protocol, capabilities, @@ -164,9 +163,12 @@ mod async_net { use async_std::net::TcpStream; - use crate::client::{git, Error}; + use crate::client::{ + git::{async_io::Connection, ConnectMode}, + Error, + }; - impl git::Connection { + impl Connection { /// Create a new TCP connection using the `git` protocol of `desired_version`, and make a connection to `host` /// at `port` for accessing the repository at `path` on the server side. /// If `trace` is `true`, all packetlines received or sent will be passed to the facilities of the `gix-trace` crate. @@ -176,20 +178,20 @@ mod async_net { path: bstr::BString, desired_version: crate::Protocol, trace: bool, - ) -> Result, Error> { + ) -> Result { let read = async_std::io::timeout( Duration::from_secs(5), TcpStream::connect(&(host, port.unwrap_or(9418))), ) .await?; let write = read.clone(); - Ok(git::Connection::new( + Ok(Self::new( read, write, desired_version, path, None::<(String, _)>, - git::ConnectMode::Daemon, + ConnectMode::Daemon, trace, )) } diff --git a/gix-transport/src/client/git/blocking_io.rs b/gix-transport/src/client/git/blocking_io.rs index 22ae7318455..afa1f3c5051 100644 --- a/gix-transport/src/client/git/blocking_io.rs +++ b/gix-transport/src/client/git/blocking_io.rs @@ -6,9 +6,8 @@ use crate::{ client::{ self, blocking_io::{RequestWriter, SetServiceResponse}, - capabilities, + capabilities::blocking_recv::Outcome, git::{self, ConnectionState}, - Capabilities, }, packetline::{ blocking_io::{StreamingPeekableIter, Writer}, @@ -92,11 +91,11 @@ where line_writer.flush()?; } - let capabilities::recv::Outcome { + let Outcome { capabilities, refs, protocol: actual_protocol, - } = Capabilities::from_lines_with_version_detection(&mut self.line_provider)?; + } = Outcome::from_lines_with_version_detection(&mut self.line_provider)?; Ok(SetServiceResponse { actual_protocol, capabilities, diff --git a/gix-transport/src/client/git/mod.rs b/gix-transport/src/client/git/mod.rs index 37d6193f5d5..02179060ca4 100644 --- a/gix-transport/src/client/git/mod.rs +++ b/gix-transport/src/client/git/mod.rs @@ -161,12 +161,10 @@ mod message { } } -#[cfg(all(feature = "async-client", not(feature = "blocking-client")))] -pub(crate) mod async_io; -#[cfg(all(feature = "async-client", not(feature = "blocking-client")))] -pub use async_io::Connection; +/// +#[cfg(feature = "async-client")] +pub mod async_io; +/// #[cfg(feature = "blocking-client")] -pub(crate) mod blocking_io; -#[cfg(feature = "blocking-client")] -pub use blocking_io::{connect, Connection}; +pub mod blocking_io; diff --git a/gix-transport/src/client/mod.rs b/gix-transport/src/client/mod.rs index 4b8aee804cb..038030a2855 100644 --- a/gix-transport/src/client/mod.rs +++ b/gix-transport/src/client/mod.rs @@ -1,5 +1,5 @@ /// -#[cfg(all(feature = "async-client", not(feature = "blocking-client")))] +#[cfg(feature = "async-client")] pub mod async_io; mod traits; diff --git a/gix-transport/tests/client/capabilities.rs b/gix-transport/tests/client/capabilities.rs index cd7d9405e00..1792de659d9 100644 --- a/gix-transport/tests/client/capabilities.rs +++ b/gix-transport/tests/client/capabilities.rs @@ -3,6 +3,10 @@ use bstr::ByteSlice; use gix_packetline::async_io::{encode, StreamingPeekableIter}; #[cfg(all(feature = "blocking-client", not(feature = "async-client")))] use gix_packetline::blocking_io::{encode, StreamingPeekableIter}; +#[cfg(all(feature = "async-client", not(feature = "blocking-client")))] +use gix_transport::client::capabilities::async_recv::Outcome; +#[cfg(all(feature = "blocking-client", not(feature = "async-client")))] +use gix_transport::client::capabilities::blocking_recv::Outcome; use gix_transport::client::Capabilities; #[test] @@ -59,7 +63,7 @@ async fn from_lines_with_version_detection_v0() -> crate::Result { let mut buf = Vec::::new(); encode::flush_to_write(&mut buf).await?; let mut stream = StreamingPeekableIter::new(buf.as_slice(), &[gix_packetline::PacketLineRef::Flush], false); - let caps = Capabilities::from_lines_with_version_detection(&mut stream) + let caps = Outcome::from_lines_with_version_detection(&mut stream) .await .expect("we can parse V0 as very special case, useful for testing stateful connections in other crates") .capabilities; diff --git a/gix-transport/tests/client/git.rs b/gix-transport/tests/client/git.rs index 88c9699aebc..9afee70146c 100644 --- a/gix-transport/tests/client/git.rs +++ b/gix-transport/tests/client/git.rs @@ -10,9 +10,15 @@ use bstr::ByteSlice; use futures_lite::{AsyncBufReadExt, AsyncWriteExt, StreamExt}; use gix_packetline::read::ProgressAction; #[cfg(feature = "async-client")] -use gix_transport::client::async_io::{Transport, TransportV2Ext}; +use gix_transport::client::{ + async_io::{Transport, TransportV2Ext}, + git::async_io::Connection, +}; #[cfg(feature = "blocking-client")] -use gix_transport::client::blocking_io::{Transport, TransportV2Ext}; +use gix_transport::client::{ + blocking_io::{Transport, TransportV2Ext}, + git::blocking_io::Connection, +}; use gix_transport::{ client, client::{git, TransportWithoutIO}, @@ -25,7 +31,7 @@ use crate::fixture_bytes; async fn handshake_v1_and_request() -> crate::Result { let mut out = Vec::new(); let server_response = fixture_bytes("v1/clone.response"); - let c = git::Connection::new( + let c = Connection::new( server_response.as_slice(), &mut out, Protocol::V1, @@ -157,7 +163,7 @@ async fn handshake_v1_and_request() -> crate::Result { async fn push_v1_simulated() -> crate::Result { let mut out = Vec::new(); let server_response = fixture_bytes("v1/push.response"); - let mut c = git::Connection::new( + let mut c = Connection::new( server_response.as_slice(), &mut out, Protocol::V1, @@ -223,7 +229,7 @@ async fn push_v1_simulated() -> crate::Result { async fn handshake_v1_process_mode() -> crate::Result { let mut out = Vec::new(); let server_response = fixture_bytes("v1/clone.response"); - let mut c = git::Connection::new( + let mut c = Connection::new( server_response.as_slice(), &mut out, Protocol::V1, @@ -246,7 +252,7 @@ async fn handshake_v1_process_mode() -> crate::Result { async fn handshake_v2_downgrade_to_v1() -> crate::Result { let mut out = Vec::new(); let input = fixture_bytes("v1/clone.response"); - let mut c = git::Connection::new( + let mut c = Connection::new( input.as_slice(), &mut out, Protocol::V2, @@ -293,7 +299,7 @@ async fn handshake_v2_and_request() -> crate::Result { async fn handshake_v2_and_request_inner() -> crate::Result { let mut out = Vec::new(); let input = fixture_bytes("v2/clone.response"); - let mut c = git::Connection::new( + let mut c = Connection::new( input.as_slice(), &mut out, Protocol::V2, From f7700e452c79ff5582626bb3377d0429a921d869 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Oct 2025 07:21:36 +0200 Subject: [PATCH 04/36] fix: refspec for shallow clones uses a single-branch (#2227) When doing shallow clones (depth != NoChange), it now uses a single-branch refspec instead of fetching all branches. This matches Git's behavior and significantly reduces the repository size for shallow clones. For shallow clones: - If ref_name is specified: uses that branch - Otherwise: attempts to detect from Protocol V1 handshake or falls back to init.defaultBranch config or "main" This addresses issue #2227 where `gix clone --depth 1` was creating repositories ~130MB vs Git's ~70MB due to fetching all branches. Co-authored-by: Byron <63622+Byron@users.noreply.github.com> --- gix/src/clone/fetch/mod.rs | 82 +++++++++++++++++++++++++++++++++++--- gix/tests/gix/clone.rs | 35 ++++++++++++++++ 2 files changed, 111 insertions(+), 6 deletions(-) diff --git a/gix/src/clone/fetch/mod.rs b/gix/src/clone/fetch/mod.rs index 34641a89fb1..c56f413061b 100644 --- a/gix/src/clone/fetch/mod.rs +++ b/gix/src/clone/fetch/mod.rs @@ -47,6 +47,8 @@ pub enum Error { }, #[error(transparent)] CommitterOrFallback(#[from] crate::config::time::Error), + #[error(transparent)] + RefMap(#[from] crate::remote::ref_map::Error), } /// Modification @@ -101,14 +103,81 @@ impl PrepareFetch { }; let mut remote = repo.remote_at(self.url.clone())?; + + // For shallow clones without custom configuration, we'll use a single-branch refspec + // to match git's behavior (matching git's single-branch behavior for shallow clones). + let use_single_branch_for_shallow = self.shallow != remote::fetch::Shallow::NoChange + && self.configure_remote.is_none() + && remote.fetch_specs.is_empty(); + + let target_ref = if use_single_branch_for_shallow { + // Determine target branch from user-specified ref_name or default branch + if let Some(ref_name) = &self.ref_name { + // User specified a branch, use that + Some(format!("refs/heads/{}", ref_name.as_ref().as_bstr())) + } else { + // For shallow clones without a specified ref, we need to determine the default branch. + // We'll connect to get HEAD information. For Protocol V2, we need to explicitly list refs. + let mut connection = remote.connect(remote::Direction::Fetch).await?; + + // Perform handshake and try to get HEAD from it (works for Protocol V1) + let _ = connection.ref_map_by_ref(&mut progress, Default::default()).await?; + + let target = if let Some(handshake) = &connection.handshake { + // Protocol V1: refs are in handshake + handshake.refs.as_ref().and_then(|refs| { + refs.iter().find_map(|r| match r { + gix_protocol::handshake::Ref::Symbolic { + full_ref_name, target, .. + } if full_ref_name == "HEAD" => Some(target.to_string()), + _ => None, + }) + }) + } else { + None + }; + + // For Protocol V2 or if we couldn't determine HEAD, use the configured default branch + let fallback_branch = target + .or_else(|| { + repo.config + .resolved + .string(crate::config::tree::Init::DEFAULT_BRANCH) + .and_then(|name| name.to_str().ok().map(|s| format!("refs/heads/{}", s))) + }) + .unwrap_or_else(|| "refs/heads/main".to_string()); + + // Drop the connection explicitly to release the borrow on remote + drop(connection); + + Some(fallback_branch) + } + } else { + None + }; + + // Set up refspec based on whether we're doing a single-branch shallow clone if remote.fetch_specs.is_empty() { - remote = remote - .with_refspecs( - Some(format!("+refs/heads/*:refs/remotes/{remote_name}/*").as_str()), - remote::Direction::Fetch, - ) - .expect("valid static spec"); + if let Some(target_ref) = &target_ref { + // Single-branch refspec for shallow clones + let short_name = target_ref.strip_prefix("refs/heads/").unwrap_or(target_ref.as_str()); + remote = remote + .with_refspecs( + Some(format!("+{target_ref}:refs/remotes/{remote_name}/{short_name}").as_str()), + remote::Direction::Fetch, + ) + .expect("valid refspec"); + } else { + // Wildcard refspec for non-shallow clones or when target couldn't be determined + remote = remote + .with_refspecs( + Some(format!("+refs/heads/*:refs/remotes/{remote_name}/*").as_str()), + remote::Direction::Fetch, + ) + .expect("valid static spec"); + } } + let mut clone_fetch_tags = None; if let Some(f) = self.configure_remote.as_mut() { remote = f(remote).map_err(Error::RemoteConfiguration)?; @@ -133,6 +202,7 @@ impl PrepareFetch { .expect("valid") .to_owned(); let pending_pack: remote::fetch::Prepare<'_, '_, _> = { + // For shallow clones, we already connected once, so we need to connect again let mut connection = remote.connect(remote::Direction::Fetch).await?; if let Some(f) = self.configure_connection.as_mut() { f(&mut connection).map_err(Error::RemoteConnection)?; diff --git a/gix/tests/gix/clone.rs b/gix/tests/gix/clone.rs index 573a8e3ddfe..3624afb7e55 100644 --- a/gix/tests/gix/clone.rs +++ b/gix/tests/gix/clone.rs @@ -83,6 +83,41 @@ mod blocking_io { Ok(()) } + #[test] + fn shallow_clone_uses_single_branch_refspec() -> crate::Result { + let tmp = gix_testtools::tempfile::TempDir::new()?; + let (repo, _out) = gix::prepare_clone_bare(remote::repo("base").path(), tmp.path())? + .with_shallow(Shallow::DepthAtRemote(1.try_into()?)) + .fetch_only(gix::progress::Discard, &std::sync::atomic::AtomicBool::default())?; + + assert!(repo.is_shallow(), "repository should be shallow"); + + // Verify that only a single-branch refspec was configured + let remote = repo.find_remote("origin")?; + let refspecs: Vec<_> = remote + .refspecs(Direction::Fetch) + .iter() + .map(|spec| spec.to_ref().to_bstring()) + .collect(); + + assert_eq!(refspecs.len(), 1, "shallow clone should have only one fetch refspec"); + + // The refspec should be for a single branch (main), not a wildcard + let refspec_str = refspecs[0].to_str().expect("valid utf8"); + assert!( + !refspec_str.contains("*"), + "shallow clone refspec should not use wildcard: {}", + refspec_str + ); + assert!( + refspec_str.contains("refs/heads/main"), + "shallow clone refspec should reference the main branch: {}", + refspec_str + ); + + Ok(()) + } + #[test] fn from_shallow_prohibited_with_option() -> crate::Result { let tmp = gix_testtools::tempfile::TempDir::new()?; From d2cd2effb00bc15c80fc36870c943d4d2e236c41 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 2 Nov 2025 14:11:42 +0100 Subject: [PATCH 05/36] feat: `Category::to_full_name()` can now be passed a full name without prefix duplication. This is for convenience, as it can then be passed a valid partial name. --- gix-ref/src/fullname.rs | 13 +++++++++---- gix-ref/tests/refs/fullname.rs | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/gix-ref/src/fullname.rs b/gix-ref/src/fullname.rs index 423baf5a26b..40cf7bc8661 100644 --- a/gix-ref/src/fullname.rs +++ b/gix-ref/src/fullname.rs @@ -1,8 +1,7 @@ +use crate::{bstr::ByteVec, name::is_pseudo_ref, Category, FullName, FullNameRef, Namespace, PartialNameRef}; use gix_object::bstr::{BStr, BString, ByteSlice}; use std::{borrow::Borrow, path::Path}; -use crate::{bstr::ByteVec, name::is_pseudo_ref, Category, FullName, FullNameRef, Namespace, PartialNameRef}; - impl TryFrom<&str> for FullName { type Error = gix_validate::reference::name::Error; @@ -165,6 +164,8 @@ impl FullNameRef { impl Category<'_> { /// As the inverse of [`FullNameRef::category_and_short_name()`], use the prefix of this category alongside /// the `short_name` to create a valid fully qualified [reference name](FullName). + /// + /// If `short_name` already contains the prefix that it would receive (and is thus a full name), no duplication will occur. pub fn to_full_name<'a>(&self, short_name: impl Into<&'a BStr>) -> Result { let mut out: BString = self.prefix().into(); let short_name = short_name.into(); @@ -185,8 +186,12 @@ impl Category<'_> { | Category::PseudoRef | Category::MainPseudoRef => short_name, }; - out.extend_from_slice(partial_name); - FullName::try_from(out) + if out.is_empty() || !partial_name.starts_with(&out) { + out.extend_from_slice(partial_name); + FullName::try_from(out) + } else { + FullName::try_from(partial_name.as_bstr()) + } } } diff --git a/gix-ref/tests/refs/fullname.rs b/gix-ref/tests/refs/fullname.rs index 3208b19dde6..ebc7f8fe01b 100644 --- a/gix-ref/tests/refs/fullname.rs +++ b/gix-ref/tests/refs/fullname.rs @@ -117,6 +117,25 @@ fn shorten_and_category() { ); } +#[test] +fn to_full_name() -> gix_testtools::Result { + assert_eq!( + Category::LocalBranch.to_full_name("refs/heads/full")?.as_bstr(), + "refs/heads/full", + "prefixes aren't duplicated" + ); + + assert_eq!( + Category::LocalBranch + .to_full_name("refs/remotes/origin/other")? + .as_bstr(), + "refs/heads/refs/remotes/origin/other", + "full names with a different category will be prefixed, to support 'main-worktree' special cases" + ); + + Ok(()) +} + #[test] fn prefix_with_namespace_and_stripping() { let ns = gix_ref::namespace::expand("foo").unwrap(); From d79f896f48449af90efcc84da58418194cb31bff Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 2 Nov 2025 15:06:54 +0100 Subject: [PATCH 06/36] Allow complex glob patterns in one-sided refspecs - Modified validated() function to skip strict ref name validation for one-sided refspecs with globs - Allow multiple asterisks in one-sided refspecs (no destination) - Two-sided refspecs still require balanced patterns and reject multiple asterisks - Added tests for complex glob pattern parsing and simple glob matching - Updated existing tests to reflect the new behavior Co-authored-by: Byron <63622+Byron@users.noreply.github.com> --- gix-refspec/src/parse.rs | 31 ++++--- gix-refspec/tests/refspec/match_group.rs | 99 ++++++++++++++++++++++ gix-refspec/tests/refspec/parse/fetch.rs | 41 +++++++++ gix-refspec/tests/refspec/parse/invalid.rs | 20 ++++- gix-refspec/tests/refspec/parse/mod.rs | 9 ++ 5 files changed, 186 insertions(+), 14 deletions(-) diff --git a/gix-refspec/src/parse.rs b/gix-refspec/src/parse.rs index 398d93294ad..772c2aa975b 100644 --- a/gix-refspec/src/parse.rs +++ b/gix-refspec/src/parse.rs @@ -116,9 +116,11 @@ pub(crate) mod function { *spec = "HEAD".into(); } } - let (src, src_had_pattern) = validated(src, operation == Operation::Push && dst.is_some())?; - let (dst, dst_had_pattern) = validated(dst, false)?; - if mode != Mode::Negative && src_had_pattern != dst_had_pattern { + let is_one_sided = dst.is_none(); + let (src, src_had_pattern) = validated(src, operation == Operation::Push && dst.is_some(), is_one_sided)?; + let (dst, dst_had_pattern) = validated(dst, false, false)?; + // For one-sided refspecs, we don't need to check for pattern balance + if !is_one_sided && mode != Mode::Negative && src_had_pattern != dst_had_pattern { return Err(Error::PatternUnbalanced); } @@ -149,20 +151,27 @@ pub(crate) mod function { spec.len() >= gix_hash::Kind::shortest().len_in_hex() && spec.iter().all(u8::is_ascii_hexdigit) } - fn validated(spec: Option<&BStr>, allow_revspecs: bool) -> Result<(Option<&BStr>, bool), Error> { + fn validated(spec: Option<&BStr>, allow_revspecs: bool, is_one_sided: bool) -> Result<(Option<&BStr>, bool), Error> { match spec { Some(spec) => { let glob_count = spec.iter().filter(|b| **b == b'*').take(2).count(); if glob_count > 1 { - return Err(Error::PatternUnsupported { pattern: spec.into() }); + // For one-sided refspecs, allow any number of globs without validation + if !is_one_sided { + return Err(Error::PatternUnsupported { pattern: spec.into() }); + } } - let has_globs = glob_count == 1; + // Check if there are any globs (one or more asterisks) + let has_globs = glob_count > 0; if has_globs { - let mut buf = smallvec::SmallVec::<[u8; 256]>::with_capacity(spec.len()); - buf.extend_from_slice(spec); - let glob_pos = buf.find_byte(b'*').expect("glob present"); - buf[glob_pos] = b'a'; - gix_validate::reference::name_partial(buf.as_bstr())?; + // For one-sided refspecs, skip validation of glob patterns + if !is_one_sided { + let mut buf = smallvec::SmallVec::<[u8; 256]>::with_capacity(spec.len()); + buf.extend_from_slice(spec); + let glob_pos = buf.find_byte(b'*').expect("glob present"); + buf[glob_pos] = b'a'; + gix_validate::reference::name_partial(buf.as_bstr())?; + } } else { gix_validate::reference::name_partial(spec) .map_err(Error::from) diff --git a/gix-refspec/tests/refspec/match_group.rs b/gix-refspec/tests/refspec/match_group.rs index 42cece6799f..6ed285ece4c 100644 --- a/gix-refspec/tests/refspec/match_group.rs +++ b/gix-refspec/tests/refspec/match_group.rs @@ -184,3 +184,102 @@ mod multiple { ); } } + +mod complex_globs { + use gix_refspec::{parse::Operation, MatchGroup}; + use gix_hash::ObjectId; + use bstr::BString; + use std::borrow::Cow; + + #[test] + fn one_sided_complex_glob_patterns_can_be_parsed() { + // The key change is that complex glob patterns with multiple asterisks + // can now be parsed for one-sided refspecs + let spec1 = gix_refspec::parse("refs/*/foo/*".into(), Operation::Fetch); + assert!(spec1.is_ok(), "Should parse complex glob pattern for one-sided refspec"); + + let spec2 = gix_refspec::parse("refs/*/*/bar".into(), Operation::Fetch); + assert!(spec2.is_ok(), "Should parse complex glob pattern with multiple asterisks"); + + let spec3 = gix_refspec::parse("refs/heads/*/release/*".into(), Operation::Fetch); + assert!(spec3.is_ok(), "Should parse complex glob pattern"); + + // Two-sided refspecs with multiple asterisks should still fail + let spec4 = gix_refspec::parse("refs/*/foo/*:refs/remotes/*".into(), Operation::Fetch); + assert!(spec4.is_err(), "Two-sided refspecs with multiple asterisks should fail"); + } + + #[test] + fn one_sided_simple_glob_patterns_match() { + // Test that simple glob patterns (one asterisk) work correctly with matching + let refs = vec![ + create_ref("refs/heads/feature/foo", "1111111111111111111111111111111111111111"), + create_ref("refs/heads/bugfix/bar", "2222222222222222222222222222222222222222"), + create_ref("refs/tags/v1.0", "3333333333333333333333333333333333333333"), + create_ref("refs/pull/123", "4444444444444444444444444444444444444444"), + ]; + let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); + + // Test: refs/heads/* should match all refs under refs/heads/ + let spec = gix_refspec::parse("refs/heads/*".into(), Operation::Fetch).unwrap(); + let group = MatchGroup::from_fetch_specs([spec]); + let outcome = group.match_lhs(items.iter().copied()); + let mappings = outcome.mappings; + + assert_eq!(mappings.len(), 2, "Should match two refs under refs/heads/"); + + // Test: refs/tags/* should match all refs under refs/tags/ + let items2: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); + let spec2 = gix_refspec::parse("refs/tags/*".into(), Operation::Fetch).unwrap(); + let group2 = MatchGroup::from_fetch_specs([spec2]); + let outcome2 = group2.match_lhs(items2.iter().copied()); + let mappings2 = outcome2.mappings; + + assert_eq!(mappings2.len(), 1, "Should match one ref under refs/tags/"); + } + + #[test] + fn one_sided_glob_with_suffix_matches() { + // Test that glob patterns with suffix work correctly + let refs = vec![ + create_ref("refs/heads/feature", "1111111111111111111111111111111111111111"), + create_ref("refs/heads/feat", "2222222222222222222222222222222222222222"), + create_ref("refs/heads/main", "3333333333333333333333333333333333333333"), + ]; + let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); + + // Test: refs/heads/feat* should match refs/heads/feature and refs/heads/feat + let spec = gix_refspec::parse("refs/heads/feat*".into(), Operation::Fetch).unwrap(); + let group = MatchGroup::from_fetch_specs([spec]); + let outcome = group.match_lhs(items.iter().copied()); + let mappings = outcome.mappings; + + assert_eq!(mappings.len(), 2, "Should match two refs starting with feat"); + } + + // Helper function to create a ref + fn create_ref(name: &str, id_hex: &str) -> Ref { + Ref { + name: name.into(), + target: ObjectId::from_hex(id_hex.as_bytes()).unwrap(), + object: None, + } + } + + #[derive(Debug, Clone)] + struct Ref { + name: BString, + target: ObjectId, + object: Option, + } + + impl Ref { + fn to_item(&self) -> gix_refspec::match_group::Item<'_> { + gix_refspec::match_group::Item { + full_ref_name: self.name.as_ref(), + target: &self.target, + object: self.object.as_deref(), + } + } + } +} diff --git a/gix-refspec/tests/refspec/parse/fetch.rs b/gix-refspec/tests/refspec/parse/fetch.rs index c10a47801b2..7df32570392 100644 --- a/gix-refspec/tests/refspec/parse/fetch.rs +++ b/gix-refspec/tests/refspec/parse/fetch.rs @@ -174,3 +174,44 @@ fn ampersand_on_left_hand_side_is_head() { fn empty_refspec_is_enough_for_fetching_head_into_fetchhead() { assert_parse("", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); } + +#[test] +fn complex_glob_patterns_are_allowed_in_one_sided_refspecs() { + // Complex patterns with multiple asterisks should work for one-sided refspecs + assert_parse( + "refs/*/foo/*", + Instruction::Fetch(Fetch::Only { + src: b("refs/*/foo/*"), + }), + ); + + assert_parse( + "+refs/heads/*/release/*", + Instruction::Fetch(Fetch::Only { + src: b("refs/heads/*/release/*"), + }), + ); + + // Even more complex patterns + assert_parse( + "refs/*/*/branch", + Instruction::Fetch(Fetch::Only { + src: b("refs/*/*/branch"), + }), + ); +} + +#[test] +fn complex_glob_patterns_still_fail_for_two_sided_refspecs() { + // Two-sided refspecs with complex patterns (multiple asterisks) should still fail + for spec in [ + "refs/*/foo/*:refs/remotes/origin/*", + "refs/*/*:refs/remotes/*", + "a/*/c/*:b/*", + ] { + assert!(matches!( + try_parse(spec, Operation::Fetch).unwrap_err(), + Error::PatternUnsupported { .. } + )); + } +} diff --git a/gix-refspec/tests/refspec/parse/invalid.rs b/gix-refspec/tests/refspec/parse/invalid.rs index db591d2e0b3..6bb165498df 100644 --- a/gix-refspec/tests/refspec/parse/invalid.rs +++ b/gix-refspec/tests/refspec/parse/invalid.rs @@ -27,24 +27,33 @@ fn whitespace() { #[test] fn complex_patterns_with_more_than_one_asterisk() { + // For one-sided refspecs, complex patterns are now allowed for op in [Operation::Fetch, Operation::Push] { - for spec in ["a/*/c/*", "a**:**b", "+:**/"] { + assert!(try_parse("a/*/c/*", op).is_ok()); + } + + // For two-sided refspecs, complex patterns should still fail + for op in [Operation::Fetch, Operation::Push] { + for spec in ["a/*/c/*:x/*/y/*", "a**:**b", "+:**/"] { assert!(matches!( try_parse(spec, op).unwrap_err(), Error::PatternUnsupported { .. } )); } } + + // Negative specs with multiple patterns still fail assert!(matches!( try_parse("^*/*", Operation::Fetch).unwrap_err(), - Error::PatternUnsupported { .. } + Error::NegativeGlobPattern )); } #[test] fn both_sides_need_pattern_if_one_uses_it() { + // For two-sided refspecs, both sides still need patterns if one uses it for op in [Operation::Fetch, Operation::Push] { - for spec in ["refs/*/a", ":a/*", "+:a/*", "a*:b/c", "a:b/*"] { + for spec in [":a/*", "+:a/*", "a*:b/c", "a:b/*"] { assert!( matches!(try_parse(spec, op).unwrap_err(), Error::PatternUnbalanced), "{}", @@ -52,6 +61,11 @@ fn both_sides_need_pattern_if_one_uses_it() { ); } } + + // One-sided refspecs with patterns are now allowed + for op in [Operation::Fetch, Operation::Push] { + assert!(try_parse("refs/*/a", op).is_ok()); + } } #[test] diff --git a/gix-refspec/tests/refspec/parse/mod.rs b/gix-refspec/tests/refspec/parse/mod.rs index 0e0cf77d56f..7662f04bd29 100644 --- a/gix-refspec/tests/refspec/parse/mod.rs +++ b/gix-refspec/tests/refspec/parse/mod.rs @@ -45,6 +45,10 @@ fn baseline() { ), true, ) => {} // we prefer failing fast, git let's it pass + // We now allow complex glob patterns in one-sided refspecs + (None, false) if is_one_sided_glob_pattern(spec, op) => { + // This is an intentional behavior change: we allow complex globs in one-sided refspecs + } _ => { eprintln!("{err_code} {res:?} {} {:?}", kind.as_bstr(), spec.as_bstr()); mismatch += 1; @@ -66,6 +70,11 @@ fn baseline() { panics ); } + + fn is_one_sided_glob_pattern(spec: &[u8], op: Operation) -> bool { + use bstr::ByteSlice; + matches!(op, Operation::Fetch) && spec.to_str().map(|s| s.contains('*') && !s.contains(':')).unwrap_or(false) + } } #[test] From bb84248d3515565b936f2cfb71a4bc79e4563a62 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 2 Nov 2025 15:07:32 +0100 Subject: [PATCH 07/36] cargo fmt --- gix-refspec/src/parse.rs | 6 +++- gix-refspec/tests/refspec/match_group.rs | 37 ++++++++++++---------- gix-refspec/tests/refspec/parse/fetch.rs | 8 ++--- gix-refspec/tests/refspec/parse/invalid.rs | 6 ++-- gix-refspec/tests/refspec/parse/mod.rs | 8 +++-- 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/gix-refspec/src/parse.rs b/gix-refspec/src/parse.rs index 772c2aa975b..6844d2f7ecb 100644 --- a/gix-refspec/src/parse.rs +++ b/gix-refspec/src/parse.rs @@ -151,7 +151,11 @@ pub(crate) mod function { spec.len() >= gix_hash::Kind::shortest().len_in_hex() && spec.iter().all(u8::is_ascii_hexdigit) } - fn validated(spec: Option<&BStr>, allow_revspecs: bool, is_one_sided: bool) -> Result<(Option<&BStr>, bool), Error> { + fn validated( + spec: Option<&BStr>, + allow_revspecs: bool, + is_one_sided: bool, + ) -> Result<(Option<&BStr>, bool), Error> { match spec { Some(spec) => { let glob_count = spec.iter().filter(|b| **b == b'*').take(2).count(); diff --git a/gix-refspec/tests/refspec/match_group.rs b/gix-refspec/tests/refspec/match_group.rs index 6ed285ece4c..d6a46ec3aac 100644 --- a/gix-refspec/tests/refspec/match_group.rs +++ b/gix-refspec/tests/refspec/match_group.rs @@ -186,9 +186,9 @@ mod multiple { } mod complex_globs { - use gix_refspec::{parse::Operation, MatchGroup}; - use gix_hash::ObjectId; use bstr::BString; + use gix_hash::ObjectId; + use gix_refspec::{parse::Operation, MatchGroup}; use std::borrow::Cow; #[test] @@ -197,18 +197,21 @@ mod complex_globs { // can now be parsed for one-sided refspecs let spec1 = gix_refspec::parse("refs/*/foo/*".into(), Operation::Fetch); assert!(spec1.is_ok(), "Should parse complex glob pattern for one-sided refspec"); - + let spec2 = gix_refspec::parse("refs/*/*/bar".into(), Operation::Fetch); - assert!(spec2.is_ok(), "Should parse complex glob pattern with multiple asterisks"); - + assert!( + spec2.is_ok(), + "Should parse complex glob pattern with multiple asterisks" + ); + let spec3 = gix_refspec::parse("refs/heads/*/release/*".into(), Operation::Fetch); assert!(spec3.is_ok(), "Should parse complex glob pattern"); - + // Two-sided refspecs with multiple asterisks should still fail let spec4 = gix_refspec::parse("refs/*/foo/*:refs/remotes/*".into(), Operation::Fetch); assert!(spec4.is_err(), "Two-sided refspecs with multiple asterisks should fail"); } - + #[test] fn one_sided_simple_glob_patterns_match() { // Test that simple glob patterns (one asterisk) work correctly with matching @@ -219,25 +222,25 @@ mod complex_globs { create_ref("refs/pull/123", "4444444444444444444444444444444444444444"), ]; let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); - + // Test: refs/heads/* should match all refs under refs/heads/ let spec = gix_refspec::parse("refs/heads/*".into(), Operation::Fetch).unwrap(); let group = MatchGroup::from_fetch_specs([spec]); let outcome = group.match_lhs(items.iter().copied()); let mappings = outcome.mappings; - + assert_eq!(mappings.len(), 2, "Should match two refs under refs/heads/"); - + // Test: refs/tags/* should match all refs under refs/tags/ let items2: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); let spec2 = gix_refspec::parse("refs/tags/*".into(), Operation::Fetch).unwrap(); let group2 = MatchGroup::from_fetch_specs([spec2]); let outcome2 = group2.match_lhs(items2.iter().copied()); let mappings2 = outcome2.mappings; - + assert_eq!(mappings2.len(), 1, "Should match one ref under refs/tags/"); } - + #[test] fn one_sided_glob_with_suffix_matches() { // Test that glob patterns with suffix work correctly @@ -247,16 +250,16 @@ mod complex_globs { create_ref("refs/heads/main", "3333333333333333333333333333333333333333"), ]; let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); - + // Test: refs/heads/feat* should match refs/heads/feature and refs/heads/feat let spec = gix_refspec::parse("refs/heads/feat*".into(), Operation::Fetch).unwrap(); let group = MatchGroup::from_fetch_specs([spec]); let outcome = group.match_lhs(items.iter().copied()); let mappings = outcome.mappings; - + assert_eq!(mappings.len(), 2, "Should match two refs starting with feat"); } - + // Helper function to create a ref fn create_ref(name: &str, id_hex: &str) -> Ref { Ref { @@ -265,14 +268,14 @@ mod complex_globs { object: None, } } - + #[derive(Debug, Clone)] struct Ref { name: BString, target: ObjectId, object: Option, } - + impl Ref { fn to_item(&self) -> gix_refspec::match_group::Item<'_> { gix_refspec::match_group::Item { diff --git a/gix-refspec/tests/refspec/parse/fetch.rs b/gix-refspec/tests/refspec/parse/fetch.rs index 7df32570392..c3e500d780a 100644 --- a/gix-refspec/tests/refspec/parse/fetch.rs +++ b/gix-refspec/tests/refspec/parse/fetch.rs @@ -180,18 +180,16 @@ fn complex_glob_patterns_are_allowed_in_one_sided_refspecs() { // Complex patterns with multiple asterisks should work for one-sided refspecs assert_parse( "refs/*/foo/*", - Instruction::Fetch(Fetch::Only { - src: b("refs/*/foo/*"), - }), + Instruction::Fetch(Fetch::Only { src: b("refs/*/foo/*") }), ); - + assert_parse( "+refs/heads/*/release/*", Instruction::Fetch(Fetch::Only { src: b("refs/heads/*/release/*"), }), ); - + // Even more complex patterns assert_parse( "refs/*/*/branch", diff --git a/gix-refspec/tests/refspec/parse/invalid.rs b/gix-refspec/tests/refspec/parse/invalid.rs index 6bb165498df..54902422378 100644 --- a/gix-refspec/tests/refspec/parse/invalid.rs +++ b/gix-refspec/tests/refspec/parse/invalid.rs @@ -31,7 +31,7 @@ fn complex_patterns_with_more_than_one_asterisk() { for op in [Operation::Fetch, Operation::Push] { assert!(try_parse("a/*/c/*", op).is_ok()); } - + // For two-sided refspecs, complex patterns should still fail for op in [Operation::Fetch, Operation::Push] { for spec in ["a/*/c/*:x/*/y/*", "a**:**b", "+:**/"] { @@ -41,7 +41,7 @@ fn complex_patterns_with_more_than_one_asterisk() { )); } } - + // Negative specs with multiple patterns still fail assert!(matches!( try_parse("^*/*", Operation::Fetch).unwrap_err(), @@ -61,7 +61,7 @@ fn both_sides_need_pattern_if_one_uses_it() { ); } } - + // One-sided refspecs with patterns are now allowed for op in [Operation::Fetch, Operation::Push] { assert!(try_parse("refs/*/a", op).is_ok()); diff --git a/gix-refspec/tests/refspec/parse/mod.rs b/gix-refspec/tests/refspec/parse/mod.rs index 7662f04bd29..61a4a2803a4 100644 --- a/gix-refspec/tests/refspec/parse/mod.rs +++ b/gix-refspec/tests/refspec/parse/mod.rs @@ -70,10 +70,14 @@ fn baseline() { panics ); } - + fn is_one_sided_glob_pattern(spec: &[u8], op: Operation) -> bool { use bstr::ByteSlice; - matches!(op, Operation::Fetch) && spec.to_str().map(|s| s.contains('*') && !s.contains(':')).unwrap_or(false) + matches!(op, Operation::Fetch) + && spec + .to_str() + .map(|s| s.contains('*') && !s.contains(':')) + .unwrap_or(false) } } From 0d5372d30bf58583419b8467ed5f3e2c5a6f16c7 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 2 Nov 2025 15:12:50 +0100 Subject: [PATCH 08/36] refactor --- Cargo.lock | 1 + gix-refspec/Cargo.toml | 1 + gix-refspec/tests/refspec/match_group.rs | 109 +++++++++++++++++------ 3 files changed, 84 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 82468ee5370..c96a4ad14a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2256,6 +2256,7 @@ dependencies = [ "gix-revision", "gix-testtools", "gix-validate", + "insta", "smallvec", "thiserror 2.0.17", ] diff --git a/gix-refspec/Cargo.toml b/gix-refspec/Cargo.toml index 2c60ce1bd93..7e187c57477 100644 --- a/gix-refspec/Cargo.toml +++ b/gix-refspec/Cargo.toml @@ -25,3 +25,4 @@ smallvec = "1.15.1" [dev-dependencies] gix-testtools = { path = "../tests/tools" } +insta = "1.43.2" \ No newline at end of file diff --git a/gix-refspec/tests/refspec/match_group.rs b/gix-refspec/tests/refspec/match_group.rs index d6a46ec3aac..981130ffbb3 100644 --- a/gix-refspec/tests/refspec/match_group.rs +++ b/gix-refspec/tests/refspec/match_group.rs @@ -189,37 +189,36 @@ mod complex_globs { use bstr::BString; use gix_hash::ObjectId; use gix_refspec::{parse::Operation, MatchGroup}; - use std::borrow::Cow; #[test] fn one_sided_complex_glob_patterns_can_be_parsed() { // The key change is that complex glob patterns with multiple asterisks // can now be parsed for one-sided refspecs - let spec1 = gix_refspec::parse("refs/*/foo/*".into(), Operation::Fetch); - assert!(spec1.is_ok(), "Should parse complex glob pattern for one-sided refspec"); + let spec = gix_refspec::parse("refs/*/foo/*".into(), Operation::Fetch); + assert!(spec.is_ok(), "Should parse complex glob pattern for one-sided refspec"); - let spec2 = gix_refspec::parse("refs/*/*/bar".into(), Operation::Fetch); + let spec = gix_refspec::parse("refs/*/*/bar".into(), Operation::Fetch); assert!( - spec2.is_ok(), + spec.is_ok(), "Should parse complex glob pattern with multiple asterisks" ); - let spec3 = gix_refspec::parse("refs/heads/*/release/*".into(), Operation::Fetch); - assert!(spec3.is_ok(), "Should parse complex glob pattern"); + let spec = gix_refspec::parse("refs/heads/[a-z.]/release/*".into(), Operation::Fetch); + assert!(spec.is_ok(), "Should parse complex glob pattern"); // Two-sided refspecs with multiple asterisks should still fail - let spec4 = gix_refspec::parse("refs/*/foo/*:refs/remotes/*".into(), Operation::Fetch); - assert!(spec4.is_err(), "Two-sided refspecs with multiple asterisks should fail"); + let spec = gix_refspec::parse("refs/*/foo/*:refs/remotes/*".into(), Operation::Fetch); + assert!(spec.is_err(), "Two-sided refspecs with multiple asterisks should fail"); } #[test] fn one_sided_simple_glob_patterns_match() { // Test that simple glob patterns (one asterisk) work correctly with matching let refs = vec![ - create_ref("refs/heads/feature/foo", "1111111111111111111111111111111111111111"), - create_ref("refs/heads/bugfix/bar", "2222222222222222222222222222222222222222"), - create_ref("refs/tags/v1.0", "3333333333333333333333333333333333333333"), - create_ref("refs/pull/123", "4444444444444444444444444444444444444444"), + new_ref("refs/heads/feature/foo", "1111111111111111111111111111111111111111"), + new_ref("refs/heads/bugfix/bar", "2222222222222222222222222222222222222222"), + new_ref("refs/tags/v1.0", "3333333333333333333333333333333333333333"), + new_ref("refs/pull/123", "4444444444444444444444444444444444444444"), ]; let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); @@ -227,27 +226,61 @@ mod complex_globs { let spec = gix_refspec::parse("refs/heads/*".into(), Operation::Fetch).unwrap(); let group = MatchGroup::from_fetch_specs([spec]); let outcome = group.match_lhs(items.iter().copied()); - let mappings = outcome.mappings; - assert_eq!(mappings.len(), 2, "Should match two refs under refs/heads/"); + insta::assert_debug_snapshot!(outcome.mappings, @r#" + [ + Mapping { + item_index: Some( + 0, + ), + lhs: FullName( + "refs/heads/feature/foo", + ), + rhs: None, + spec_index: 0, + }, + Mapping { + item_index: Some( + 1, + ), + lhs: FullName( + "refs/heads/bugfix/bar", + ), + rhs: None, + spec_index: 0, + }, + ] + "#); // Test: refs/tags/* should match all refs under refs/tags/ - let items2: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); - let spec2 = gix_refspec::parse("refs/tags/*".into(), Operation::Fetch).unwrap(); - let group2 = MatchGroup::from_fetch_specs([spec2]); - let outcome2 = group2.match_lhs(items2.iter().copied()); - let mappings2 = outcome2.mappings; + let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); + let spec = gix_refspec::parse("refs/tags/v[0.9]*".into(), Operation::Fetch).unwrap(); + let group = MatchGroup::from_fetch_specs([spec]); + let outcome = group.match_lhs(items.iter().copied()); - assert_eq!(mappings2.len(), 1, "Should match one ref under refs/tags/"); + insta::assert_debug_snapshot!(outcome.mappings, @r#" + [ + Mapping { + item_index: Some( + 2, + ), + lhs: FullName( + "refs/tags/v1.0", + ), + rhs: None, + spec_index: 0, + }, + ] + "#); } #[test] fn one_sided_glob_with_suffix_matches() { // Test that glob patterns with suffix work correctly let refs = vec![ - create_ref("refs/heads/feature", "1111111111111111111111111111111111111111"), - create_ref("refs/heads/feat", "2222222222222222222222222222222222222222"), - create_ref("refs/heads/main", "3333333333333333333333333333333333333333"), + new_ref("refs/heads/feature", "1111111111111111111111111111111111111111"), + new_ref("refs/heads/feat", "2222222222222222222222222222222222222222"), + new_ref("refs/heads/main", "3333333333333333333333333333333333333333"), ]; let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); @@ -257,11 +290,33 @@ mod complex_globs { let outcome = group.match_lhs(items.iter().copied()); let mappings = outcome.mappings; - assert_eq!(mappings.len(), 2, "Should match two refs starting with feat"); + insta::assert_debug_snapshot!(mappings, @r#" + [ + Mapping { + item_index: Some( + 0, + ), + lhs: FullName( + "refs/heads/feature", + ), + rhs: None, + spec_index: 0, + }, + Mapping { + item_index: Some( + 1, + ), + lhs: FullName( + "refs/heads/feat", + ), + rhs: None, + spec_index: 0, + }, + ] + "#); } - // Helper function to create a ref - fn create_ref(name: &str, id_hex: &str) -> Ref { + fn new_ref(name: &str, id_hex: &str) -> Ref { Ref { name: name.into(), target: ObjectId::from_hex(id_hex.as_bytes()).unwrap(), From 1bccc54111f9cd888c4f8d42ba6b9b5e3a5e9994 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 2 Nov 2025 15:41:47 +0100 Subject: [PATCH 09/36] Add a way to match refspecs with full globs support --- Cargo.lock | 1 + gix-refspec/Cargo.toml | 1 + gix-refspec/src/match_group/util.rs | 10 +++++++++- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index c96a4ad14a7..b565828fd95 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2252,6 +2252,7 @@ name = "gix-refspec" version = "0.32.0" dependencies = [ "bstr", + "gix-glob", "gix-hash", "gix-revision", "gix-testtools", diff --git a/gix-refspec/Cargo.toml b/gix-refspec/Cargo.toml index 7e187c57477..c7513cc2c50 100644 --- a/gix-refspec/Cargo.toml +++ b/gix-refspec/Cargo.toml @@ -18,6 +18,7 @@ doctest = false gix-revision = { version = "^0.36.0", path = "../gix-revision", default-features = false } gix-validate = { version = "^0.10.1", path = "../gix-validate" } gix-hash = { version = "^0.20.0", path = "../gix-hash" } +gix-glob = { version = "^0.22.1", path = "../gix-glob" } bstr = { version = "1.12.0", default-features = false, features = ["std"] } thiserror = "2.0.17" diff --git a/gix-refspec/src/match_group/util.rs b/gix-refspec/src/match_group/util.rs index c4012717e57..7a55e90621c 100644 --- a/gix-refspec/src/match_group/util.rs +++ b/gix-refspec/src/match_group/util.rs @@ -6,6 +6,7 @@ use gix_hash::ObjectId; use crate::{match_group::Item, RefSpecRef}; /// A type keeping enough information about a ref-spec to be able to efficiently match it against multiple matcher items. +#[derive(Debug)] pub struct Matcher<'a> { pub(crate) lhs: Option>, pub(crate) rhs: Option>, @@ -46,6 +47,7 @@ pub(crate) enum Needle<'a> { FullName(&'a BStr), PartialName(&'a BStr), Glob { name: &'a BStr, asterisk_pos: usize }, + Pattern(&'a BStr), Object(ObjectId), } @@ -168,9 +170,15 @@ impl<'a> From<&'a BStr> for Needle<'a> { impl<'a> From> for Matcher<'a> { fn from(v: RefSpecRef<'a>) -> Self { - Matcher { + let mut m = Matcher { lhs: v.src.map(Into::into), rhs: v.dst.map(Into::into), + }; + if m.rhs.is_none() { + if let Some(src) = v.src { + m.lhs = Some(Needle::Pattern(src)) + } } + m } } From d6835f25374d17e940017359f41b3cce2ff1ea20 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 2 Nov 2025 14:55:10 +0000 Subject: [PATCH 10/36] Implement Needle::Pattern for full glob support in one-sided refspecs - Added Pattern matching case in Needle::matches() using gix_glob::wildmatch - Added Pattern handling in to_bstr_replace() method - Added is_complex_pattern() helper to detect patterns requiring wildmatch - Only use Needle::Pattern for complex patterns (multiple asterisks or [, ], ?, \) - Simple single-asterisk globs continue using efficient Needle::Glob - Fixed test pattern from v[0.9]* to v[0-9]* to match v1.0 correctly Co-authored-by: Byron <63622+Byron@users.noreply.github.com> --- gix-refspec/src/match_group/util.rs | 27 +++++++++++++++++++++++- gix-refspec/tests/refspec/match_group.rs | 2 +- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/gix-refspec/src/match_group/util.rs b/gix-refspec/src/match_group/util.rs index 7a55e90621c..aa8d3653116 100644 --- a/gix-refspec/src/match_group/util.rs +++ b/gix-refspec/src/match_group/util.rs @@ -104,6 +104,13 @@ impl<'a> Needle<'a> { let end = item.full_ref_name.len() - tail.len(); Match::GlobRange(*asterisk_pos..end) } + Needle::Pattern(pattern) => { + if gix_glob::wildmatch(pattern, item.full_ref_name, gix_glob::wildmatch::Mode::empty()) { + Match::Normal + } else { + Match::None + } + } Needle::Object(id) => { if *id == item.target { return Match::Normal; @@ -139,7 +146,11 @@ impl<'a> Needle<'a> { name.insert_str(0, "refs/heads/"); Cow::Owned(name.into()) } + (Needle::Pattern(name), None) => Cow::Borrowed(name), (Needle::Glob { .. }, None) => unreachable!("BUG: no range provided for glob pattern"), + (Needle::Pattern(_), Some(_)) => { + unreachable!("BUG: range provided for pattern, but patterns don't use ranges") + } (_, Some(_)) => { unreachable!("BUG: range provided even though needle wasn't a glob. Globs are symmetric.") } @@ -176,9 +187,23 @@ impl<'a> From> for Matcher<'a> { }; if m.rhs.is_none() { if let Some(src) = v.src { - m.lhs = Some(Needle::Pattern(src)) + // Only use Pattern for complex globs (multiple asterisks or other glob features) + // Simple single-asterisk globs can use the more efficient Needle::Glob + if is_complex_pattern(src) { + m.lhs = Some(Needle::Pattern(src)); + } } } m } } + +/// Check if a pattern is complex enough to require wildmatch instead of simple glob matching +fn is_complex_pattern(pattern: &BStr) -> bool { + let asterisk_count = pattern.iter().filter(|&&b| b == b'*').count(); + if asterisk_count > 1 { + return true; + } + // Check for other glob features: ?, [, ], \ + pattern.iter().any(|&b| b == b'?' || b == b'[' || b == b']' || b == b'\\') +} diff --git a/gix-refspec/tests/refspec/match_group.rs b/gix-refspec/tests/refspec/match_group.rs index 981130ffbb3..e522e105564 100644 --- a/gix-refspec/tests/refspec/match_group.rs +++ b/gix-refspec/tests/refspec/match_group.rs @@ -254,7 +254,7 @@ mod complex_globs { // Test: refs/tags/* should match all refs under refs/tags/ let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect(); - let spec = gix_refspec::parse("refs/tags/v[0.9]*".into(), Operation::Fetch).unwrap(); + let spec = gix_refspec::parse("refs/tags/v[0-9]*".into(), Operation::Fetch).unwrap(); let group = MatchGroup::from_fetch_specs([spec]); let outcome = group.match_lhs(items.iter().copied()); From c331afc01388a37b66eaeb39c302742b3187de27 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 24 Oct 2025 07:24:12 +0200 Subject: [PATCH 11/36] refactor --- gitoxide-core/src/repository/fetch.rs | 2 +- gix/src/clone/fetch/mod.rs | 21 +++++++++------- gix/src/config/tree/sections/init.rs | 1 + gix/tests/gix/clone.rs | 36 ++++++++++++++++----------- 4 files changed, 36 insertions(+), 24 deletions(-) diff --git a/gitoxide-core/src/repository/fetch.rs b/gitoxide-core/src/repository/fetch.rs index 21198349bb3..2f143a9ec8e 100644 --- a/gitoxide-core/src/repository/fetch.rs +++ b/gitoxide-core/src/repository/fetch.rs @@ -320,7 +320,7 @@ pub(crate) mod function { err, "server sent {} tips, {} were filtered due to {} refspec(s).", map.remote_refs.len(), - map.remote_refs.len() - map.mappings.len(), + map.remote_refs.len().saturating_sub(map.mappings.len()), refspecs.len() )?; } diff --git a/gix/src/clone/fetch/mod.rs b/gix/src/clone/fetch/mod.rs index c56f413061b..46453d960e9 100644 --- a/gix/src/clone/fetch/mod.rs +++ b/gix/src/clone/fetch/mod.rs @@ -2,6 +2,7 @@ use crate::{ bstr::{BString, ByteSlice}, clone::PrepareFetch, }; +use gix_ref::Category; /// The error returned by [`PrepareFetch::fetch_only()`]. #[derive(Debug, thiserror::Error)] @@ -49,6 +50,8 @@ pub enum Error { CommitterOrFallback(#[from] crate::config::time::Error), #[error(transparent)] RefMap(#[from] crate::remote::ref_map::Error), + #[error(transparent)] + ReferenceName(#[from] gix_validate::reference::name::Error), } /// Modification @@ -107,14 +110,13 @@ impl PrepareFetch { // For shallow clones without custom configuration, we'll use a single-branch refspec // to match git's behavior (matching git's single-branch behavior for shallow clones). let use_single_branch_for_shallow = self.shallow != remote::fetch::Shallow::NoChange - && self.configure_remote.is_none() - && remote.fetch_specs.is_empty(); + && remote.fetch_specs.is_empty() + && self.fetch_options.extra_refspecs.is_empty(); let target_ref = if use_single_branch_for_shallow { // Determine target branch from user-specified ref_name or default branch if let Some(ref_name) = &self.ref_name { - // User specified a branch, use that - Some(format!("refs/heads/{}", ref_name.as_ref().as_bstr())) + Some(Category::LocalBranch.to_full_name(ref_name.as_ref().as_bstr())?) } else { // For shallow clones without a specified ref, we need to determine the default branch. // We'll connect to get HEAD information. For Protocol V2, we need to explicitly list refs. @@ -129,7 +131,7 @@ impl PrepareFetch { refs.iter().find_map(|r| match r { gix_protocol::handshake::Ref::Symbolic { full_ref_name, target, .. - } if full_ref_name == "HEAD" => Some(target.to_string()), + } if full_ref_name == "HEAD" => gix_ref::FullName::try_from(target).ok(), _ => None, }) }) @@ -143,9 +145,9 @@ impl PrepareFetch { repo.config .resolved .string(crate::config::tree::Init::DEFAULT_BRANCH) - .and_then(|name| name.to_str().ok().map(|s| format!("refs/heads/{}", s))) + .and_then(|name| Category::LocalBranch.to_full_name(name.as_bstr()).ok()) }) - .unwrap_or_else(|| "refs/heads/main".to_string()); + .unwrap_or_else(|| gix_ref::FullName::try_from("refs/heads/main").expect("known to be valid")); // Drop the connection explicitly to release the borrow on remote drop(connection); @@ -156,11 +158,12 @@ impl PrepareFetch { None }; - // Set up refspec based on whether we're doing a single-branch shallow clone + // Set up refspec based on whether we're doing a single-branch shallow clone, + // which requires a single ref to match Git unless it's overridden. if remote.fetch_specs.is_empty() { if let Some(target_ref) = &target_ref { // Single-branch refspec for shallow clones - let short_name = target_ref.strip_prefix("refs/heads/").unwrap_or(target_ref.as_str()); + let short_name = target_ref.shorten(); remote = remote .with_refspecs( Some(format!("+{target_ref}:refs/remotes/{remote_name}/{short_name}").as_str()), diff --git a/gix/src/config/tree/sections/init.rs b/gix/src/config/tree/sections/init.rs index de42d3b6257..453a906a168 100644 --- a/gix/src/config/tree/sections/init.rs +++ b/gix/src/config/tree/sections/init.rs @@ -5,6 +5,7 @@ use crate::{ impl Init { /// The `init.defaultBranch` key. + // TODO: review its usage for cases where this key is not set - sometimes it's 'master', sometimes it's 'main'. pub const DEFAULT_BRANCH: keys::Any = keys::Any::new("defaultBranch", &config::Tree::INIT) .with_deviation("If not set, we use `main` instead of `master`"); } diff --git a/gix/tests/gix/clone.rs b/gix/tests/gix/clone.rs index 3624afb7e55..c843645b571 100644 --- a/gix/tests/gix/clone.rs +++ b/gix/tests/gix/clone.rs @@ -4,6 +4,10 @@ use crate::{remote, util::restricted}; mod blocking_io { use std::{borrow::Cow, path::Path, sync::atomic::AtomicBool}; + use crate::{ + remote, + util::{hex_to_id, restricted}, + }; use gix::{ bstr::BString, config::tree::{Clone, Core, Init, Key}, @@ -14,11 +18,7 @@ mod blocking_io { }; use gix_object::bstr::ByteSlice; use gix_ref::TargetRef; - - use crate::{ - remote, - util::{hex_to_id, restricted}, - }; + use gix_refspec::parse::Operation; #[test] fn fetch_shallow_no_checkout_then_unshallow() -> crate::Result { @@ -104,15 +104,14 @@ mod blocking_io { // The refspec should be for a single branch (main), not a wildcard let refspec_str = refspecs[0].to_str().expect("valid utf8"); - assert!( - !refspec_str.contains("*"), - "shallow clone refspec should not use wildcard: {}", - refspec_str - ); - assert!( - refspec_str.contains("refs/heads/main"), - "shallow clone refspec should reference the main branch: {}", - refspec_str + assert_eq!( + refspec_str, + if cfg!(windows) { + "+refs/heads/master:refs/remotes/origin/master" + } else { + "+refs/heads/main:refs/remotes/origin/main" + }, + "shallow clone refspec should not use wildcard and should be the main branch: {refspec_str}" ); Ok(()) @@ -238,7 +237,16 @@ mod blocking_io { fn from_non_shallow_by_deepen_exclude_then_deepen_to_unshallow() -> crate::Result { let tmp = gix_testtools::tempfile::TempDir::new()?; let excluded_leaf_refs = ["g", "h", "j"]; + let (repo, _change) = gix::prepare_clone_bare(remote::repo("base").path(), tmp.path())? + .with_fetch_options(gix::remote::ref_map::Options { + extra_refspecs: vec![gix::refspec::parse( + "refs/heads/*:refs/remotes/origin/*".into(), + Operation::Fetch, + )? + .into()], + ..Default::default() + }) .with_shallow(Shallow::Exclude { remote_refs: excluded_leaf_refs .into_iter() From 21840379aa5e576ce1f7b425ea00a3aac28de937 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 3 Nov 2025 05:07:37 +0100 Subject: [PATCH 12/36] feat: add `Time::to_zoned()` to unleash the power of `jiff::Zoned`. --- gix-date/src/time/format.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gix-date/src/time/format.rs b/gix-date/src/time/format.rs index eeb4bf706b7..76f45f25563 100644 --- a/gix-date/src/time/format.rs +++ b/gix-date/src/time/format.rs @@ -42,7 +42,7 @@ impl Time { fn format_inner(&self, format: Format) -> String { match format { - Format::Custom(CustomFormat(format)) => self.to_time().strftime(format).to_string(), + Format::Custom(CustomFormat(format)) => self.to_zoned().strftime(format).to_string(), Format::Unix => self.seconds.to_string(), Format::Raw => self.to_string(), } @@ -50,7 +50,8 @@ impl Time { } impl Time { - fn to_time(self) -> jiff::Zoned { + /// Produce a `Zoned` time for complex time computations and limitless formatting. + pub fn to_zoned(self) -> jiff::Zoned { let offset = jiff::tz::Offset::from_seconds(self.offset).expect("valid offset"); jiff::Timestamp::from_second(self.seconds) .expect("always valid unix time") From fda4e40c043f8378d188ae5deb1fa1d4cf0cb40c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 3 Nov 2025 08:43:55 +0100 Subject: [PATCH 13/36] fix!: make `Time::to_zoned()` fallible and add `Time::format_or_raw()`. After all, the timezone can be invalid, which makes the conversion (and formatting) fail. The `format_or_raw()` method brings back an infallible version. --- gix-date/src/time/format.rs | 26 +++++++++------ gix-date/tests/time/baseline.rs | 20 ++++++------ gix-date/tests/time/format.rs | 57 +++++++++++++++++++-------------- 3 files changed, 60 insertions(+), 43 deletions(-) diff --git a/gix-date/src/time/format.rs b/gix-date/src/time/format.rs index 76f45f25563..552c143018f 100644 --- a/gix-date/src/time/format.rs +++ b/gix-date/src/time/format.rs @@ -36,25 +36,31 @@ impl Time { /// /// Use [`Format::Unix`], [`Format::Raw`] or one of the custom formats /// defined in the [`format`](mod@crate::time::format) submodule. - pub fn format(&self, format: impl Into) -> String { + /// + /// Note that this can fail if the timezone isn't valid and the format requires a conversion to [`jiff::Zoned`]. + pub fn format(&self, format: impl Into) -> Result { self.format_inner(format.into()) } - fn format_inner(&self, format: Format) -> String { - match format { - Format::Custom(CustomFormat(format)) => self.to_zoned().strftime(format).to_string(), + /// Like [`Self::format()`], but on time conversion error, produce the [RAW] format instead to make it + /// infallible. + pub fn format_or_raw(&self, format: impl Into) -> String { + self.format_inner(format.into()).unwrap_or_else(|_| self.to_string()) + } + + fn format_inner(&self, format: Format) -> Result { + Ok(match format { + Format::Custom(CustomFormat(format)) => self.to_zoned()?.strftime(format).to_string(), Format::Unix => self.seconds.to_string(), Format::Raw => self.to_string(), - } + }) } } impl Time { /// Produce a `Zoned` time for complex time computations and limitless formatting. - pub fn to_zoned(self) -> jiff::Zoned { - let offset = jiff::tz::Offset::from_seconds(self.offset).expect("valid offset"); - jiff::Timestamp::from_second(self.seconds) - .expect("always valid unix time") - .to_zoned(offset.to_time_zone()) + pub fn to_zoned(self) -> Result { + let offset = jiff::tz::Offset::from_seconds(self.offset)?; + Ok(jiff::Timestamp::from_second(self.seconds)?.to_zoned(offset.to_time_zone())) } } diff --git a/gix-date/tests/time/baseline.rs b/gix-date/tests/time/baseline.rs index 4eff7cef066..67b9e95f8cd 100644 --- a/gix-date/tests/time/baseline.rs +++ b/gix-date/tests/time/baseline.rs @@ -66,15 +66,17 @@ fn parse_compare_format() { "{pattern:?} disagrees with baseline seconds since epoch: {actual:?}" ); if let Some(format_name) = format_name { - let reformatted = t.format(match format_name.as_str() { - "RFC2822" => Format::Custom(format::RFC2822), - "ISO8601" => Format::Custom(format::ISO8601), - "ISO8601_STRICT" => Format::Custom(format::ISO8601_STRICT), - "GITOXIDE" => Format::Custom(format::GITOXIDE), - "UNIX" => Format::Unix, - "RAW" => Format::Raw, - unknown => unreachable!("All formats should be well-known and implemented: {unknown:?}"), - }); + let reformatted = t + .format(match format_name.as_str() { + "RFC2822" => Format::Custom(format::RFC2822), + "ISO8601" => Format::Custom(format::ISO8601), + "ISO8601_STRICT" => Format::Custom(format::ISO8601_STRICT), + "GITOXIDE" => Format::Custom(format::GITOXIDE), + "UNIX" => Format::Unix, + "RAW" => Format::Raw, + unknown => unreachable!("All formats should be well-known and implemented: {unknown:?}"), + }) + .expect("valid input time"); assert_eq!( reformatted, *pattern, "{reformatted:?} disagrees with baseline pattern: {pattern:?}" diff --git a/gix-date/tests/time/format.rs b/gix-date/tests/time/format.rs index 06e5b8b774c..d5376aa19ec 100644 --- a/gix-date/tests/time/format.rs +++ b/gix-date/tests/time/format.rs @@ -4,19 +4,21 @@ use gix_date::{ }; #[test] -fn short() { - assert_eq!(time().format(format::SHORT), "1973-11-30"); +fn short() -> gix_testtools::Result { + assert_eq!(time().format(format::SHORT)?, "1973-11-30"); + Ok(()) } #[test] -fn unix() { +fn unix() -> gix_testtools::Result { let expected = "123456789"; - assert_eq!(time().format(Format::Unix), expected); - assert_eq!(time().format(format::UNIX), expected); + assert_eq!(time().format(Format::Unix)?, expected); + assert_eq!(time().format(format::UNIX)?, expected); + Ok(()) } #[test] -fn raw() { +fn raw() -> gix_testtools::Result { for (time, expected) in [ (time(), "123456789 +0230"), ( @@ -27,58 +29,65 @@ fn raw() { "1112911993 +0100", ), ] { - assert_eq!(time.format(Format::Raw), expected); - assert_eq!(time.format(format::RAW), expected); + assert_eq!(time.format(Format::Raw)?, expected); + assert_eq!(time.format(format::RAW)?, expected); } + Ok(()) } #[test] -fn iso8601() { - assert_eq!(time().format(format::ISO8601), "1973-11-30 00:03:09 +0230"); +fn iso8601() -> gix_testtools::Result { + assert_eq!(time().format(format::ISO8601)?, "1973-11-30 00:03:09 +0230"); + Ok(()) } #[test] -fn iso8601_strict() { - assert_eq!(time().format(format::ISO8601_STRICT), "1973-11-30T00:03:09+02:30"); +fn iso8601_strict() -> gix_testtools::Result { + assert_eq!(time().format(format::ISO8601_STRICT)?, "1973-11-30T00:03:09+02:30"); + Ok(()) } #[test] -fn rfc2822() { - assert_eq!(time().format(format::RFC2822), "Fri, 30 Nov 1973 00:03:09 +0230"); - assert_eq!(time_dec1().format(format::RFC2822), "Sat, 01 Dec 1973 00:03:09 +0230"); +fn rfc2822() -> gix_testtools::Result { + assert_eq!(time().format(format::RFC2822)?, "Fri, 30 Nov 1973 00:03:09 +0230"); + assert_eq!(time_dec1().format(format::RFC2822)?, "Sat, 01 Dec 1973 00:03:09 +0230"); + Ok(()) } #[test] -fn git_rfc2822() { - assert_eq!(time().format(format::GIT_RFC2822), "Fri, 30 Nov 1973 00:03:09 +0230"); +fn git_rfc2822() -> gix_testtools::Result { + assert_eq!(time().format(format::GIT_RFC2822)?, "Fri, 30 Nov 1973 00:03:09 +0230"); assert_eq!( - time_dec1().format(format::GIT_RFC2822), + time_dec1().format(format::GIT_RFC2822)?, "Sat, 1 Dec 1973 00:03:09 +0230" ); + Ok(()) } #[test] -fn default() { +fn default() -> gix_testtools::Result { assert_eq!( - time().format(gix_date::time::format::GITOXIDE), + time().format(gix_date::time::format::GITOXIDE)?, "Fri Nov 30 1973 00:03:09 +0230" ); assert_eq!( - time_dec1().format(gix_date::time::format::GITOXIDE), + time_dec1().format(gix_date::time::format::GITOXIDE)?, "Sat Dec 01 1973 00:03:09 +0230" ); + Ok(()) } #[test] -fn git_default() { +fn git_default() -> gix_testtools::Result { assert_eq!( - time().format(gix_date::time::format::DEFAULT), + time().format(gix_date::time::format::DEFAULT)?, "Fri Nov 30 00:03:09 1973 +0230" ); assert_eq!( - time_dec1().format(gix_date::time::format::DEFAULT), + time_dec1().format(gix_date::time::format::DEFAULT)?, "Sat Dec 1 00:03:09 1973 +0230" ); + Ok(()) } fn time() -> Time { From d5e194d0ab0e58b02d159b4f48c4e2edd946f355 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 3 Nov 2025 08:49:19 +0100 Subject: [PATCH 14/36] adapt to changes in `gix-date` --- examples/log.rs | 2 +- gitoxide-core/src/query/engine/command.rs | 4 ++-- gitoxide-core/src/repository/revision/explain.rs | 2 +- gix/src/repository/identity.rs | 2 +- gix/src/revision/spec/parse/error.rs | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/examples/log.rs b/examples/log.rs index 0ca592cb57d..76a651e3171 100644 --- a/examples/log.rs +++ b/examples/log.rs @@ -150,7 +150,7 @@ fn run(args: Args) -> anyhow::Result<()> { commit_ref.author.actor().write_to(&mut buf)?; buf.into() }, - time: commit_ref.author.time()?.format(format::DEFAULT), + time: commit_ref.author.time()?.format_or_raw(format::DEFAULT), message: commit_ref.message.to_owned(), }) }), diff --git a/gitoxide-core/src/query/engine/command.rs b/gitoxide-core/src/query/engine/command.rs index 1b020030821..ef3e6f85414 100644 --- a/gitoxide-core/src/query/engine/command.rs +++ b/gitoxide-core/src/query/engine/command.rs @@ -180,7 +180,7 @@ mod trace_path { out, "{}| {} | {} {} {} ➡ {}", self.diff.unwrap_or_default().format(max_diff_lines), - self.commit_time.format(gix::date::time::format::SHORT), + self.commit_time.format_or_raw(gix::date::time::format::SHORT), id.shorten_or_id(), self.mode.as_str(), path_by_id[&source_id], @@ -192,7 +192,7 @@ mod trace_path { out, "{}| {} | {} {} {}", self.diff.unwrap_or_default().format(max_diff_lines), - self.commit_time.format(gix::date::time::format::SHORT), + self.commit_time.format_or_raw(gix::date::time::format::SHORT), id.shorten_or_id(), self.mode.as_str(), path_by_id[&self.file_id] diff --git a/gitoxide-core/src/repository/revision/explain.rs b/gitoxide-core/src/repository/revision/explain.rs index 07bee89cb89..ecc75c47d3f 100644 --- a/gitoxide-core/src/repository/revision/explain.rs +++ b/gitoxide-core/src/repository/revision/explain.rs @@ -88,7 +88,7 @@ impl delegate::Revision for Explain<'_> { ReflogLookup::Date(time) => writeln!( self.out, "Find entry closest to time {} in reflog of '{}' reference", - time.format(gix::date::time::format::ISO8601), + time.format_or_raw(gix::date::time::format::ISO8601), ref_name ) .ok(), diff --git a/gix/src/repository/identity.rs b/gix/src/repository/identity.rs index 7837e9f2d65..212fe78b4b3 100644 --- a/gix/src/repository/identity.rs +++ b/gix/src/repository/identity.rs @@ -140,7 +140,7 @@ impl Personas { .and_then(|date| gix_date::parse(date, Some(SystemTime::now())).ok()) }) .or_else(|| Some(gix_date::Time::now_local_or_utc())) - .map(|time| time.format(gix_date::time::Format::Raw)) + .map(|time| time.format_or_raw(gix_date::time::Format::Raw)) }; let fallback = ( diff --git a/gix/src/revision/spec/parse/error.rs b/gix/src/revision/spec/parse/error.rs index eb361238995..513b3e886e2 100644 --- a/gix/src/revision/spec/parse/error.rs +++ b/gix/src/revision/spec/parse/error.rs @@ -44,7 +44,7 @@ impl std::fmt::Display for CandidateInfo { "commit {} {title:?}", gix_date::parse_header(date) .unwrap_or_default() - .format(gix_date::time::format::SHORT) + .format_or_raw(gix_date::time::format::SHORT) ) } } From e61832ed1ae08dde8a1fe943cea8cc001fc2a857 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 3 Nov 2025 12:52:13 +0100 Subject: [PATCH 15/36] fix: change `Time::format_or_raw()` to `Time::format_or_unix()` As it turns out, `raw` could also panic, but `unix` cannot. --- gix-date/src/time/format.rs | 9 +++++---- gix-date/tests/time/format.rs | 20 +++++++++++++------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/gix-date/src/time/format.rs b/gix-date/src/time/format.rs index 552c143018f..b318f129366 100644 --- a/gix-date/src/time/format.rs +++ b/gix-date/src/time/format.rs @@ -42,10 +42,11 @@ impl Time { self.format_inner(format.into()) } - /// Like [`Self::format()`], but on time conversion error, produce the [RAW] format instead to make it - /// infallible. - pub fn format_or_raw(&self, format: impl Into) -> String { - self.format_inner(format.into()).unwrap_or_else(|_| self.to_string()) + /// Like [`Self::format()`], but on time conversion error, produce the [UNIX] format instead + /// to make it infallible. + pub fn format_or_unix(&self, format: impl Into) -> String { + self.format_inner(format.into()) + .unwrap_or_else(|_| self.seconds.to_string()) } fn format_inner(&self, format: Format) -> Result { diff --git a/gix-date/tests/time/format.rs b/gix-date/tests/time/format.rs index d5376aa19ec..2713eb78757 100644 --- a/gix-date/tests/time/format.rs +++ b/gix-date/tests/time/format.rs @@ -66,15 +66,21 @@ fn git_rfc2822() -> gix_testtools::Result { #[test] fn default() -> gix_testtools::Result { + assert_eq!(time().format(format::GITOXIDE)?, "Fri Nov 30 1973 00:03:09 +0230"); + assert_eq!(time_dec1().format(format::GITOXIDE)?, "Sat Dec 01 1973 00:03:09 +0230"); + Ok(()) +} + +#[test] +fn format_or_unix() { assert_eq!( - time().format(gix_date::time::format::GITOXIDE)?, - "Fri Nov 30 1973 00:03:09 +0230" - ); - assert_eq!( - time_dec1().format(gix_date::time::format::GITOXIDE)?, - "Sat Dec 01 1973 00:03:09 +0230" + Time { + seconds: 42, + offset: 7200 * 60 + } + .format_or_unix(format::GITOXIDE), + "42" ); - Ok(()) } #[test] From 2bcac0c1115e77c5e20692e7cf0dcb1bbb87149c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 3 Nov 2025 12:56:59 +0100 Subject: [PATCH 16/36] adapt to changes in `gix-date` --- examples/log.rs | 2 +- gitoxide-core/src/query/engine/command.rs | 4 ++-- gitoxide-core/src/repository/revision/explain.rs | 2 +- gix/src/repository/identity.rs | 2 +- gix/src/revision/spec/parse/error.rs | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/examples/log.rs b/examples/log.rs index 76a651e3171..293d358529e 100644 --- a/examples/log.rs +++ b/examples/log.rs @@ -150,7 +150,7 @@ fn run(args: Args) -> anyhow::Result<()> { commit_ref.author.actor().write_to(&mut buf)?; buf.into() }, - time: commit_ref.author.time()?.format_or_raw(format::DEFAULT), + time: commit_ref.author.time()?.format_or_unix(format::DEFAULT), message: commit_ref.message.to_owned(), }) }), diff --git a/gitoxide-core/src/query/engine/command.rs b/gitoxide-core/src/query/engine/command.rs index ef3e6f85414..05bdc48a024 100644 --- a/gitoxide-core/src/query/engine/command.rs +++ b/gitoxide-core/src/query/engine/command.rs @@ -180,7 +180,7 @@ mod trace_path { out, "{}| {} | {} {} {} ➡ {}", self.diff.unwrap_or_default().format(max_diff_lines), - self.commit_time.format_or_raw(gix::date::time::format::SHORT), + self.commit_time.format_or_unix(gix::date::time::format::SHORT), id.shorten_or_id(), self.mode.as_str(), path_by_id[&source_id], @@ -192,7 +192,7 @@ mod trace_path { out, "{}| {} | {} {} {}", self.diff.unwrap_or_default().format(max_diff_lines), - self.commit_time.format_or_raw(gix::date::time::format::SHORT), + self.commit_time.format_or_unix(gix::date::time::format::SHORT), id.shorten_or_id(), self.mode.as_str(), path_by_id[&self.file_id] diff --git a/gitoxide-core/src/repository/revision/explain.rs b/gitoxide-core/src/repository/revision/explain.rs index ecc75c47d3f..45e67427b48 100644 --- a/gitoxide-core/src/repository/revision/explain.rs +++ b/gitoxide-core/src/repository/revision/explain.rs @@ -88,7 +88,7 @@ impl delegate::Revision for Explain<'_> { ReflogLookup::Date(time) => writeln!( self.out, "Find entry closest to time {} in reflog of '{}' reference", - time.format_or_raw(gix::date::time::format::ISO8601), + time.format_or_unix(gix::date::time::format::ISO8601), ref_name ) .ok(), diff --git a/gix/src/repository/identity.rs b/gix/src/repository/identity.rs index 212fe78b4b3..ea9f3a506c4 100644 --- a/gix/src/repository/identity.rs +++ b/gix/src/repository/identity.rs @@ -140,7 +140,7 @@ impl Personas { .and_then(|date| gix_date::parse(date, Some(SystemTime::now())).ok()) }) .or_else(|| Some(gix_date::Time::now_local_or_utc())) - .map(|time| time.format_or_raw(gix_date::time::Format::Raw)) + .map(|time| time.format_or_unix(gix_date::time::Format::Raw)) }; let fallback = ( diff --git a/gix/src/revision/spec/parse/error.rs b/gix/src/revision/spec/parse/error.rs index 513b3e886e2..7e043386110 100644 --- a/gix/src/revision/spec/parse/error.rs +++ b/gix/src/revision/spec/parse/error.rs @@ -44,7 +44,7 @@ impl std::fmt::Display for CandidateInfo { "commit {} {title:?}", gix_date::parse_header(date) .unwrap_or_default() - .format_or_raw(gix_date::time::format::SHORT) + .format_or_unix(gix_date::time::format::SHORT) ) } } From ba2301f16261e9e1c49b51b94d6ac43548ec2114 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 2 Nov 2025 20:42:17 +0100 Subject: [PATCH 17/36] refactor --- gix-refspec/src/match_group/util.rs | 17 ++++++++++------- gix-refspec/tests/refspec/match_group.rs | 4 ++-- gix/tests/gix/config/tree.rs | 8 ++++---- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/gix-refspec/src/match_group/util.rs b/gix-refspec/src/match_group/util.rs index aa8d3653116..532b04630fd 100644 --- a/gix-refspec/src/match_group/util.rs +++ b/gix-refspec/src/match_group/util.rs @@ -105,7 +105,11 @@ impl<'a> Needle<'a> { Match::GlobRange(*asterisk_pos..end) } Needle::Pattern(pattern) => { - if gix_glob::wildmatch(pattern, item.full_ref_name, gix_glob::wildmatch::Mode::empty()) { + if gix_glob::wildmatch( + pattern, + item.full_ref_name, + gix_glob::wildmatch::Mode::NO_MATCH_SLASH_LITERAL, + ) { Match::Normal } else { Match::None @@ -187,9 +191,7 @@ impl<'a> From> for Matcher<'a> { }; if m.rhs.is_none() { if let Some(src) = v.src { - // Only use Pattern for complex globs (multiple asterisks or other glob features) - // Simple single-asterisk globs can use the more efficient Needle::Glob - if is_complex_pattern(src) { + if must_use_pattern_matching(src) { m.lhs = Some(Needle::Pattern(src)); } } @@ -199,11 +201,12 @@ impl<'a> From> for Matcher<'a> { } /// Check if a pattern is complex enough to require wildmatch instead of simple glob matching -fn is_complex_pattern(pattern: &BStr) -> bool { +fn must_use_pattern_matching(pattern: &BStr) -> bool { let asterisk_count = pattern.iter().filter(|&&b| b == b'*').count(); if asterisk_count > 1 { return true; } - // Check for other glob features: ?, [, ], \ - pattern.iter().any(|&b| b == b'?' || b == b'[' || b == b']' || b == b'\\') + pattern + .iter() + .any(|&b| b == b'?' || b == b'[' || b == b']' || b == b'\\') } diff --git a/gix-refspec/tests/refspec/match_group.rs b/gix-refspec/tests/refspec/match_group.rs index e522e105564..032dc360338 100644 --- a/gix-refspec/tests/refspec/match_group.rs +++ b/gix-refspec/tests/refspec/match_group.rs @@ -214,7 +214,7 @@ mod complex_globs { #[test] fn one_sided_simple_glob_patterns_match() { // Test that simple glob patterns (one asterisk) work correctly with matching - let refs = vec![ + let refs = [ new_ref("refs/heads/feature/foo", "1111111111111111111111111111111111111111"), new_ref("refs/heads/bugfix/bar", "2222222222222222222222222222222222222222"), new_ref("refs/tags/v1.0", "3333333333333333333333333333333333333333"), @@ -277,7 +277,7 @@ mod complex_globs { #[test] fn one_sided_glob_with_suffix_matches() { // Test that glob patterns with suffix work correctly - let refs = vec![ + let refs = [ new_ref("refs/heads/feature", "1111111111111111111111111111111111111111"), new_ref("refs/heads/feat", "2222222222222222222222222222222222222222"), new_ref("refs/heads/main", "3333333333333333333333333333333333333333"), diff --git a/gix/tests/gix/config/tree.rs b/gix/tests/gix/config/tree.rs index 686f26f461d..22190235f94 100644 --- a/gix/tests/gix/config/tree.rs +++ b/gix/tests/gix/config/tree.rs @@ -1125,17 +1125,17 @@ mod remote { assert_eq!( Remote::FETCH - .try_into_refspec(bcow("*/*/*"), gix_refspec::parse::Operation::Fetch) + .try_into_refspec(bcow("*/*/*:refs/heads/*"), gix_refspec::parse::Operation::Fetch) .unwrap_err() .to_string(), - "The refspec at \"remote..fetch=*/*/*\" could not be parsed" + "The refspec at \"remote..fetch=*/*/*:refs/heads/*\" could not be parsed" ); assert_eq!( Remote::PUSH - .try_into_refspec(bcow("*/*/*"), gix_refspec::parse::Operation::Push) + .try_into_refspec(bcow("*/*/*:refs/heads/*"), gix_refspec::parse::Operation::Push) .unwrap_err() .to_string(), - "The refspec at \"remote..push=*/*/*\" could not be parsed" + "The refspec at \"remote..push=*/*/*:refs/heads/*\" could not be parsed" ); } } From ae65278263c8d65697acfe33207819a7867ad389 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 4 Nov 2025 07:50:02 +0100 Subject: [PATCH 18/36] refactor --- gix-transport/Cargo.toml | 6 +++- .../src/client/blocking_io/http/mod.rs | 6 ++-- gix-transport/src/client/capabilities.rs | 36 +++++++++---------- gix-transport/src/client/git/async_io.rs | 6 ++-- gix-transport/src/client/git/blocking_io.rs | 6 ++-- gix-transport/tests/client/capabilities.rs | 6 ++-- 6 files changed, 35 insertions(+), 31 deletions(-) diff --git a/gix-transport/Cargo.toml b/gix-transport/Cargo.toml index 28983270981..507bc758aec 100644 --- a/gix-transport/Cargo.toml +++ b/gix-transport/Cargo.toml @@ -18,6 +18,8 @@ doctest = false default = [] ## If set, blocking implementations of the typical git transports become available in `crate::client::blocking_io` +## +## If used in conjunction with an async implementation, this one takes precedence. blocking-client = ["gix-packetline/blocking-io"] ## Implies `blocking-client`, and adds support for the http and https transports. http-client = [ @@ -27,10 +29,12 @@ http-client = [ "gix-credentials", ] ## Implies `http-client`, and adds support for the http and https transports using the Rust bindings for `libcurl`. +## +## If used in conjunction with other blocking implementations like `http-client-reqwest`, this one takes precedence. http-client-curl = ["curl", "http-client"] ## Implies `http-client-curl` and enables `rustls` for creating `https://` connections. http-client-curl-rust-tls = ["http-client-curl", "curl/rustls"] -### Implies `http-client` and adds support for http and https transports using the blocking version of `reqwest`. +## Implies `http-client` and adds support for http and https transports using the blocking version of `reqwest`. http-client-reqwest = ["reqwest", "http-client"] ## Stacks with `blocking-http-transport-reqwest` and enables `https://` via the `rustls` crate. http-client-reqwest-rust-tls = ["http-client-reqwest", "reqwest/rustls-tls"] diff --git a/gix-transport/src/client/blocking_io/http/mod.rs b/gix-transport/src/client/blocking_io/http/mod.rs index 864fe3809f7..df4bda43512 100644 --- a/gix-transport/src/client/blocking_io/http/mod.rs +++ b/gix-transport/src/client/blocking_io/http/mod.rs @@ -19,7 +19,7 @@ use crate::{ http::options::{HttpVersion, SslVersionRangeInclusive}, ExtendedBufRead, HandleProgress, RequestWriter, SetServiceResponse, }, - capabilities::blocking_recv::Outcome, + capabilities::blocking_recv::Handshake, MessageKind, }, packetline::{blocking_io::StreamingPeekableIter, PacketLineRef}, @@ -403,11 +403,11 @@ impl blocking_io::Transport for Transport { line_reader.as_read().read_to_end(&mut Vec::new())?; } - let Outcome { + let Handshake { capabilities, refs, protocol: actual_protocol, - } = Outcome::from_lines_with_version_detection(line_reader)?; + } = Handshake::from_lines_with_version_detection(line_reader)?; self.actual_version = actual_protocol; self.service = Some(service); Ok(SetServiceResponse { diff --git a/gix-transport/src/client/capabilities.rs b/gix-transport/src/client/capabilities.rs index dc0fd2bd296..79bf34a6347 100644 --- a/gix-transport/src/client/capabilities.rs +++ b/gix-transport/src/client/capabilities.rs @@ -165,8 +165,8 @@ impl Capabilities { } } -#[cfg(feature = "blocking-client")] /// +#[cfg(feature = "blocking-client")] pub mod blocking_recv { use std::io; @@ -178,8 +178,8 @@ pub mod blocking_recv { Protocol, }; - /// Success outcome of [`Outcome::from_lines_with_version_detection`]. - pub struct Outcome<'a> { + /// The information provided by the server upon first connection. + pub struct Handshake<'a> { /// The [`Capabilities`] the remote advertised. pub capabilities: Capabilities, /// The remote refs as a [`io::BufRead`]. @@ -191,14 +191,14 @@ pub mod blocking_recv { pub protocol: Protocol, } - impl Outcome<'_> { + impl Handshake<'_> { /// Read the capabilities and version advertisement from the given packetline reader. /// /// If [`Protocol::V1`] was requested, or the remote decided to downgrade, the remote refs - /// advertisement will also be included in the [`Outcome`]. + /// advertisement will also be included in the [`Handshake`]. pub fn from_lines_with_version_detection( rd: &mut StreamingPeekableIter, - ) -> Result, client::Error> { + ) -> Result, client::Error> { // NOTE that this is vitally important - it is turned on and stays on for all following requests so // we automatically abort if the server sends an ERR line anywhere. // We are sure this can't clash with binary data when sent due to the way the PACK @@ -214,13 +214,13 @@ pub mod blocking_recv { Protocol::V1 => { let (capabilities, delimiter_position) = Capabilities::from_bytes(line.0)?; rd.peek_buffer_replace_and_truncate(delimiter_position, b'\n'); - Outcome { + Handshake { capabilities, refs: Some(Box::new(rd.as_read())), protocol: Protocol::V1, } } - Protocol::V2 => Outcome { + Protocol::V2 => Handshake { capabilities: { let mut rd = rd.as_read(); let mut buf = Vec::new(); @@ -243,7 +243,7 @@ pub mod blocking_recv { }, } } - None => Outcome { + None => Handshake { capabilities: Capabilities::default(), refs: Some(Box::new(rd.as_read())), protocol: Protocol::V0, @@ -253,9 +253,9 @@ pub mod blocking_recv { } } +/// #[cfg(feature = "async-client")] #[allow(missing_docs)] -/// pub mod async_recv { use bstr::ByteVec; use futures_io::AsyncRead; @@ -266,8 +266,8 @@ pub mod async_recv { Protocol, }; - /// Success outcome of [`Outcome::from_lines_with_version_detection`]. - pub struct Outcome<'a> { + /// The information provided by the server upon first connection. + pub struct Handshake<'a> { /// The [`Capabilities`] the remote advertised. pub capabilities: Capabilities, /// The remote refs as an [`AsyncBufRead`]. @@ -279,14 +279,14 @@ pub mod async_recv { pub protocol: Protocol, } - impl Outcome<'_> { + impl Handshake<'_> { /// Read the capabilities and version advertisement from the given packetline reader. /// /// If [`Protocol::V1`] was requested, or the remote decided to downgrade, the remote refs - /// advertisement will also be included in the [`Outcome`]. + /// advertisement will also be included in the [`Handshake`]. pub async fn from_lines_with_version_detection( rd: &mut StreamingPeekableIter, - ) -> Result, client::Error> { + ) -> Result, client::Error> { // NOTE that this is vitally important - it is turned on and stays on for all following requests so // we automatically abort if the server sends an ERR line anywhere. // We are sure this can't clash with binary data when sent due to the way the PACK @@ -302,13 +302,13 @@ pub mod async_recv { Protocol::V1 => { let (capabilities, delimiter_position) = Capabilities::from_bytes(line.0)?; rd.peek_buffer_replace_and_truncate(delimiter_position, b'\n'); - Outcome { + Handshake { capabilities, refs: Some(Box::new(rd.as_read())), protocol: Protocol::V1, } } - Protocol::V2 => Outcome { + Protocol::V2 => Handshake { capabilities: { let mut rd = rd.as_read(); let mut buf = Vec::new(); @@ -331,7 +331,7 @@ pub mod async_recv { }, } } - None => Outcome { + None => Handshake { capabilities: Capabilities::default(), refs: Some(Box::new(rd.as_read())), protocol: Protocol::V0, diff --git a/gix-transport/src/client/git/async_io.rs b/gix-transport/src/client/git/async_io.rs index b8bfc6539bc..74feeddc7fb 100644 --- a/gix-transport/src/client/git/async_io.rs +++ b/gix-transport/src/client/git/async_io.rs @@ -9,7 +9,7 @@ use crate::{ client::{ self, async_io::{RequestWriter, SetServiceResponse}, - capabilities::async_recv::Outcome, + capabilities::async_recv::Handshake, git::{self, ConnectionState}, }, packetline::{ @@ -96,11 +96,11 @@ where line_writer.flush().await?; } - let Outcome { + let Handshake { capabilities, refs, protocol: actual_protocol, - } = Outcome::from_lines_with_version_detection(&mut self.line_provider).await?; + } = Handshake::from_lines_with_version_detection(&mut self.line_provider).await?; Ok(SetServiceResponse { actual_protocol, capabilities, diff --git a/gix-transport/src/client/git/blocking_io.rs b/gix-transport/src/client/git/blocking_io.rs index afa1f3c5051..92fe5b61a95 100644 --- a/gix-transport/src/client/git/blocking_io.rs +++ b/gix-transport/src/client/git/blocking_io.rs @@ -6,7 +6,7 @@ use crate::{ client::{ self, blocking_io::{RequestWriter, SetServiceResponse}, - capabilities::blocking_recv::Outcome, + capabilities::blocking_recv::Handshake, git::{self, ConnectionState}, }, packetline::{ @@ -91,11 +91,11 @@ where line_writer.flush()?; } - let Outcome { + let Handshake { capabilities, refs, protocol: actual_protocol, - } = Outcome::from_lines_with_version_detection(&mut self.line_provider)?; + } = Handshake::from_lines_with_version_detection(&mut self.line_provider)?; Ok(SetServiceResponse { actual_protocol, capabilities, diff --git a/gix-transport/tests/client/capabilities.rs b/gix-transport/tests/client/capabilities.rs index 1792de659d9..6b6d356e8f4 100644 --- a/gix-transport/tests/client/capabilities.rs +++ b/gix-transport/tests/client/capabilities.rs @@ -4,9 +4,9 @@ use gix_packetline::async_io::{encode, StreamingPeekableIter}; #[cfg(all(feature = "blocking-client", not(feature = "async-client")))] use gix_packetline::blocking_io::{encode, StreamingPeekableIter}; #[cfg(all(feature = "async-client", not(feature = "blocking-client")))] -use gix_transport::client::capabilities::async_recv::Outcome; +use gix_transport::client::capabilities::async_recv::Handshake; #[cfg(all(feature = "blocking-client", not(feature = "async-client")))] -use gix_transport::client::capabilities::blocking_recv::Outcome; +use gix_transport::client::capabilities::blocking_recv::Handshake; use gix_transport::client::Capabilities; #[test] @@ -63,7 +63,7 @@ async fn from_lines_with_version_detection_v0() -> crate::Result { let mut buf = Vec::::new(); encode::flush_to_write(&mut buf).await?; let mut stream = StreamingPeekableIter::new(buf.as_slice(), &[gix_packetline::PacketLineRef::Flush], false); - let caps = Outcome::from_lines_with_version_detection(&mut stream) + let caps = Handshake::from_lines_with_version_detection(&mut stream) .await .expect("we can parse V0 as very special case, useful for testing stateful connections in other crates") .capabilities; From 102077b3207ee24ff971ff543880158c04324b98 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 26 Oct 2025 11:14:00 +0100 Subject: [PATCH 19/36] extract helper utility from RefMap::new() --- gix-protocol/src/fetch/refmap/init.rs | 59 ++++++++++++++++----------- gix-protocol/src/fetch/types.rs | 5 +-- 2 files changed, 37 insertions(+), 27 deletions(-) diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index ac005be60dd..0f95bcf5d3b 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -1,18 +1,21 @@ use std::collections::HashSet; -use bstr::{BString, ByteVec}; +use bstr::{BString, ByteSlice, ByteVec}; use gix_features::progress::Progress; +use gix_refspec::RefSpec; +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, fetch::{ + self, refmap::{Mapping, Source, SpecIndex}, RefMap, }, + handshake::Ref, }; /// The error returned by [`RefMap::new()`]. @@ -78,8 +81,6 @@ impl RefMap { T: Transport, { let _span = gix_trace::coarse!("gix_protocol::fetch::RefMap::new()"); - let null = gix_hash::ObjectId::null(gix_hash::Kind::Sha1); // OK to hardcode Sha1, it's not supposed to match, ever. - let all_refspecs = { let mut s: Vec<_> = fetch_refspecs.to_vec(); s.extend(extra_refspecs.clone()); @@ -115,8 +116,26 @@ impl RefMap { .await? } }; + + Self::from_refs( + remote_refs, + &handshake.capabilities, + fetch_refspecs, + all_refspecs, + extra_refspecs, + ) + } + + fn from_refs( + remote_refs: Vec, + capabilities: &Capabilities, + fetch_refspecs: &[RefSpec], + all_refspecs: Vec, + extra_refspecs: Vec, + ) -> Result { let num_explicit_specs = fetch_refspecs.len(); let group = gix_refspec::MatchGroup::from_fetch_specs(all_refspecs.iter().map(gix_refspec::RefSpec::to_ref)); + let null = gix_hash::ObjectId::null(gix_hash::Kind::Sha1); // OK to hardcode Sha1, it's not supposed to match, ever. let (res, fixes) = group .match_lhs(remote_refs.iter().map(|r| { let (full_ref_name, target, object) = r.unpack(); @@ -150,24 +169,9 @@ impl RefMap { }) .collect(); - let object_hash = extract_object_format(handshake)?; - Ok(RefMap { - mappings, - refspecs: fetch_refspecs.to_vec(), - extra_refspecs, - fixes, - remote_refs, - object_hash, - }) - } -} - -/// Assume sha1 if server says nothing, otherwise configure anything beyond sha1 in the local repo configuration -#[allow(clippy::result_large_err)] -fn extract_object_format(outcome: &crate::handshake::Outcome) -> Result { - use bstr::ByteSlice; - let object_hash = - if let Some(object_format) = outcome.capabilities.capability("object-format").and_then(|c| c.value()) { + // Assume sha1 if server says nothing, otherwise configure anything beyond sha1 in the local repo configuration + let object_hash = if let Some(object_format) = capabilities.capability("object-format").and_then(|c| c.value()) + { let object_format = object_format.to_str().map_err(|_| Error::UnknownObjectFormat { format: object_format.into(), })?; @@ -178,5 +182,14 @@ fn extract_object_format(outcome: &crate::handshake::Outcome) -> Result { #[cfg(feature = "fetch")] mod with_fetch { - use crate::{ - fetch, - fetch::{negotiate, refmap}, - }; + use crate::fetch::{self, negotiate, refmap}; /// For use in [`fetch`](crate::fetch()). pub struct NegotiateContext<'a, 'b, 'c, Objects, Alternates, AlternatesOut, AlternatesErr, Find> From b05eba5cca4294b9baeafd0c15885cefd21d0d6c Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 1 Nov 2025 16:29:53 +0100 Subject: [PATCH 20/36] refactor!: pass user agent into ls_refs() separately --- gix-protocol/src/fetch/refmap/init.rs | 4 ++-- gix-protocol/src/ls_refs.rs | 10 ++++------ gix-protocol/tests/protocol/fetch/_impl.rs | 14 ++++---------- gix-protocol/tests/protocol/fetch/mod.rs | 2 -- 4 files changed, 10 insertions(+), 20 deletions(-) diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index 0f95bcf5d3b..c1618cd3109 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -92,8 +92,7 @@ impl RefMap { crate::ls_refs( transport, &handshake.capabilities, - |_capabilities, arguments, features| { - features.push(user_agent); + |_capabilities, arguments| { if prefix_from_spec_as_filter_on_remote { let mut seen = HashSet::new(); for spec in &all_refspecs { @@ -112,6 +111,7 @@ impl RefMap { }, &mut progress, trace_packetlines, + user_agent, ) .await? } diff --git a/gix-protocol/src/ls_refs.rs b/gix-protocol/src/ls_refs.rs index 3a435c5cd53..949ae8f161a 100644 --- a/gix-protocol/src/ls_refs.rs +++ b/gix-protocol/src/ls_refs.rs @@ -68,17 +68,15 @@ pub(crate) mod function { pub async fn ls_refs( mut transport: impl Transport, capabilities: &Capabilities, - prepare_ls_refs: impl FnOnce( - &Capabilities, - &mut Vec, - &mut Vec<(&str, Option>)>, - ) -> std::io::Result, + prepare_ls_refs: impl FnOnce(&Capabilities, &mut Vec) -> std::io::Result, 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") @@ -87,7 +85,7 @@ pub(crate) mod function { { ls_args.push("unborn".into()); } - let refs = match prepare_ls_refs(capabilities, &mut ls_args, &mut ls_features) { + let refs = match prepare_ls_refs(capabilities, &mut ls_args) { 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 e3d7077d195..0496ec64f73 100644 --- a/gix-protocol/tests/protocol/fetch/_impl.rs +++ b/gix-protocol/tests/protocol/fetch/_impl.rs @@ -94,13 +94,10 @@ mod fetch_fn { gix_protocol::ls_refs( &mut transport, &capabilities, - |a, b, c| { - let res = delegate.prepare_ls_refs(a, b, c); - c.push(("agent", Some(Cow::Owned(agent.clone())))); - res - }, + |a, b| delegate.prepare_ls_refs(a, b), &mut progress, trace, + ("agent", Some(Cow::Owned(agent.clone()))), ) .await? } @@ -240,7 +237,6 @@ mod delegate { &mut self, _server: &Capabilities, _arguments: &mut Vec, - _features: &mut Vec<(&str, Option>)>, ) -> std::io::Result { Ok(ls_refs::Action::Continue) } @@ -310,9 +306,8 @@ mod delegate { &mut self, _server: &Capabilities, _arguments: &mut Vec, - _features: &mut Vec<(&str, Option>)>, ) -> io::Result { - self.deref_mut().prepare_ls_refs(_server, _arguments, _features) + self.deref_mut().prepare_ls_refs(_server, _arguments) } fn prepare_fetch( @@ -344,9 +339,8 @@ mod delegate { &mut self, _server: &Capabilities, _arguments: &mut Vec, - _features: &mut Vec<(&str, Option>)>, ) -> io::Result { - self.deref_mut().prepare_ls_refs(_server, _arguments, _features) + self.deref_mut().prepare_ls_refs(_server, _arguments) } fn prepare_fetch( diff --git a/gix-protocol/tests/protocol/fetch/mod.rs b/gix-protocol/tests/protocol/fetch/mod.rs index 03f69bed8d8..6725959a58f 100644 --- a/gix-protocol/tests/protocol/fetch/mod.rs +++ b/gix-protocol/tests/protocol/fetch/mod.rs @@ -106,7 +106,6 @@ impl DelegateBlocking for CloneRefInWantDelegate { &mut self, _server: &Capabilities, _arguments: &mut Vec, - _features: &mut Vec<(&str, Option>)>, ) -> io::Result { Ok(ls_refs::Action::Skip) } @@ -150,7 +149,6 @@ impl DelegateBlocking for LsRemoteDelegate { &mut self, _server: &Capabilities, _arguments: &mut Vec, - _features: &mut Vec<(&str, Option>)>, ) -> std::io::Result { match self.abort_with.take() { Some(err) => Err(err), From f05dfab881481de5de1d23e3266cad9c069f59c5 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 1 Nov 2025 18:22:25 +0100 Subject: [PATCH 21/36] refactor!: flatten RefMap::new() arguments --- gitoxide-core/src/pack/receive.rs | 10 ++++------ gix-protocol/src/fetch/refmap/init.rs | 18 ++++++------------ gix/src/remote/connection/ref_map.rs | 10 ++++------ 3 files changed, 14 insertions(+), 24 deletions(-) diff --git a/gitoxide-core/src/pack/receive.rs b/gitoxide-core/src/pack/receive.rs index 288133ca1c6..0cde14499f8 100644 --- a/gitoxide-core/src/pack/receive.rs +++ b/gitoxide-core/src/pack/receive.rs @@ -85,12 +85,10 @@ where let refmap = gix::protocol::fetch::RefMap::new( &mut progress, &fetch_refspecs, - gix::protocol::fetch::Context { - handshake: &mut handshake, - transport: &mut transport.inner, - user_agent: user_agent.clone(), - trace_packetlines, - }, + &mut handshake, + &mut transport.inner, + user_agent.clone(), + trace_packetlines, gix::protocol::fetch::refmap::init::Options::default(), ) .await?; diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index c1618cd3109..3a424c99f9c 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::{borrow::Cow, collections::HashSet}; use bstr::{BString, ByteSlice, ByteVec}; use gix_features::progress::Progress; @@ -11,11 +11,10 @@ use crate::transport::client::async_io::Transport; use crate::transport::client::blocking_io::Transport; use crate::{ fetch::{ - self, refmap::{Mapping, Source, SpecIndex}, RefMap, }, - handshake::Ref, + handshake::{Outcome, Ref}, }; /// The error returned by [`RefMap::new()`]. @@ -55,9 +54,6 @@ impl RefMap { /// Create a new instance by obtaining all references on the remote that have been filtered through our remote's /// for _fetching_. /// - /// A [context](fetch::Context) is provided to bundle what would be additional parameters, - /// and [options](Options) are used to further configure the call. - /// /// * `progress` is used if `ls-refs` is invoked on the remote. Always the case when V2 is used. /// * `fetch_refspecs` are all explicit refspecs to identify references on the remote that you are interested in. /// Note that these are copied to [`RefMap::refspecs`] for convenience, as `RefMap::mappings` refer to them by index. @@ -66,12 +62,10 @@ impl RefMap { pub async fn new( mut progress: impl Progress, fetch_refspecs: &[gix_refspec::RefSpec], - fetch::Context { - handshake, - transport, - user_agent, - trace_packetlines, - }: fetch::Context<'_, T>, + handshake: &mut Outcome, + transport: &mut T, + user_agent: (&'static str, Option>), + trace_packetlines: bool, Options { prefix_from_spec_as_filter_on_remote, extra_refspecs, diff --git a/gix/src/remote/connection/ref_map.rs b/gix/src/remote/connection/ref_map.rs index a66813ec0d0..f8e32ea5ba2 100644 --- a/gix/src/remote/connection/ref_map.rs +++ b/gix/src/remote/connection/ref_map.rs @@ -153,12 +153,10 @@ where let refmap = gix_protocol::fetch::RefMap::new( progress, &self.remote.fetch_specs, - gix_protocol::fetch::Context { - handshake: &mut handshake, - transport: &mut self.transport.inner, - user_agent: self.remote.repo.config.user_agent_tuple(), - trace_packetlines: self.trace, - }, + &mut handshake, + &mut self.transport.inner, + self.remote.repo.config.user_agent_tuple(), + self.trace, gix_protocol::fetch::refmap::init::Options { prefix_from_spec_as_filter_on_remote, extra_refspecs, From 37ab03687a940c89131141b882e5ef1a7fef85be Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 1 Nov 2025 21:18:23 +0100 Subject: [PATCH 22/36] refactor!: store fetch refspecs in Options --- gitoxide-core/src/pack/receive.rs | 3 +-- gix-protocol/src/fetch/refmap/init.rs | 17 ++++++++++------- gix/src/remote/connection/ref_map.rs | 2 +- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/gitoxide-core/src/pack/receive.rs b/gitoxide-core/src/pack/receive.rs index 0cde14499f8..52ab85acb73 100644 --- a/gitoxide-core/src/pack/receive.rs +++ b/gitoxide-core/src/pack/receive.rs @@ -84,12 +84,11 @@ where let user_agent = ("agent", Some(agent.clone().into())); let refmap = gix::protocol::fetch::RefMap::new( &mut progress, - &fetch_refspecs, &mut handshake, &mut transport.inner, user_agent.clone(), trace_packetlines, - gix::protocol::fetch::refmap::init::Options::default(), + gix::protocol::fetch::refmap::init::Options::fetch(fetch_refspecs.clone()), ) .await?; diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index 3a424c99f9c..4c870398426 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -32,6 +32,9 @@ pub enum Error { /// For use in [`RefMap::new()`]. #[derive(Debug, Clone)] pub struct Options { + /// All explicit refspecs to identify references on the remote that you are interested in. + /// Note that these are copied to [`RefMap::refspecs`] for convenience, as `RefMap::mappings` refer to them by index. + pub fetch_refspecs: Vec, /// 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. Defaults to `true`. pub prefix_from_spec_as_filter_on_remote: bool, @@ -41,9 +44,11 @@ pub struct Options { pub extra_refspecs: Vec, } -impl Default for Options { - fn default() -> Self { +impl Options { + /// Create options to fetch the given `fetch_refspecs`. + pub fn fetch(fetch_refspecs: Vec) -> Self { Options { + fetch_refspecs, prefix_from_spec_as_filter_on_remote: true, extra_refspecs: Vec::new(), } @@ -55,18 +60,16 @@ impl RefMap { /// for _fetching_. /// /// * `progress` is used if `ls-refs` is invoked on the remote. Always the case when V2 is used. - /// * `fetch_refspecs` are all explicit refspecs to identify references on the remote that you are interested in. - /// Note that these are copied to [`RefMap::refspecs`] for convenience, as `RefMap::mappings` refer to them by index. #[allow(clippy::result_large_err)] #[maybe_async::maybe_async] pub async fn new( mut progress: impl Progress, - fetch_refspecs: &[gix_refspec::RefSpec], handshake: &mut Outcome, transport: &mut T, user_agent: (&'static str, Option>), trace_packetlines: bool, Options { + fetch_refspecs, prefix_from_spec_as_filter_on_remote, extra_refspecs, }: Options, @@ -123,7 +126,7 @@ impl RefMap { fn from_refs( remote_refs: Vec, capabilities: &Capabilities, - fetch_refspecs: &[RefSpec], + fetch_refspecs: Vec, all_refspecs: Vec, extra_refspecs: Vec, ) -> Result { @@ -179,7 +182,7 @@ impl RefMap { Ok(Self { mappings, - refspecs: fetch_refspecs.to_vec(), + refspecs: fetch_refspecs, extra_refspecs, fixes, remote_refs, diff --git a/gix/src/remote/connection/ref_map.rs b/gix/src/remote/connection/ref_map.rs index f8e32ea5ba2..13517fab756 100644 --- a/gix/src/remote/connection/ref_map.rs +++ b/gix/src/remote/connection/ref_map.rs @@ -152,12 +152,12 @@ where .await?; let refmap = gix_protocol::fetch::RefMap::new( progress, - &self.remote.fetch_specs, &mut handshake, &mut self.transport.inner, self.remote.repo.config.user_agent_tuple(), self.trace, gix_protocol::fetch::refmap::init::Options { + fetch_refspecs: self.remote.fetch_specs.clone(), prefix_from_spec_as_filter_on_remote, extra_refspecs, }, From 50120de92dab3d8347672df7a43df02291162c9a Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 1 Nov 2025 21:23:37 +0100 Subject: [PATCH 23/36] refactor!: store all_refspecs in Options --- gix-protocol/src/fetch/refmap/init.rs | 34 +++++++++++++++++---------- gix/src/remote/connection/ref_map.rs | 6 ++--- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index 4c870398426..4271c15d639 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -34,23 +34,37 @@ pub enum Error { pub struct Options { /// All explicit refspecs to identify references on the remote that you are interested in. /// Note that these are copied to [`RefMap::refspecs`] for convenience, as `RefMap::mappings` refer to them by index. - pub fetch_refspecs: Vec, + fetch_refspecs: Vec, /// 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. Defaults to `true`. - pub prefix_from_spec_as_filter_on_remote: bool, + prefix_from_spec_as_filter_on_remote: bool, /// A list of refspecs to use as implicit refspecs which won't be saved or otherwise be part of the remote in question. /// /// This is useful for handling `remote..tagOpt` for example. - pub extra_refspecs: Vec, + extra_refspecs: Vec, + all_refspecs: Vec, } impl Options { /// Create options to fetch the given `fetch_refspecs`. pub fn fetch(fetch_refspecs: Vec) -> Self { - Options { - fetch_refspecs, - prefix_from_spec_as_filter_on_remote: true, - extra_refspecs: Vec::new(), + Self::new(fetch_refspecs, true, Vec::new()) + } + + /// Specify all options for fetching. + pub fn new( + fetch_refspecs: Vec, + prefix_from_spec_as_filter_on_remote: bool, + extra_refspecs: Vec, + ) -> Self { + let mut all_refspecs = fetch_refspecs.clone(); + all_refspecs.extend(extra_refspecs.iter().cloned()); + + Self { + all_refspecs, + fetch_refspecs: fetch_refspecs.clone(), + prefix_from_spec_as_filter_on_remote, + extra_refspecs, } } } @@ -72,17 +86,13 @@ impl RefMap { fetch_refspecs, prefix_from_spec_as_filter_on_remote, extra_refspecs, + all_refspecs, }: Options, ) -> Result where T: Transport, { let _span = gix_trace::coarse!("gix_protocol::fetch::RefMap::new()"); - let all_refspecs = { - let mut s: Vec<_> = fetch_refspecs.to_vec(); - s.extend(extra_refspecs.clone()); - s - }; let remote_refs = match handshake.refs.take() { Some(refs) => refs, None => { diff --git a/gix/src/remote/connection/ref_map.rs b/gix/src/remote/connection/ref_map.rs index 13517fab756..3b218b0fa8b 100644 --- a/gix/src/remote/connection/ref_map.rs +++ b/gix/src/remote/connection/ref_map.rs @@ -156,11 +156,11 @@ where &mut self.transport.inner, self.remote.repo.config.user_agent_tuple(), self.trace, - gix_protocol::fetch::refmap::init::Options { - fetch_refspecs: self.remote.fetch_specs.clone(), + gix_protocol::fetch::refmap::init::Options::new( + self.remote.fetch_specs.clone(), prefix_from_spec_as_filter_on_remote, extra_refspecs, - }, + ), ) .await?; self.handshake = Some(handshake); From 4c40484391c32b4d42b5904a1a0c42639369673e Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 1 Nov 2025 21:28:51 +0100 Subject: [PATCH 24/36] refactor: keep Options intact through fetching --- gix-protocol/src/fetch/refmap/init.rs | 63 ++++++++++++--------------- 1 file changed, 29 insertions(+), 34 deletions(-) diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index 4271c15d639..74c38bef9c9 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -2,7 +2,6 @@ use std::{borrow::Cow, collections::HashSet}; use bstr::{BString, ByteSlice, ByteVec}; use gix_features::progress::Progress; -use gix_refspec::RefSpec; use gix_transport::client::Capabilities; #[cfg(feature = "async-client")] @@ -67,6 +66,25 @@ impl Options { extra_refspecs, } } + + fn push_prefix_arguments(&self, arguments: &mut Vec) { + if !self.prefix_from_spec_as_filter_on_remote { + return; + } + + let mut seen = HashSet::new(); + for spec in &self.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); + } + } + } + } } impl RefMap { @@ -82,12 +100,7 @@ impl RefMap { transport: &mut T, user_agent: (&'static str, Option>), trace_packetlines: bool, - Options { - fetch_refspecs, - prefix_from_spec_as_filter_on_remote, - extra_refspecs, - all_refspecs, - }: Options, + options: Options, ) -> Result where T: Transport, @@ -100,20 +113,7 @@ impl RefMap { transport, &handshake.capabilities, |_capabilities, arguments| { - if prefix_from_spec_as_filter_on_remote { - 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); - } - } - } - } + options.push_prefix_arguments(arguments); Ok(crate::ls_refs::Action::Continue) }, &mut progress, @@ -124,22 +124,17 @@ impl RefMap { } }; - Self::from_refs( - remote_refs, - &handshake.capabilities, + Self::from_refs(remote_refs, &handshake.capabilities, options) + } + + fn from_refs(remote_refs: Vec, capabilities: &Capabilities, options: Options) -> Result { + let Options { fetch_refspecs, - all_refspecs, extra_refspecs, - ) - } + all_refspecs, + .. + } = options; - fn from_refs( - remote_refs: Vec, - capabilities: &Capabilities, - fetch_refspecs: Vec, - all_refspecs: Vec, - extra_refspecs: Vec, - ) -> Result { let num_explicit_specs = fetch_refspecs.len(); let group = gix_refspec::MatchGroup::from_fetch_specs(all_refspecs.iter().map(gix_refspec::RefSpec::to_ref)); let null = gix_hash::ObjectId::null(gix_hash::Kind::Sha1); // OK to hardcode Sha1, it's not supposed to match, ever. From 19a005135b91641a19a3a70fd714985ce1203333 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sat, 1 Nov 2025 21:50:36 +0100 Subject: [PATCH 25/36] refactor!: hoist handshake ref handling out of transport code --- gitoxide-core/src/pack/receive.rs | 25 ++++++++++------ gix-protocol/src/fetch/refmap/init.rs | 40 ++++++++++++------------- gix/src/remote/connection/ref_map.rs | 42 +++++++++++++++++---------- 3 files changed, 61 insertions(+), 46 deletions(-) diff --git a/gitoxide-core/src/pack/receive.rs b/gitoxide-core/src/pack/receive.rs index 52ab85acb73..26ac31e5be4 100644 --- a/gitoxide-core/src/pack/receive.rs +++ b/gitoxide-core/src/pack/receive.rs @@ -82,15 +82,22 @@ where }) .collect::>()?; let user_agent = ("agent", Some(agent.clone().into())); - let refmap = gix::protocol::fetch::RefMap::new( - &mut progress, - &mut handshake, - &mut transport.inner, - user_agent.clone(), - trace_packetlines, - gix::protocol::fetch::refmap::init::Options::fetch(fetch_refspecs.clone()), - ) - .await?; + + let fetch_opts = gix::protocol::fetch::refmap::init::Options::fetch(fetch_refspecs.clone()); + let refmap = match handshake.refs.take() { + Some(refs) => gix::protocol::fetch::RefMap::from_refs(refs, &handshake.capabilities, fetch_opts)?, + None => { + gix::protocol::fetch::RefMap::new( + &mut progress, + &handshake.capabilities, + &mut transport.inner, + user_agent.clone(), + trace_packetlines, + fetch_opts, + ) + .await? + } + }; if refmap.mappings.is_empty() && !refmap.remote_refs.is_empty() { return Err(Error::NoMapping { diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index 74c38bef9c9..7a3d1287cac 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -13,7 +13,7 @@ use crate::{ refmap::{Mapping, Source, SpecIndex}, RefMap, }, - handshake::{Outcome, Ref}, + handshake::Ref, }; /// The error returned by [`RefMap::new()`]. @@ -96,7 +96,7 @@ impl RefMap { #[maybe_async::maybe_async] pub async fn new( mut progress: impl Progress, - handshake: &mut Outcome, + capabilities: &Capabilities, transport: &mut T, user_agent: (&'static str, Option>), trace_packetlines: bool, @@ -106,28 +106,24 @@ impl RefMap { T: Transport, { let _span = gix_trace::coarse!("gix_protocol::fetch::RefMap::new()"); - let remote_refs = match handshake.refs.take() { - Some(refs) => refs, - None => { - crate::ls_refs( - transport, - &handshake.capabilities, - |_capabilities, arguments| { - options.push_prefix_arguments(arguments); - Ok(crate::ls_refs::Action::Continue) - }, - &mut progress, - trace_packetlines, - user_agent, - ) - .await? - } - }; - - Self::from_refs(remote_refs, &handshake.capabilities, options) + let remote_refs = crate::ls_refs( + transport, + capabilities, + |_capabilities, arguments| { + options.push_prefix_arguments(arguments); + Ok(crate::ls_refs::Action::Continue) + }, + &mut progress, + trace_packetlines, + user_agent, + ) + .await?; + + Self::from_refs(remote_refs, capabilities, options) } - fn from_refs(remote_refs: Vec, capabilities: &Capabilities, options: Options) -> Result { + /// Create a ref-map from already obtained `remote_refs`. + pub fn from_refs(remote_refs: Vec, capabilities: &Capabilities, options: Options) -> Result { let Options { fetch_refspecs, extra_refspecs, diff --git a/gix/src/remote/connection/ref_map.rs b/gix/src/remote/connection/ref_map.rs index 3b218b0fa8b..f31ec7578d0 100644 --- a/gix/src/remote/connection/ref_map.rs +++ b/gix/src/remote/connection/ref_map.rs @@ -150,20 +150,32 @@ where &mut progress, ) .await?; - let refmap = gix_protocol::fetch::RefMap::new( - progress, - &mut handshake, - &mut self.transport.inner, - self.remote.repo.config.user_agent_tuple(), - self.trace, - gix_protocol::fetch::refmap::init::Options::new( - self.remote.fetch_specs.clone(), - prefix_from_spec_as_filter_on_remote, - extra_refspecs, - ), - ) - .await?; - self.handshake = Some(handshake); - Ok(refmap) + + let fetch_opts = gix_protocol::fetch::refmap::init::Options::new( + self.remote.fetch_specs.clone(), + prefix_from_spec_as_filter_on_remote, + extra_refspecs, + ); + + Ok(match handshake.refs.take() { + Some(refs) => { + let refmap = fetch::RefMap::from_refs(refs, &handshake.capabilities, fetch_opts)?; + self.handshake = Some(handshake); + refmap + } + None => { + let refmap = gix_protocol::fetch::RefMap::new( + progress, + &handshake.capabilities, + &mut self.transport.inner, + self.remote.repo.config.user_agent_tuple(), + self.trace, + fetch_opts, + ) + .await?; + self.handshake = Some(handshake); + refmap + } + }) } } From 48fdf5d9d32f45811eba9600f2a3f154c69ce5a9 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 4 Nov 2025 08:30:40 +0100 Subject: [PATCH 26/36] refactor --- gitoxide-core/src/pack/receive.rs | 12 ++- gix-protocol/src/fetch/mod.rs | 2 +- gix-protocol/src/fetch/negotiate.rs | 2 +- gix-protocol/src/fetch/refmap/init.rs | 114 ++++++++++++-------------- gix-protocol/src/fetch/types.rs | 6 +- gix-protocol/src/ls_refs.rs | 7 +- gix-transport/Cargo.toml | 2 - gix/src/remote/connection/ref_map.rs | 29 +++---- 8 files changed, 82 insertions(+), 92 deletions(-) diff --git a/gitoxide-core/src/pack/receive.rs b/gitoxide-core/src/pack/receive.rs index 26ac31e5be4..dcfb4ceef14 100644 --- a/gitoxide-core/src/pack/receive.rs +++ b/gitoxide-core/src/pack/receive.rs @@ -83,17 +83,21 @@ where .collect::>()?; let user_agent = ("agent", Some(agent.clone().into())); - let fetch_opts = gix::protocol::fetch::refmap::init::Options::fetch(fetch_refspecs.clone()); + let context = gix::protocol::fetch::refmap::init::Context { + fetch_refspecs: fetch_refspecs.clone(), + extra_refspecs: vec![], + }; let refmap = match handshake.refs.take() { - Some(refs) => gix::protocol::fetch::RefMap::from_refs(refs, &handshake.capabilities, fetch_opts)?, + Some(refs) => gix::protocol::fetch::RefMap::from_refs(refs, &handshake.capabilities, context)?, None => { - gix::protocol::fetch::RefMap::new( + gix::protocol::fetch::RefMap::fetch( &mut progress, &handshake.capabilities, &mut transport.inner, user_agent.clone(), trace_packetlines, - fetch_opts, + true, + context, ) .await? } diff --git a/gix-protocol/src/fetch/mod.rs b/gix-protocol/src/fetch/mod.rs index 99f8df15162..a273ab694a6 100644 --- a/gix-protocol/src/fetch/mod.rs +++ b/gix-protocol/src/fetch/mod.rs @@ -5,7 +5,7 @@ /// /// * [handshake](handshake()) /// * **ls-refs** -/// * [get available refs by refspecs](RefMap::new()) +/// * [get available refs by refspecs](RefMap::fetch()) /// * **fetch pack** /// * `negotiate` until a pack can be received (TBD) /// * [officially terminate the connection](crate::indicate_end_of_interaction()) diff --git a/gix-protocol/src/fetch/negotiate.rs b/gix-protocol/src/fetch/negotiate.rs index 1cd8ba9c666..7480aea9960 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::new()`]. +/// - The references known on the remote, as previously obtained with [`RefMap::fetch()`]. /// * `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 7a3d1287cac..0f53039d3a7 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -16,7 +16,7 @@ use crate::{ handshake::Ref, }; -/// The error returned by [`RefMap::new()`]. +/// The error returned by [`RefMap::fetch()`]. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { @@ -28,89 +28,60 @@ pub enum Error { ListRefs(#[from] crate::ls_refs::Error), } -/// For use in [`RefMap::new()`]. +/// For use in [`RefMap::fetch()`]. #[derive(Debug, Clone)] -pub struct Options { +pub struct Context { /// All explicit refspecs to identify references on the remote that you are interested in. /// Note that these are copied to [`RefMap::refspecs`] for convenience, as `RefMap::mappings` refer to them by index. - fetch_refspecs: Vec, - /// 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. Defaults to `true`. - prefix_from_spec_as_filter_on_remote: bool, + pub fetch_refspecs: Vec, /// A list of refspecs to use as implicit refspecs which won't be saved or otherwise be part of the remote in question. /// /// This is useful for handling `remote..tagOpt` for example. - extra_refspecs: Vec, - all_refspecs: Vec, + pub extra_refspecs: Vec, } -impl Options { - /// Create options to fetch the given `fetch_refspecs`. - pub fn fetch(fetch_refspecs: Vec) -> Self { - Self::new(fetch_refspecs, true, Vec::new()) - } - - /// Specify all options for fetching. - pub fn new( - fetch_refspecs: Vec, - prefix_from_spec_as_filter_on_remote: bool, - extra_refspecs: Vec, - ) -> Self { - let mut all_refspecs = fetch_refspecs.clone(); - all_refspecs.extend(extra_refspecs.iter().cloned()); - - Self { - all_refspecs, - fetch_refspecs: fetch_refspecs.clone(), - prefix_from_spec_as_filter_on_remote, - extra_refspecs, - } - } - - fn push_prefix_arguments(&self, arguments: &mut Vec) { - if !self.prefix_from_spec_as_filter_on_remote { - return; - } - - let mut seen = HashSet::new(); - for spec in &self.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); - } - } - } +impl Context { + fn aggregate_refspecs(&self) -> Vec { + let mut all_refspecs = self.fetch_refspecs.clone(); + all_refspecs.extend(self.extra_refspecs.iter().cloned()); + all_refspecs } } impl RefMap { - /// Create a new instance by obtaining all references on the remote that have been filtered through our remote's + /// 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 new( + pub async fn fetch( mut progress: impl Progress, capabilities: &Capabilities, transport: &mut T, user_agent: (&'static str, Option>), trace_packetlines: bool, - options: Options, + 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 remote_refs = crate::ls_refs( transport, capabilities, |_capabilities, arguments| { - options.push_prefix_arguments(arguments); + push_prefix_arguments(prefix_from_spec_as_filter_on_remote, arguments, &all_refspecs); Ok(crate::ls_refs::Action::Continue) }, &mut progress, @@ -119,18 +90,16 @@ impl RefMap { ) .await?; - Self::from_refs(remote_refs, capabilities, options) + Self::from_refs(remote_refs, capabilities, context) } - /// Create a ref-map from already obtained `remote_refs`. - pub fn from_refs(remote_refs: Vec, capabilities: &Capabilities, options: Options) -> Result { - let Options { + /// Create a ref-map from already obtained `remote_refs`. Use `context` to pass in refspecs. + pub fn from_refs(remote_refs: Vec, capabilities: &Capabilities, context: Context) -> Result { + let all_refspecs = context.aggregate_refspecs(); + let Context { fetch_refspecs, extra_refspecs, - all_refspecs, - .. - } = options; - + } = context; let num_explicit_specs = fetch_refspecs.len(); let group = gix_refspec::MatchGroup::from_fetch_specs(all_refspecs.iter().map(gix_refspec::RefSpec::to_ref)); let null = gix_hash::ObjectId::null(gix_hash::Kind::Sha1); // OK to hardcode Sha1, it's not supposed to match, ever. @@ -191,3 +160,26 @@ impl RefMap { }) } } + +fn push_prefix_arguments( + prefix_from_spec_as_filter_on_remote: bool, + arguments: &mut Vec, + all_refspecs: &[gix_refspec::RefSpec], +) { + if !prefix_from_spec_as_filter_on_remote { + return; + } + + 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/fetch/types.rs b/gix-protocol/src/fetch/types.rs index 0277ff098c8..e883ba55e3f 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::new()`] and [`fetch`](crate::fetch()). +/// For use in [`RefMap::fetch()`] 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::new()`] or the `fetch` call in [`fetch()`](crate::fetch()). + /// How to self-identify during the `ls-refs` call in [`RefMap::fetch()`] or the `fetch` call in [`fetch()`](crate::fetch()). /// /// This could be read from the `gitoxide.userAgent` configuration variable. pub user_agent: (&'static str, Option>), @@ -123,7 +123,7 @@ mod with_fetch { /// [`refmap::SpecIndex::ExplicitInRemote`] in [`refmap::Mapping`]. pub refspecs: Vec, /// Refspecs which have been added implicitly due to settings of the `remote`, usually pre-initialized from - /// [`extra_refspecs` in RefMap options](refmap::init::Options). + /// [`extra_refspecs` in RefMap options](refmap::init::Context). /// They are referred to by [`refmap::SpecIndex::Implicit`] in [`refmap::Mapping`]. /// /// They are never persisted nor are they typically presented to the user. diff --git a/gix-protocol/src/ls_refs.rs b/gix-protocol/src/ls_refs.rs index 949ae8f161a..d9fd8c95c5d 100644 --- a/gix-protocol/src/ls_refs.rs +++ b/gix-protocol/src/ls_refs.rs @@ -60,9 +60,10 @@ pub(crate) mod function { indicate_end_of_interaction, Command, }; - /// Invoke an ls-refs V2 command on `transport`, which requires a prior handshake that yielded - /// server `capabilities`. `prepare_ls_refs(capabilities, arguments, features)` can be used to alter the _ls-refs_. `progress` is used to provide feedback. - /// Note that `prepare_ls_refs()` is expected to add the `(agent, Some(name))` to the list of `features`. + /// 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_. + /// `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( diff --git a/gix-transport/Cargo.toml b/gix-transport/Cargo.toml index 507bc758aec..a73c202b379 100644 --- a/gix-transport/Cargo.toml +++ b/gix-transport/Cargo.toml @@ -18,8 +18,6 @@ doctest = false default = [] ## If set, blocking implementations of the typical git transports become available in `crate::client::blocking_io` -## -## If used in conjunction with an async implementation, this one takes precedence. blocking-client = ["gix-packetline/blocking-io"] ## Implies `blocking-client`, and adds support for the http and https transports. http-client = [ diff --git a/gix/src/remote/connection/ref_map.rs b/gix/src/remote/connection/ref_map.rs index f31ec7578d0..000f6bb8490 100644 --- a/gix/src/remote/connection/ref_map.rs +++ b/gix/src/remote/connection/ref_map.rs @@ -151,31 +151,26 @@ where ) .await?; - let fetch_opts = gix_protocol::fetch::refmap::init::Options::new( - self.remote.fetch_specs.clone(), - prefix_from_spec_as_filter_on_remote, + let context = gix_protocol::fetch::refmap::init::Context { + fetch_refspecs: self.remote.fetch_specs.clone(), extra_refspecs, - ); - - Ok(match handshake.refs.take() { - Some(refs) => { - let refmap = fetch::RefMap::from_refs(refs, &handshake.capabilities, fetch_opts)?; - self.handshake = Some(handshake); - refmap - } + }; + let ref_map = match handshake.refs.take() { + Some(refs) => fetch::RefMap::from_refs(refs, &handshake.capabilities, context)?, None => { - let refmap = gix_protocol::fetch::RefMap::new( + gix_protocol::fetch::RefMap::fetch( progress, &handshake.capabilities, &mut self.transport.inner, self.remote.repo.config.user_agent_tuple(), self.trace, - fetch_opts, + prefix_from_spec_as_filter_on_remote, + context, ) - .await?; - self.handshake = Some(handshake); - refmap + .await? } - }) + }; + self.handshake = Some(handshake); + Ok(ref_map) } } From 541a7a6aab6a56fa2949a5577acbacf6dc59a9dd Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 4 Nov 2025 10:46:30 +0100 Subject: [PATCH 27/36] refactor!: turn `handshake::Outcome` into `Handshake` type. --- gix-protocol/src/fetch/handshake.rs | 7 ++--- gix-protocol/src/fetch/response/mod.rs | 2 +- gix-protocol/src/fetch/types.rs | 2 +- gix-protocol/src/handshake/function.rs | 7 ++--- gix-protocol/src/handshake/mod.rs | 30 ++++++++++++---------- gix-protocol/src/lib.rs | 2 ++ gix-protocol/tests/protocol/fetch/_impl.rs | 2 +- 7 files changed, 28 insertions(+), 24 deletions(-) diff --git a/gix-protocol/src/fetch/handshake.rs b/gix-protocol/src/fetch/handshake.rs index 0c5dde26d62..83367d5778a 100644 --- a/gix-protocol/src/fetch/handshake.rs +++ b/gix-protocol/src/fetch/handshake.rs @@ -6,10 +6,7 @@ use maybe_async::maybe_async; use crate::transport::client::async_io::Transport; #[cfg(feature = "blocking-client")] use crate::transport::client::blocking_io::Transport; -use crate::{ - credentials, - handshake::{Error, Outcome}, -}; +use crate::{credentials, handshake::Error, Handshake}; /// Perform a handshake with the server on the other side of `transport`, with `authenticate` being used if authentication /// turns out to be required. `extra_parameters` are the parameters `(name, optional value)` to add to the handshake, @@ -22,7 +19,7 @@ pub async fn upload_pack( authenticate: AuthFn, extra_parameters: Vec<(String, Option)>, progress: &mut impl Progress, -) -> Result +) -> Result where AuthFn: FnMut(credentials::helper::Action) -> credentials::protocol::Result, T: Transport, diff --git a/gix-protocol/src/fetch/response/mod.rs b/gix-protocol/src/fetch/response/mod.rs index 1bc3fa163a1..1af0b49e5f6 100644 --- a/gix-protocol/src/fetch/response/mod.rs +++ b/gix-protocol/src/fetch/response/mod.rs @@ -185,7 +185,7 @@ impl Response { } /// Append the given `updates` which may have been obtained from a - /// (handshake::Outcome)[crate::handshake::Outcome::v1_shallow_updates]. + /// (handshake::Outcome)[crate::handshake::Handshake::v1_shallow_updates]. /// /// In V2, these are received as part of the pack, but V1 sends them early, so we /// offer to re-integrate them here. diff --git a/gix-protocol/src/fetch/types.rs b/gix-protocol/src/fetch/types.rs index e883ba55e3f..549451f4703 100644 --- a/gix-protocol/src/fetch/types.rs +++ b/gix-protocol/src/fetch/types.rs @@ -24,7 +24,7 @@ pub struct Context<'a, T> { /// The outcome of the handshake performed with the remote. /// /// Note that it's mutable as depending on the protocol, it may contain refs that have been sent unconditionally. - pub handshake: &'a mut crate::handshake::Outcome, + pub handshake: &'a mut crate::Handshake, /// The transport to use when making an `ls-refs` or `fetch` call. /// /// This is always done if the underlying protocol is V2, which is implied by the absence of refs in the `handshake` outcome. diff --git a/gix-protocol/src/handshake/function.rs b/gix-protocol/src/handshake/function.rs index a4b9261c38a..a4ee366b3d6 100644 --- a/gix-protocol/src/handshake/function.rs +++ b/gix-protocol/src/handshake/function.rs @@ -2,11 +2,12 @@ use gix_features::{progress, progress::Progress}; use gix_transport::{client, Service}; use maybe_async::maybe_async; -use super::{Error, Outcome}; +use super::Error; #[cfg(feature = "async-client")] use crate::transport::client::async_io::{SetServiceResponse, Transport}; #[cfg(feature = "blocking-client")] use crate::transport::client::blocking_io::{SetServiceResponse, Transport}; +use crate::Handshake; use crate::{credentials, handshake::refs}; /// Perform a handshake with the server on the other side of `transport`, with `authenticate` being used if authentication @@ -21,7 +22,7 @@ pub async fn handshake( mut authenticate: AuthFn, extra_parameters: Vec<(String, Option)>, progress: &mut impl Progress, -) -> Result +) -> Result where AuthFn: FnMut(credentials::helper::Action) -> credentials::protocol::Result, T: Transport, @@ -103,7 +104,7 @@ where .map(|(refs, shallow)| (Some(refs), Some(shallow))) .unwrap_or_default(); - Ok(Outcome { + Ok(Handshake { server_protocol_version, refs, v1_shallow_updates, diff --git a/gix-protocol/src/handshake/mod.rs b/gix-protocol/src/handshake/mod.rs index 64ba19aa0eb..222ae150f9a 100644 --- a/gix-protocol/src/handshake/mod.rs +++ b/gix-protocol/src/handshake/mod.rs @@ -50,20 +50,24 @@ pub enum Ref { }, } -/// The result of the [`handshake()`][super::handshake()] function. -#[derive(Default, Debug, Clone)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg(feature = "handshake")] -pub struct Outcome { - /// The protocol version the server responded with. It might have downgraded the desired version. - pub server_protocol_version: gix_transport::Protocol, - /// The references reported as part of the `Protocol::V1` handshake, or `None` otherwise as V2 requires a separate request. - pub refs: Option>, - /// Shallow updates as part of the `Protocol::V1`, to shallow a particular object. - /// Note that unshallowing isn't supported here. - pub v1_shallow_updates: Option>, - /// The server capabilities. - pub capabilities: gix_transport::client::Capabilities, +pub(crate) mod hero { + use crate::handshake::Ref; + + /// The result of the [`handshake()`](crate::handshake()) function. + #[derive(Default, Debug, Clone)] + #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] + pub struct Handshake { + /// The protocol version the server responded with. It might have downgraded the desired version. + pub server_protocol_version: gix_transport::Protocol, + /// The references reported as part of the `Protocol::V1` handshake, or `None` otherwise as V2 requires a separate request. + pub refs: Option>, + /// Shallow updates as part of the `Protocol::V1`, to shallow a particular object. + /// Note that unshallowing isn't supported here. + pub v1_shallow_updates: Option>, + /// The server capabilities. + pub capabilities: gix_transport::client::Capabilities, + } } #[cfg(feature = "handshake")] diff --git a/gix-protocol/src/lib.rs b/gix-protocol/src/lib.rs index 8c75512928d..9765b102d61 100644 --- a/gix-protocol/src/lib.rs +++ b/gix-protocol/src/lib.rs @@ -61,6 +61,8 @@ pub mod handshake; #[cfg(any(feature = "blocking-client", feature = "async-client"))] #[cfg(feature = "handshake")] pub use handshake::function::handshake; +#[cfg(feature = "handshake")] +pub use handshake::hero::Handshake; /// pub mod ls_refs; diff --git a/gix-protocol/tests/protocol/fetch/_impl.rs b/gix-protocol/tests/protocol/fetch/_impl.rs index 0496ec64f73..77866afda1b 100644 --- a/gix-protocol/tests/protocol/fetch/_impl.rs +++ b/gix-protocol/tests/protocol/fetch/_impl.rs @@ -74,7 +74,7 @@ mod fetch_fn { P: NestedProgress + 'static, P::SubProgress: 'static, { - let gix_protocol::handshake::Outcome { + let gix_protocol::Handshake { server_protocol_version: protocol_version, refs, v1_shallow_updates: _ignored_shallow_updates_as_it_is_deprecated, From a61b2abd74e112450788bfa6355dfa8239f360b6 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 4 Nov 2025 11:15:51 +0100 Subject: [PATCH 28/36] adapt to changes in `gix-protocol`: Outcome -> Handshake --- gix/src/remote/connection/fetch/mod.rs | 2 +- gix/src/remote/connection/mod.rs | 2 +- gix/src/remote/connection/ref_map.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gix/src/remote/connection/fetch/mod.rs b/gix/src/remote/connection/fetch/mod.rs index 91838be08b5..6efb6be4f73 100644 --- a/gix/src/remote/connection/fetch/mod.rs +++ b/gix/src/remote/connection/fetch/mod.rs @@ -76,7 +76,7 @@ pub struct Outcome { /// The result of the initial mapping of references, the prerequisite for any fetch. pub ref_map: RefMap, /// The outcome of the handshake with the server. - pub handshake: gix_protocol::handshake::Outcome, + pub handshake: gix_protocol::Handshake, /// The status of the operation to indicate what happened. pub status: Status, } diff --git a/gix/src/remote/connection/mod.rs b/gix/src/remote/connection/mod.rs index 9155a880ed2..c5322c519f0 100644 --- a/gix/src/remote/connection/mod.rs +++ b/gix/src/remote/connection/mod.rs @@ -20,7 +20,7 @@ where pub(crate) authenticate: Option>, pub(crate) transport_options: Option>, pub(crate) transport: gix_protocol::SendFlushOnDrop, - pub(crate) handshake: Option, + pub(crate) handshake: Option, pub(crate) trace: bool, } diff --git a/gix/src/remote/connection/ref_map.rs b/gix/src/remote/connection/ref_map.rs index 000f6bb8490..6a9509c0f95 100644 --- a/gix/src/remote/connection/ref_map.rs +++ b/gix/src/remote/connection/ref_map.rs @@ -92,7 +92,7 @@ where mut self, progress: impl Progress, options: Options, - ) -> Result<(fetch::RefMap, gix_protocol::handshake::Outcome), Error> { + ) -> Result<(fetch::RefMap, gix_protocol::Handshake), Error> { let refmap = self.ref_map_by_ref(progress, options).await?; let handshake = self .handshake From aeee98234002f0cf79bd3629ada5d3ec3d7b86cd Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 4 Nov 2025 11:17:08 +0100 Subject: [PATCH 29/36] feat: add `Handshake::ref_map()` to produce a ref-map from a V1 or V2 handshake. --- gitoxide-core/src/pack/receive.rs | 30 +++++++---------- gix-protocol/src/fetch/refmap/init.rs | 1 + gix-protocol/src/fetch/response/mod.rs | 2 +- gix-protocol/src/handshake/mod.rs | 46 ++++++++++++++++++++++++++ gix/src/remote/connection/ref_map.rs | 29 +++++++--------- 5 files changed, 72 insertions(+), 36 deletions(-) diff --git a/gitoxide-core/src/pack/receive.rs b/gitoxide-core/src/pack/receive.rs index dcfb4ceef14..2a1ccea79f6 100644 --- a/gitoxide-core/src/pack/receive.rs +++ b/gitoxide-core/src/pack/receive.rs @@ -4,6 +4,7 @@ use std::{ sync::{atomic::AtomicBool, Arc}, }; +use crate::{net, pack::receive::protocol::fetch::negotiate, OutputFormat}; #[cfg(feature = "async-client")] use gix::protocol::transport::client::async_io::connect; #[cfg(feature = "blocking-client")] @@ -23,8 +24,6 @@ pub use gix::{ NestedProgress, Progress, }; -use crate::{net, pack::receive::protocol::fetch::negotiate, OutputFormat}; - pub const PROGRESS_RANGE: std::ops::RangeInclusive = 1..=3; pub struct Context { pub thread_limit: Option, @@ -65,7 +64,7 @@ where .is_some(); let agent = gix::protocol::agent(gix::env::agent()); - let mut handshake = gix::protocol::fetch::handshake( + let mut handshake: gix::protocol::Handshake = gix::protocol::fetch::handshake( &mut transport.inner, gix::protocol::credentials::builtin, vec![("agent".into(), Some(agent.clone()))], @@ -87,21 +86,16 @@ where fetch_refspecs: fetch_refspecs.clone(), extra_refspecs: vec![], }; - let refmap = match handshake.refs.take() { - Some(refs) => gix::protocol::fetch::RefMap::from_refs(refs, &handshake.capabilities, context)?, - None => { - gix::protocol::fetch::RefMap::fetch( - &mut progress, - &handshake.capabilities, - &mut transport.inner, - user_agent.clone(), - trace_packetlines, - true, - context, - ) - .await? - } - }; + let refmap = handshake + .fetch_or_extract_refmap( + &mut progress, + &mut transport.inner, + user_agent.clone(), + trace_packetlines, + true, + context, + ) + .await?; if refmap.mappings.is_empty() && !refmap.remote_refs.is_empty() { return Err(Error::NoMapping { diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index 0f53039d3a7..abcf027cfb1 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -94,6 +94,7 @@ impl RefMap { } /// 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 { let all_refspecs = context.aggregate_refspecs(); let Context { diff --git a/gix-protocol/src/fetch/response/mod.rs b/gix-protocol/src/fetch/response/mod.rs index 1af0b49e5f6..804e07f7d4a 100644 --- a/gix-protocol/src/fetch/response/mod.rs +++ b/gix-protocol/src/fetch/response/mod.rs @@ -185,7 +185,7 @@ impl Response { } /// Append the given `updates` which may have been obtained from a - /// (handshake::Outcome)[crate::handshake::Handshake::v1_shallow_updates]. + /// (handshake::Outcome)[crate::Handshake::v1_shallow_updates]. /// /// In V2, these are received as part of the pack, but V1 sends them early, so we /// offer to re-integrate them here. diff --git a/gix-protocol/src/handshake/mod.rs b/gix-protocol/src/handshake/mod.rs index 222ae150f9a..2a6a08911cc 100644 --- a/gix-protocol/src/handshake/mod.rs +++ b/gix-protocol/src/handshake/mod.rs @@ -68,6 +68,52 @@ pub(crate) mod hero { /// The server capabilities. pub capabilities: gix_transport::client::Capabilities, } + + #[cfg(feature = "fetch")] + mod fetch { + #[cfg(feature = "async-client")] + use crate::transport::client::async_io::Transport; + #[cfg(feature = "blocking-client")] + use crate::transport::client::blocking_io::Transport; + use crate::Handshake; + 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. + #[allow(clippy::result_large_err)] + #[maybe_async::maybe_async] + pub async fn fetch_or_extract_refmap( + &mut self, + mut progress: impl Progress, + transport: &mut T, + user_agent: (&'static str, Option>), + trace_packetlines: bool, + prefix_from_spec_as_filter_on_remote: bool, + refmap_context: crate::fetch::refmap::init::Context, + ) -> Result + 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? + } + }) + } + } + } } #[cfg(feature = "handshake")] diff --git a/gix/src/remote/connection/ref_map.rs b/gix/src/remote/connection/ref_map.rs index 6a9509c0f95..51b550a7e8a 100644 --- a/gix/src/remote/connection/ref_map.rs +++ b/gix/src/remote/connection/ref_map.rs @@ -143,7 +143,7 @@ where if let Some(config) = self.transport_options.as_ref() { self.transport.inner.configure(&**config)?; } - let mut handshake = gix_protocol::fetch::handshake( + let mut handshake: gix_protocol::Handshake = gix_protocol::fetch::handshake( &mut self.transport.inner, authenticate, handshake_parameters, @@ -151,25 +151,20 @@ where ) .await?; - let context = gix_protocol::fetch::refmap::init::Context { + let context = fetch::refmap::init::Context { fetch_refspecs: self.remote.fetch_specs.clone(), extra_refspecs, }; - let ref_map = match handshake.refs.take() { - Some(refs) => fetch::RefMap::from_refs(refs, &handshake.capabilities, context)?, - None => { - gix_protocol::fetch::RefMap::fetch( - progress, - &handshake.capabilities, - &mut self.transport.inner, - self.remote.repo.config.user_agent_tuple(), - self.trace, - prefix_from_spec_as_filter_on_remote, - context, - ) - .await? - } - }; + 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, + ) + .await?; self.handshake = Some(handshake); Ok(ref_map) } From 37e7e022f6c008882d44c16353958bad768230ff Mon Sep 17 00:00:00 2001 From: iczero Date: Thu, 6 Nov 2025 17:05:28 -0800 Subject: [PATCH 30/36] move worktree_branches call out of loop --- gix/src/remote/connection/fetch/update_refs/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gix/src/remote/connection/fetch/update_refs/mod.rs b/gix/src/remote/connection/fetch/update_refs/mod.rs index e33cfe2151a..64200f98805 100644 --- a/gix/src/remote/connection/fetch/update_refs/mod.rs +++ b/gix/src/remote/connection/fetch/update_refs/mod.rs @@ -76,6 +76,7 @@ pub(crate) fn update( let mut updates = Vec::new(); let mut edit_indices_to_validate = Vec::new(); + let mut checked_out_branches = worktree_branches(repo)?; let implicit_tag_refspec = fetch_tags .to_refspec() .filter(|_| matches!(fetch_tags, crate::remote::fetch::Tags::Included)); @@ -110,7 +111,6 @@ pub(crate) fn update( continue; } } - let mut checked_out_branches = worktree_branches(repo)?; let (mode, edit_index, type_change) = match local { Some(name) => { let (mode, reflog_message, name, previous_value) = match repo.try_find_reference(name)? { From a004329195ff9dcbe11e6d52195ff30536fe7a16 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 8 Nov 2025 12:05:43 +0100 Subject: [PATCH 31/36] feat: add `Repository::worktree_proxy_by_id(id)`. That way it's more straightforward to obtain information about worktrees which are known by ID. --- gix/src/repository/worktree.rs | 18 ++++++++++++------ gix/src/worktree/proxy.rs | 9 +++++++++ gix/tests/gix/repository/worktree.rs | 24 ++++++++++++++++++++++-- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/gix/src/repository/worktree.rs b/gix/src/repository/worktree.rs index db395c01dc3..5dc76603494 100644 --- a/gix/src/repository/worktree.rs +++ b/gix/src/repository/worktree.rs @@ -1,3 +1,4 @@ +use crate::bstr::BStr; use crate::{worktree, Worktree}; /// Interact with individual worktrees and their information. @@ -20,17 +21,22 @@ impl crate::Repository { for entry in iter { let entry = entry?; let worktree_git_dir = entry.path(); - if worktree_git_dir.join("gitdir").is_file() { - res.push(worktree::Proxy { - parent: self, - git_dir: worktree_git_dir, - }); - } + res.extend(worktree::Proxy::new_if_gitdir_file_exists(self, worktree_git_dir)); } res.sort_by(|a, b| a.git_dir.cmp(&b.git_dir)); Ok(res) } + /// Return the worktree that [is identified](Worktree::id) by the given `id`, if it exists at + /// `.git/worktrees/` and its `gitdir` file exists. + /// Return `None` otherwise. + pub fn worktree_proxy_by_id<'a>(&self, id: impl Into<&'a BStr>) -> Option> { + worktree::Proxy::new_if_gitdir_file_exists( + self, + self.common_dir().join("worktrees").join(gix_path::from_bstr(id.into())), + ) + } + /// Return the repository owning the main worktree, typically from a linked worktree. /// /// Note that it might be the one that is currently open if this repository doesn't point to a linked worktree. diff --git a/gix/src/worktree/proxy.rs b/gix/src/worktree/proxy.rs index e4b6672774c..b6f1b423e00 100644 --- a/gix/src/worktree/proxy.rs +++ b/gix/src/worktree/proxy.rs @@ -31,6 +31,15 @@ impl<'repo> Proxy<'repo> { git_dir: git_dir.into(), } } + + pub(crate) fn new_if_gitdir_file_exists(parent: &'repo Repository, git_dir: impl Into) -> Option { + let git_dir = git_dir.into(); + if git_dir.join("gitdir").is_file() { + Some(Proxy::new(parent, git_dir)) + } else { + None + } + } } impl Proxy<'_> { diff --git a/gix/tests/gix/repository/worktree.rs b/gix/tests/gix/repository/worktree.rs index 9ab8eb95c8c..773d875f2f1 100644 --- a/gix/tests/gix/repository/worktree.rs +++ b/gix/tests/gix/repository/worktree.rs @@ -350,8 +350,14 @@ fn run_assertions(main_repo: gix::Repository, should_be_bare: bool) { "in our case prunable repos have no worktree base" ); + assert_eq!( + main_repo.worktree_proxy_by_id(actual.id()).expect("exists").git_dir(), + actual.git_dir(), + "we can basically get the same proxy by its ID explicitly" + ); + let repo = if base.is_dir() { - let repo = actual.into_repo().unwrap(); + let repo = actual.clone().into_repo().unwrap(); assert_eq!( &gix::open(base).unwrap(), &repo, @@ -366,7 +372,7 @@ fn run_assertions(main_repo: gix::Repository, should_be_bare: bool) { ), "missing bases are detected" ); - actual.into_repo_with_possibly_inaccessible_worktree().unwrap() + actual.clone().into_repo_with_possibly_inaccessible_worktree().unwrap() }; let worktree = repo.worktree().expect("linked worktrees have at least a base path"); assert!(!worktree.is_main()); @@ -378,5 +384,19 @@ fn run_assertions(main_repo: gix::Repository, should_be_bare: bool) { main_repo, "main repo from worktree repo is the actual main repo" ); + + let proxy_by_id = repo + .worktree_proxy_by_id(actual.id()) + .expect("can get the proxy from a linked repo as well"); + assert_ne!( + proxy_by_id.git_dir(), + actual.git_dir(), + "The git directories might not look the same…" + ); + assert_eq!( + gix_path::realpath(proxy_by_id.git_dir()).ok(), + gix_path::realpath(actual.git_dir()).ok(), + "…but they are the same effectively" + ); } } From 70fcdb7ac53070c6f8577092dbe593f6acaf05b0 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 8 Nov 2025 12:34:04 +0100 Subject: [PATCH 32/36] Thanks clippy --- gix-macros/src/momo.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gix-macros/src/momo.rs b/gix-macros/src/momo.rs index a007c1b2082..f8b8047dab4 100644 --- a/gix-macros/src/momo.rs +++ b/gix-macros/src/momo.rs @@ -184,13 +184,13 @@ fn convert( let pat_ident = match &mut *pat_type.pat { Pat::Ident(pat_ident) if pat_ident.by_ref.is_none() && pat_ident.subpat.is_none() => pat_ident, _ => { - pat_type.pat = Box::new(Pat::Ident(PatIdent { + *pat_type.pat = Pat::Ident(PatIdent { ident: Ident::new(&format!("arg_{i}_gen_by_momo_"), proc_macro2::Span::call_site()), attrs: Default::default(), by_ref: None, mutability: None, subpat: None, - })); + }); if let Pat::Ident(pat_ident) = &mut *pat_type.pat { pat_ident From 36c2be80e61a7a0ef637a5385e1960de13261984 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 7 Nov 2025 17:19:10 +0100 Subject: [PATCH 33/36] refactor!: simplify gix-protocol handshake API by avoiding duplicates that remove the `server` parameter. --- gitoxide-core/src/pack/receive.rs | 2 +- gix-protocol/src/fetch/handshake.rs | 28 ---------------------- gix-protocol/src/fetch/mod.rs | 7 ------ gix-protocol/src/handshake/function.rs | 2 +- gix-protocol/src/handshake/mod.rs | 2 +- gix-protocol/tests/protocol/fetch/_impl.rs | 2 +- gix/src/remote/connection/ref_map.rs | 2 +- 7 files changed, 5 insertions(+), 40 deletions(-) delete mode 100644 gix-protocol/src/fetch/handshake.rs diff --git a/gitoxide-core/src/pack/receive.rs b/gitoxide-core/src/pack/receive.rs index 2a1ccea79f6..9b048966c39 100644 --- a/gitoxide-core/src/pack/receive.rs +++ b/gitoxide-core/src/pack/receive.rs @@ -64,7 +64,7 @@ where .is_some(); let agent = gix::protocol::agent(gix::env::agent()); - let mut handshake: gix::protocol::Handshake = gix::protocol::fetch::handshake( + let mut handshake = gix::protocol::handshake( &mut transport.inner, gix::protocol::credentials::builtin, vec![("agent".into(), Some(agent.clone()))], diff --git a/gix-protocol/src/fetch/handshake.rs b/gix-protocol/src/fetch/handshake.rs deleted file mode 100644 index 83367d5778a..00000000000 --- a/gix-protocol/src/fetch/handshake.rs +++ /dev/null @@ -1,28 +0,0 @@ -use gix_features::progress::Progress; -use gix_transport::Service; -use maybe_async::maybe_async; - -#[cfg(feature = "async-client")] -use crate::transport::client::async_io::Transport; -#[cfg(feature = "blocking-client")] -use crate::transport::client::blocking_io::Transport; -use crate::{credentials, handshake::Error, Handshake}; - -/// Perform a handshake with the server on the other side of `transport`, with `authenticate` being used if authentication -/// turns out to be required. `extra_parameters` are the parameters `(name, optional value)` to add to the handshake, -/// each time it is performed in case authentication is required. -/// `progress` is used to inform about what's currently happening. -#[allow(clippy::result_large_err)] -#[maybe_async] -pub async fn upload_pack( - transport: T, - authenticate: AuthFn, - extra_parameters: Vec<(String, Option)>, - progress: &mut impl Progress, -) -> Result -where - AuthFn: FnMut(credentials::helper::Action) -> credentials::protocol::Result, - T: Transport, -{ - crate::handshake(transport, Service::UploadPack, authenticate, extra_parameters, progress).await -} diff --git a/gix-protocol/src/fetch/mod.rs b/gix-protocol/src/fetch/mod.rs index a273ab694a6..1f8eb8445eb 100644 --- a/gix-protocol/src/fetch/mod.rs +++ b/gix-protocol/src/fetch/mod.rs @@ -34,13 +34,6 @@ pub mod response; #[cfg(feature = "fetch")] pub(crate) mod function; -#[cfg(any(feature = "blocking-client", feature = "async-client"))] -#[cfg(feature = "handshake")] -mod handshake; -#[cfg(any(feature = "blocking-client", feature = "async-client"))] -#[cfg(feature = "handshake")] -pub use handshake::upload_pack as handshake; - #[cfg(feature = "fetch")] pub mod negotiate; diff --git a/gix-protocol/src/handshake/function.rs b/gix-protocol/src/handshake/function.rs index a4ee366b3d6..ea2835f9dd3 100644 --- a/gix-protocol/src/handshake/function.rs +++ b/gix-protocol/src/handshake/function.rs @@ -18,7 +18,6 @@ use crate::{credentials, handshake::refs}; #[maybe_async] pub async fn handshake( mut transport: T, - service: Service, mut authenticate: AuthFn, extra_parameters: Vec<(String, Option)>, progress: &mut impl Progress, @@ -27,6 +26,7 @@ where AuthFn: FnMut(credentials::helper::Action) -> credentials::protocol::Result, T: Transport, { + let service = Service::UploadPack; let _span = gix_features::trace::detail!("gix_protocol::handshake()", service = ?service, extra_parameters = ?extra_parameters); let (server_protocol_version, refs, capabilities) = { progress.init(None, progress::steps()); diff --git a/gix-protocol/src/handshake/mod.rs b/gix-protocol/src/handshake/mod.rs index 2a6a08911cc..31128063a52 100644 --- a/gix-protocol/src/handshake/mod.rs +++ b/gix-protocol/src/handshake/mod.rs @@ -123,7 +123,7 @@ mod error { use crate::{credentials, handshake::refs}; - /// The error returned by [`handshake()`][crate::fetch::handshake()]. + /// The error returned by [`handshake()`][crate::handshake()]. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { diff --git a/gix-protocol/tests/protocol/fetch/_impl.rs b/gix-protocol/tests/protocol/fetch/_impl.rs index 77866afda1b..a2d1e4623d7 100644 --- a/gix-protocol/tests/protocol/fetch/_impl.rs +++ b/gix-protocol/tests/protocol/fetch/_impl.rs @@ -79,7 +79,7 @@ mod fetch_fn { refs, v1_shallow_updates: _ignored_shallow_updates_as_it_is_deprecated, capabilities, - } = gix_protocol::fetch::handshake( + } = gix_protocol::handshake( &mut transport, authenticate, delegate.handshake_extra_parameters(), diff --git a/gix/src/remote/connection/ref_map.rs b/gix/src/remote/connection/ref_map.rs index 51b550a7e8a..f1f6649f212 100644 --- a/gix/src/remote/connection/ref_map.rs +++ b/gix/src/remote/connection/ref_map.rs @@ -143,7 +143,7 @@ where if let Some(config) = self.transport_options.as_ref() { self.transport.inner.configure(&**config)?; } - let mut handshake: gix_protocol::Handshake = gix_protocol::fetch::handshake( + let mut handshake = gix_protocol::handshake( &mut self.transport.inner, authenticate, handshake_parameters, From c9a97dbe35d1447319fd953062c465cd1c45d9b6 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 9 Nov 2025 11:07:31 +0100 Subject: [PATCH 34/36] Bring the `service` parameter back to not hardcode the handshake for fetches. --- gitoxide-core/src/pack/receive.rs | 1 + gix-protocol/src/handshake/function.rs | 3 ++- gix-protocol/tests/protocol/fetch/_impl.rs | 1 + gix/src/remote/connection/ref_map.rs | 1 + 4 files changed, 5 insertions(+), 1 deletion(-) diff --git a/gitoxide-core/src/pack/receive.rs b/gitoxide-core/src/pack/receive.rs index 9b048966c39..0be29d7afd0 100644 --- a/gitoxide-core/src/pack/receive.rs +++ b/gitoxide-core/src/pack/receive.rs @@ -66,6 +66,7 @@ where let agent = gix::protocol::agent(gix::env::agent()); let mut handshake = gix::protocol::handshake( &mut transport.inner, + transport::Service::UploadPack, gix::protocol::credentials::builtin, vec![("agent".into(), Some(agent.clone()))], &mut progress, diff --git a/gix-protocol/src/handshake/function.rs b/gix-protocol/src/handshake/function.rs index ea2835f9dd3..3eac8f50d3e 100644 --- a/gix-protocol/src/handshake/function.rs +++ b/gix-protocol/src/handshake/function.rs @@ -14,10 +14,12 @@ use crate::{credentials, handshake::refs}; /// turns out to be required. `extra_parameters` are the parameters `(name, optional value)` to add to the handshake, /// each time it is performed in case authentication is required. /// `progress` is used to inform about what's currently happening. +/// The `service` tells the server whether to be in 'send' or 'receive' mode. #[allow(clippy::result_large_err)] #[maybe_async] pub async fn handshake( mut transport: T, + service: Service, mut authenticate: AuthFn, extra_parameters: Vec<(String, Option)>, progress: &mut impl Progress, @@ -26,7 +28,6 @@ where AuthFn: FnMut(credentials::helper::Action) -> credentials::protocol::Result, T: Transport, { - let service = Service::UploadPack; let _span = gix_features::trace::detail!("gix_protocol::handshake()", service = ?service, extra_parameters = ?extra_parameters); let (server_protocol_version, refs, capabilities) = { progress.init(None, progress::steps()); diff --git a/gix-protocol/tests/protocol/fetch/_impl.rs b/gix-protocol/tests/protocol/fetch/_impl.rs index a2d1e4623d7..c4484fda920 100644 --- a/gix-protocol/tests/protocol/fetch/_impl.rs +++ b/gix-protocol/tests/protocol/fetch/_impl.rs @@ -81,6 +81,7 @@ mod fetch_fn { capabilities, } = gix_protocol::handshake( &mut transport, + gix_transport::Service::UploadPack, authenticate, delegate.handshake_extra_parameters(), &mut progress, diff --git a/gix/src/remote/connection/ref_map.rs b/gix/src/remote/connection/ref_map.rs index f1f6649f212..e149705dc6b 100644 --- a/gix/src/remote/connection/ref_map.rs +++ b/gix/src/remote/connection/ref_map.rs @@ -145,6 +145,7 @@ where } let mut handshake = gix_protocol::handshake( &mut self.transport.inner, + gix_transport::Service::UploadPack, authenticate, handshake_parameters, &mut progress, From 924cd62f7ef74f479114c80cee0db0ba5f522579 Mon Sep 17 00:00:00 2001 From: ralphmodales Date: Tue, 11 Nov 2025 03:54:24 +0800 Subject: [PATCH 35/36] feat: allow credential fill with `gix credential fill` to run without a repo (#2198) --- gitoxide-core/src/repository/credential.rs | 25 ++++++++++++++++++---- src/plumbing/main.rs | 2 +- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/gitoxide-core/src/repository/credential.rs b/gitoxide-core/src/repository/credential.rs index 9684250bd4d..a5648b3d8e0 100644 --- a/gitoxide-core/src/repository/credential.rs +++ b/gitoxide-core/src/repository/credential.rs @@ -6,9 +6,11 @@ enum Error { Configuration(#[from] gix::config::credential_helpers::Error), #[error(transparent)] Protocol(#[from] gix::credentials::protocol::Error), + #[error(transparent)] + ConfigLoad(#[from] gix::config::file::init::from_paths::Error), } -pub fn function(repo: gix::Repository, action: gix::credentials::program::main::Action) -> anyhow::Result<()> { +pub fn function(repo: Option, action: gix::credentials::program::main::Action) -> anyhow::Result<()> { use gix::credentials::program::main::Action::*; gix::credentials::program::main( Some(action.as_str().into()), @@ -20,9 +22,24 @@ pub fn function(repo: gix::Repository, action: gix::credentials::program::main:: .clone() .or_else(|| context.to_url()) .ok_or(Error::Protocol(gix::credentials::protocol::Error::UrlMissing))?; - let (mut cascade, _action, prompt_options) = repo - .config_snapshot() - .credential_helpers(gix::url::parse(url.as_ref())?)?; + + let (mut cascade, _action, prompt_options) = match repo { + Some(ref repo) => repo + .config_snapshot() + .credential_helpers(gix::url::parse(url.as_ref())?)?, + None => { + let config = gix::config::File::from_globals()?; + let environment = gix::open::permissions::Environment::all(); + gix::config::credential_helpers( + gix::url::parse(url.as_ref())?, + &config, + false, + |_| true, + environment, + false, + )? + } + }; cascade .invoke( match action { diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index 4b62fa626c2..1ff9821d904 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -658,7 +658,7 @@ pub fn main() -> Result<()> { } Subcommands::ConfigTree => show_progress(), Subcommands::Credential(cmd) => core::repository::credential( - repository(Mode::StrictWithGitInstallConfig)?, + repository(Mode::StrictWithGitInstallConfig).ok(), match cmd { credential::Subcommands::Fill => gix::credentials::program::main::Action::Get, credential::Subcommands::Approve => gix::credentials::program::main::Action::Store, From 41b4a8f74a2310d5c8c02918dc6e842e33eac0e5 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 11 Nov 2025 04:55:30 +0100 Subject: [PATCH 36/36] refactor --- gitoxide-core/src/repository/credential.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gitoxide-core/src/repository/credential.rs b/gitoxide-core/src/repository/credential.rs index a5648b3d8e0..451e453b7c9 100644 --- a/gitoxide-core/src/repository/credential.rs +++ b/gitoxide-core/src/repository/credential.rs @@ -33,10 +33,10 @@ pub fn function(repo: Option, action: gix::credentials::program gix::config::credential_helpers( gix::url::parse(url.as_ref())?, &config, - false, - |_| true, + false, /* lenient config */ + |_| true, /* section filter */ environment, - false, + false, /* use http path (override, uses configuration now)*/ )? } };