From b6d58225f6760db7a4b3c47c8d72e346d6295ec9 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 22 Oct 2025 14:31:05 +0200 Subject: [PATCH] fix!: move TransportWithoutIO::request() method to I/O-specific traits --- .../tests/protocol/fetch/arguments.rs | 36 +++---- gix-transport/src/client/async_io/traits.rs | 32 +++++- gix-transport/src/client/blocking_io/file.rs | 24 ++--- .../src/client/blocking_io/http/mod.rs | 100 +++++++++--------- .../src/client/blocking_io/traits.rs | 32 +++++- gix-transport/src/client/git/async_io.rs | 30 +++--- gix-transport/src/client/git/blocking_io.rs | 30 +++--- gix-transport/src/client/traits.rs | 38 ------- 8 files changed, 172 insertions(+), 150 deletions(-) diff --git a/gix-protocol/tests/protocol/fetch/arguments.rs b/gix-protocol/tests/protocol/fetch/arguments.rs index 2dd9a982b9e..9debb359557 100644 --- a/gix-protocol/tests/protocol/fetch/arguments.rs +++ b/gix-protocol/tests/protocol/fetch/arguments.rs @@ -34,15 +34,6 @@ mod impls { self.inner.set_identity(identity) } - fn request( - &mut self, - write_mode: WriteMode, - on_into_read: MessageKind, - trace: bool, - ) -> Result, Error> { - self.inner.request(write_mode, on_into_read, trace) - } - fn to_url(&self) -> Cow<'_, BStr> { self.inner.to_url() } @@ -71,6 +62,15 @@ mod impls { ) -> Result, Error> { self.inner.handshake(service, extra_parameters) } + + fn request( + &mut self, + write_mode: WriteMode, + on_into_read: MessageKind, + trace: bool, + ) -> Result, Error> { + self.inner.request(write_mode, on_into_read, trace) + } } } @@ -92,15 +92,6 @@ mod impls { self.inner.set_identity(identity) } - fn request( - &mut self, - write_mode: WriteMode, - on_into_read: MessageKind, - trace: bool, - ) -> Result, Error> { - self.inner.request(write_mode, on_into_read, trace) - } - fn to_url(&self) -> Cow<'_, BStr> { self.inner.to_url() } @@ -130,6 +121,15 @@ mod impls { ) -> Result, Error> { self.inner.handshake(service, extra_parameters).await } + + fn request( + &mut self, + write_mode: WriteMode, + on_into_read: MessageKind, + trace: bool, + ) -> Result, Error> { + self.inner.request(write_mode, on_into_read, trace) + } } } diff --git a/gix-transport/src/client/async_io/traits.rs b/gix-transport/src/client/async_io/traits.rs index 592c0b971c5..574e83893e7 100644 --- a/gix-transport/src/client/async_io/traits.rs +++ b/gix-transport/src/client/async_io/traits.rs @@ -6,7 +6,7 @@ use futures_lite::io::AsyncWriteExt; use crate::{ client::{ - async_io::{ExtendedBufRead, ReadlineBufRead}, + async_io::{request::RequestWriter, ExtendedBufRead, ReadlineBufRead}, Capabilities, Error, MessageKind, TransportWithoutIO, WriteMode, }, Protocol, Service, @@ -45,6 +45,18 @@ pub trait Transport: TransportWithoutIO { service: Service, extra_parameters: &'a [(&'a str, Option<&'a str>)], ) -> Result, Error>; + + /// Get a writer for sending data and obtaining the response. It can be configured in various ways + /// to support the task at hand. + /// `write_mode` determines how calls to the `write(…)` method are interpreted, and `on_into_read` determines + /// which message to write when the writer is turned into the response reader using [`into_read()`][RequestWriter::into_read()]. + /// If `trace` is `true`, then all packetlines written and received will be traced using facilities provided by the `gix_trace` crate. + fn request( + &mut self, + write_mode: WriteMode, + on_into_read: MessageKind, + trace: bool, + ) -> Result, Error>; } // Would be nice if the box implementation could auto-forward to all implemented traits. @@ -57,6 +69,15 @@ impl Transport for Box { ) -> Result, Error> { self.deref_mut().handshake(service, extra_parameters).await } + + fn request( + &mut self, + write_mode: WriteMode, + on_into_read: MessageKind, + trace: bool, + ) -> Result, Error> { + self.deref_mut().request(write_mode, on_into_read, trace) + } } // Would be nice if the box implementation could auto-forward to all implemented traits. @@ -69,6 +90,15 @@ impl Transport for &mut T { ) -> Result, Error> { self.deref_mut().handshake(service, extra_parameters).await } + + fn request( + &mut self, + write_mode: WriteMode, + on_into_read: MessageKind, + trace: bool, + ) -> Result, Error> { + self.deref_mut().request(write_mode, on_into_read, trace) + } } /// An extension trait to add more methods to everything implementing [`Transport`]. diff --git a/gix-transport/src/client/blocking_io/file.rs b/gix-transport/src/client/blocking_io/file.rs index a83f83554bb..520e160f847 100644 --- a/gix-transport/src/client/blocking_io/file.rs +++ b/gix-transport/src/client/blocking_io/file.rs @@ -108,18 +108,6 @@ impl client::TransportWithoutIO for SpawnProcessOnDemand { } } - fn request( - &mut self, - write_mode: WriteMode, - on_into_read: MessageKind, - trace: bool, - ) -> Result, client::Error> { - self.connection - .as_mut() - .ok_or(client::Error::MissingHandshake)? - .request(write_mode, on_into_read, trace) - } - fn to_url(&self) -> Cow<'_, BStr> { Cow::Owned(self.url.to_bstring()) } @@ -281,6 +269,18 @@ impl client::blocking_io::Transport for SpawnProcessOnDemand { .expect("connection to be there right after setting it") .handshake(service, extra_parameters) } + + fn request( + &mut self, + write_mode: WriteMode, + on_into_read: MessageKind, + trace: bool, + ) -> Result, client::Error> { + self.connection + .as_mut() + .ok_or(client::Error::MissingHandshake)? + .request(write_mode, on_into_read, trace) + } } /// Connect to a locally readable repository at `path` using the given `desired_version`. diff --git a/gix-transport/src/client/blocking_io/http/mod.rs b/gix-transport/src/client/blocking_io/http/mod.rs index 154aa89fd4a..22572436420 100644 --- a/gix-transport/src/client/blocking_io/http/mod.rs +++ b/gix-transport/src/client/blocking_io/http/mod.rs @@ -336,56 +336,6 @@ impl client::TransportWithoutIO for Transport { Ok(()) } - fn request( - &mut self, - write_mode: client::WriteMode, - on_into_read: MessageKind, - trace: bool, - ) -> Result, client::Error> { - let service = self.service.ok_or(client::Error::MissingHandshake)?; - let url = append_url(&self.url, service.as_str()); - let static_headers = &[ - Cow::Borrowed(self.user_agent_header), - Cow::Owned(format!("Content-Type: application/x-{}-request", service.as_str())), - format!("Accept: application/x-{}-result", service.as_str()).into(), - ]; - let mut dynamic_headers = Vec::new(); - self.add_basic_auth_if_present(&mut dynamic_headers)?; - if self.actual_version != Protocol::V1 { - dynamic_headers.push(Cow::Owned(format!( - "Git-Protocol: version={}", - self.actual_version as usize - ))); - } - - let PostResponse { - headers, - body, - post_body, - } = self.http.post( - &url, - &self.url, - static_headers.iter().chain(&dynamic_headers), - write_mode.into(), - )?; - let line_provider = self - .line_provider - .as_mut() - .expect("handshake to have been called first"); - line_provider.replace(body); - Ok(RequestWriter::new_from_bufread( - post_body, - Box::new(HeadersThenBody:: { - service, - headers: Some(headers), - body: line_provider.as_read_without_sidebands(), - }), - write_mode, - on_into_read, - trace, - )) - } - fn to_url(&self) -> Cow<'_, BStr> { Cow::Borrowed(self.url.as_str().into()) } @@ -475,6 +425,56 @@ impl blocking_io::Transport for Transport { refs, }) } + + fn request( + &mut self, + write_mode: client::WriteMode, + on_into_read: MessageKind, + trace: bool, + ) -> Result, client::Error> { + let service = self.service.ok_or(client::Error::MissingHandshake)?; + let url = append_url(&self.url, service.as_str()); + let static_headers = &[ + Cow::Borrowed(self.user_agent_header), + Cow::Owned(format!("Content-Type: application/x-{}-request", service.as_str())), + format!("Accept: application/x-{}-result", service.as_str()).into(), + ]; + let mut dynamic_headers = Vec::new(); + self.add_basic_auth_if_present(&mut dynamic_headers)?; + if self.actual_version != Protocol::V1 { + dynamic_headers.push(Cow::Owned(format!( + "Git-Protocol: version={}", + self.actual_version as usize + ))); + } + + let PostResponse { + headers, + body, + post_body, + } = self.http.post( + &url, + &self.url, + static_headers.iter().chain(&dynamic_headers), + write_mode.into(), + )?; + let line_provider = self + .line_provider + .as_mut() + .expect("handshake to have been called first"); + line_provider.replace(body); + Ok(RequestWriter::new_from_bufread( + post_body, + Box::new(HeadersThenBody:: { + service, + headers: Some(headers), + body: line_provider.as_read_without_sidebands(), + }), + write_mode, + on_into_read, + trace, + )) + } } struct HeadersThenBody { diff --git a/gix-transport/src/client/blocking_io/traits.rs b/gix-transport/src/client/blocking_io/traits.rs index 6287fb1adfc..831ade8d574 100644 --- a/gix-transport/src/client/blocking_io/traits.rs +++ b/gix-transport/src/client/blocking_io/traits.rs @@ -4,7 +4,7 @@ use bstr::BString; use crate::{ client::{ - blocking_io::{ExtendedBufRead, ReadlineBufRead}, + blocking_io::{request::RequestWriter, ExtendedBufRead, ReadlineBufRead}, Capabilities, Error, MessageKind, TransportWithoutIO, WriteMode, }, Protocol, Service, @@ -42,6 +42,18 @@ pub trait Transport: TransportWithoutIO { service: Service, extra_parameters: &'a [(&'a str, Option<&'a str>)], ) -> Result, Error>; + + /// Get a writer for sending data and obtaining the response. It can be configured in various ways + /// to support the task at hand. + /// `write_mode` determines how calls to the `write(…)` method are interpreted, and `on_into_read` determines + /// which message to write when the writer is turned into the response reader using [`into_read()`][RequestWriter::into_read()]. + /// If `trace` is `true`, then all packetlines written and received will be traced using facilities provided by the `gix_trace` crate. + fn request( + &mut self, + write_mode: WriteMode, + on_into_read: MessageKind, + trace: bool, + ) -> Result, Error>; } // Would be nice if the box implementation could auto-forward to all implemented traits. @@ -53,6 +65,15 @@ impl Transport for Box { ) -> Result, Error> { self.deref_mut().handshake(service, extra_parameters) } + + fn request( + &mut self, + write_mode: WriteMode, + on_into_read: MessageKind, + trace: bool, + ) -> Result, Error> { + self.deref_mut().request(write_mode, on_into_read, trace) + } } impl Transport for &mut T { @@ -63,6 +84,15 @@ impl Transport for &mut T { ) -> Result, Error> { self.deref_mut().handshake(service, extra_parameters) } + + fn request( + &mut self, + write_mode: WriteMode, + on_into_read: MessageKind, + trace: bool, + ) -> Result, Error> { + self.deref_mut().request(write_mode, on_into_read, trace) + } } /// An extension trait to add more methods to everything implementing [`Transport`]. diff --git a/gix-transport/src/client/git/async_io.rs b/gix-transport/src/client/git/async_io.rs index ecb997a77a6..9ecbfd84976 100644 --- a/gix-transport/src/client/git/async_io.rs +++ b/gix-transport/src/client/git/async_io.rs @@ -51,21 +51,6 @@ where R: AsyncRead + Unpin, W: AsyncWrite + Unpin, { - fn request( - &mut self, - write_mode: client::WriteMode, - on_into_read: client::MessageKind, - trace: bool, - ) -> Result, client::Error> { - Ok(RequestWriter::new_from_bufread( - &mut self.writer, - Box::new(self.line_provider.as_read_without_sidebands()), - write_mode, - on_into_read, - trace, - )) - } - fn to_url(&self) -> Cow<'_, BStr> { self.state.custom_url.as_ref().map_or_else( || { @@ -123,6 +108,21 @@ where refs, }) } + + fn request( + &mut self, + write_mode: client::WriteMode, + on_into_read: client::MessageKind, + trace: bool, + ) -> Result, client::Error> { + Ok(RequestWriter::new_from_bufread( + &mut self.writer, + Box::new(self.line_provider.as_read_without_sidebands()), + write_mode, + on_into_read, + trace, + )) + } } impl Connection diff --git a/gix-transport/src/client/git/blocking_io.rs b/gix-transport/src/client/git/blocking_io.rs index d32340a1f46..1f61bd970ec 100644 --- a/gix-transport/src/client/git/blocking_io.rs +++ b/gix-transport/src/client/git/blocking_io.rs @@ -49,21 +49,6 @@ where R: std::io::Read, W: std::io::Write, { - fn request( - &mut self, - write_mode: client::WriteMode, - on_into_read: client::MessageKind, - trace: bool, - ) -> Result, client::Error> { - Ok(RequestWriter::new_from_bufread( - &mut self.writer, - Box::new(self.line_provider.as_read_without_sidebands()), - write_mode, - on_into_read, - trace, - )) - } - fn to_url(&self) -> Cow<'_, BStr> { self.state.custom_url.as_ref().map_or_else( || { @@ -118,6 +103,21 @@ where refs, }) } + + fn request( + &mut self, + write_mode: client::WriteMode, + on_into_read: client::MessageKind, + trace: bool, + ) -> Result, client::Error> { + Ok(RequestWriter::new_from_bufread( + &mut self.writer, + Box::new(self.line_provider.as_read_without_sidebands()), + write_mode, + on_into_read, + trace, + )) + } } impl Connection diff --git a/gix-transport/src/client/traits.rs b/gix-transport/src/client/traits.rs index 79c01a7425e..4c759c9d839 100644 --- a/gix-transport/src/client/traits.rs +++ b/gix-transport/src/client/traits.rs @@ -6,12 +6,6 @@ use std::{ use bstr::BStr; -#[cfg(all(feature = "async-client", not(feature = "blocking-client")))] -use crate::client::async_io::RequestWriter; -#[cfg(feature = "blocking-client")] -use crate::client::blocking_io::RequestWriter; -#[cfg(any(feature = "blocking-client", feature = "async-client"))] -use crate::client::{MessageKind, WriteMode}; use crate::{client::Error, Protocol}; /// This trait represents all transport related functions that don't require any input/output to be done which helps @@ -26,18 +20,6 @@ pub trait TransportWithoutIO { fn set_identity(&mut self, _identity: gix_sec::identity::Account) -> Result<(), Error> { Err(Error::AuthenticationUnsupported) } - /// Get a writer for sending data and obtaining the response. It can be configured in various ways - /// to support the task at hand. - /// `write_mode` determines how calls to the `write(…)` method are interpreted, and `on_into_read` determines - /// which message to write when the writer is turned into the response reader using [`into_read()`][RequestWriter::into_read()]. - /// If `trace` is `true`, then all packetlines written and received will be traced using facilities provided by the `gix_trace` crate. - #[cfg(any(feature = "blocking-client", feature = "async-client"))] - fn request( - &mut self, - write_mode: WriteMode, - on_into_read: MessageKind, - trace: bool, - ) -> Result, Error>; /// Returns the canonical URL pointing to the destination of this transport. fn to_url(&self) -> Cow<'_, BStr>; @@ -75,16 +57,6 @@ impl TransportWithoutIO for Box { self.deref_mut().set_identity(identity) } - #[cfg(any(feature = "blocking-client", feature = "async-client"))] - fn request( - &mut self, - write_mode: WriteMode, - on_into_read: MessageKind, - trace: bool, - ) -> Result, Error> { - self.deref_mut().request(write_mode, on_into_read, trace) - } - fn to_url(&self) -> Cow<'_, BStr> { self.deref().to_url() } @@ -107,16 +79,6 @@ impl TransportWithoutIO for &mut T { self.deref_mut().set_identity(identity) } - #[cfg(any(feature = "blocking-client", feature = "async-client"))] - fn request( - &mut self, - write_mode: WriteMode, - on_into_read: MessageKind, - trace: bool, - ) -> Result, Error> { - self.deref_mut().request(write_mode, on_into_read, trace) - } - fn to_url(&self) -> Cow<'_, BStr> { self.deref().to_url() }