Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion gix-transport/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ gix-sec = { version = "^0.12.2", path = "../gix-sec" }
gix-packetline = { version = "^0.20.0", path = "../gix-packetline" }
gix-credentials = { version = "^0.34.1", path = "../gix-credentials", optional = true }
gix-quote = { version = "^0.6.1", path = "../gix-quote" }
gix-error = { version = "^0.0.0", path = "../gix-error" }
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The gix-error dependency is added but never actually imported or used in the codebase. The code defines its own Error enum and uses the IsSpuriousError trait that's already defined in gix-transport/src/lib.rs, not from gix-error.

Either:

  1. Remove the unused gix-error dependency, or
  2. Actually use types/traits from gix-error if that was the intended design

Based on the PR description mentioning "use gix-error", it seems like the intent was to use it, but the implementation doesn't actually import anything from it.

Suggested change
gix-error = { version = "^0.0.0", path = "../gix-error" }

Copilot uses AI. Check for mistakes.

serde = { version = "1.0.114", optional = true, default-features = false, features = [
"std",
Expand All @@ -95,7 +96,6 @@ bstr = { version = "1.12.0", default-features = false, features = [
"std",
"unicode",
] }
thiserror = "2.0.17"

# for async-client
async-trait = { version = "0.1.51", optional = true }
Expand Down
3 changes: 1 addition & 2 deletions gix-transport/src/client/blocking_io/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,7 @@ impl client::blocking_io::Transport for SpawnProcessOnDemand {
) -> Result<SetServiceResponse<'_>, client::Error> {
let (mut cmd, ssh_kind, cmd_name) = match &self.ssh_cmd {
Some((command, kind)) => (
kind.prepare_invocation(command, &self.url, self.desired_version, self.ssh_disallow_shell)
.map_err(client::Error::SshInvocation)?
kind.prepare_invocation(command, &self.url, self.desired_version, self.ssh_disallow_shell)?
.stderr(Stdio::piped()),
Some(*kind),
Cow::Owned(command.to_owned()),
Expand Down
24 changes: 2 additions & 22 deletions gix-transport/src/client/blocking_io/http/curl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,7 @@ pub struct Options {
/// The error returned by the 'remote' helper, a purely internal construct to perform http requests.
///
/// It can be used for downcasting errors, which are boxed to hide the actual implementation.
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error {
#[error(transparent)]
Curl(#[from] curl::Error),
#[error(transparent)]
Redirect(#[from] http::redirect::Error),
#[error("Could not finish reading all data to post to the remote")]
ReadPostBody(#[from] std::io::Error),
#[error(transparent)]
Authenticate(#[from] gix_credentials::protocol::Error),
}

impl crate::IsSpuriousError for Error {
fn is_spurious(&self) -> bool {
match self {
Error::Curl(err) => curl_is_spurious(err),
_ => false,
}
}
}
pub type Error = crate::Error;

pub(crate) fn curl_is_spurious(err: &curl::Error) -> bool {
err.is_couldnt_connect()
Expand Down Expand Up @@ -77,7 +57,7 @@ impl Curl {
self.handle = Some(handle);
self.req = req;
self.res = res;
err_that_brought_thread_down.into()
err_that_brought_thread_down
}

fn make_request(
Expand Down
16 changes: 0 additions & 16 deletions gix-transport/src/client/blocking_io/http/curl/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,19 +378,3 @@ fn to_curl_ssl_version(vers: SslVersion) -> curl::easy::SslVersion {
SslVersion::TlsV1_3 => Tlsv13,
}
}

impl From<Error> for http::Error {
fn from(err: Error) -> Self {
http::Error::Detail {
description: err.to_string(),
}
}
}

impl From<curl::Error> for http::Error {
fn from(err: curl::Error) -> Self {
http::Error::Detail {
description: err.to_string(),
}
}
}
20 changes: 8 additions & 12 deletions gix-transport/src/client/blocking_io/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,9 @@ impl<H: Http> Transport<H> {
name.eq_ignore_ascii_case("content-type") && value.trim() == wanted_content_type
})
}) {
return Err(client::Error::Http(Error::Detail {
description: format!(
"Didn't find '{wanted_content_type}' header to indicate 'smart' protocol, and 'dumb' protocol is not supported."
),
}));
return Err(client::Error::HttpDetail(format!(
"Didn't find '{wanted_content_type}' header to indicate 'smart' protocol, and 'dumb' protocol is not supported."
)));
}
Ok(())
}
Expand Down Expand Up @@ -391,13 +389,11 @@ impl<H: Http> blocking_io::Transport for Transport<H> {

if let Some(announced_service) = line.as_bstr().strip_prefix(b"# service=") {
if announced_service != service.as_str().as_bytes() {
return Err(client::Error::Http(Error::Detail {
description: format!(
"Expected to see service {:?}, but got {:?}",
service.as_str(),
announced_service
),
}));
return Err(client::Error::HttpDetail(format!(
"Expected to see service {:?}, but got {:?}",
service.as_str(),
announced_service
)));
}

line_reader.as_read().read_to_end(&mut Vec::new())?;
Expand Down
18 changes: 5 additions & 13 deletions gix-transport/src/client/blocking_io/http/redirect.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,14 @@
/// The error provided when redirection went beyond what we deem acceptable.
#[derive(Debug, thiserror::Error)]
#[error("Redirect url {redirect_url:?} could not be reconciled with original url {expected_url} as they don't share the same suffix")]
pub struct Error {
redirect_url: String,
expected_url: String,
}
pub type Error = crate::Error;

pub(crate) fn base_url(redirect_url: &str, base_url: &str, url: String) -> Result<String, Error> {
let tail = url
.strip_prefix(base_url)
.expect("BUG: caller assures `base_url` is subset of `url`");
redirect_url
.strip_suffix(tail)
.ok_or_else(|| Error {
redirect_url: redirect_url.into(),
expected_url: url,
})
.map(ToOwned::to_owned)
redirect_url.strip_suffix(tail).ok_or_else(|| crate::Error::Redirect {
redirect_url: redirect_url.into(),
expected_url: url,
}).map(ToOwned::to_owned)
}

pub(crate) fn swap_tails(effective_base_url: Option<&str>, base_url: &str, mut url: String) -> String {
Expand Down
30 changes: 3 additions & 27 deletions gix-transport/src/client/blocking_io/http/reqwest/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +12,7 @@ use crate::client::blocking_io::http::{
};

/// The error returned by the 'remote' helper, a purely internal construct to perform http requests.
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error {
#[error(transparent)]
Reqwest(#[from] reqwest::Error),
#[error("Could not finish reading all data to post to the remote")]
ReadPostBody(#[from] std::io::Error),
#[error("Request configuration failed")]
ConfigureRequest(#[from] Box<dyn std::error::Error + Send + Sync + 'static>),
#[error(transparent)]
Redirect(#[from] redirect::Error),
}

impl crate::IsSpuriousError for Error {
fn is_spurious(&self) -> bool {
match self {
Error::Reqwest(err) => {
err.is_timeout() || err.is_connect() || err.status().is_some_and(|status| status.is_server_error())
}
_ => false,
}
}
}
pub type Error = crate::Error;

impl Default for Remote {
fn default() -> Self {
Expand Down Expand Up @@ -122,7 +100,7 @@ impl Default for Remote {
if let Some(ref mut request_options) = config.backend.as_ref().and_then(|backend| backend.lock().ok()) {
if let Some(options) = request_options.downcast_mut::<super::Options>() {
if let Some(configure_request) = &mut options.configure_request {
configure_request(&mut req)?;
configure_request(&mut req).map_err(|e| Error::ConfigureRequest(e.to_string()))?;
}
}
}
Expand Down Expand Up @@ -215,9 +193,7 @@ impl Remote {
.expect("handler thread should never panic")
.expect_err("something should have gone wrong with curl (we join on error only)");
*self = Remote::default();
http::Error::InitHttpClient {
source: Box::new(err_that_brought_thread_down),
}
http::Error::InitHttpClient(err_that_brought_thread_down.to_string())
}

fn make_request(
Expand Down
34 changes: 1 addition & 33 deletions gix-transport/src/client/blocking_io/http/traits.rs
Original file line number Diff line number Diff line change
@@ -1,39 +1,7 @@
use crate::client::WriteMode;

/// The error used by the [Http] trait.
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error {
#[error("Could not initialize the http client")]
InitHttpClient {
source: Box<dyn std::error::Error + Send + Sync + 'static>,
},
#[error("{description}")]
Detail { description: String },
#[error("An IO error occurred while uploading the body of a POST request")]
PostBody(#[from] std::io::Error),
}

impl crate::IsSpuriousError for Error {
fn is_spurious(&self) -> bool {
match self {
Error::PostBody(err) => err.is_spurious(),
#[cfg(any(feature = "http-client-reqwest", feature = "http-client-curl"))]
Error::InitHttpClient { source } => {
#[cfg(feature = "http-client-curl")]
if let Some(err) = source.downcast_ref::<crate::client::blocking_io::http::curl::Error>() {
return err.is_spurious();
}
#[cfg(feature = "http-client-reqwest")]
if let Some(err) = source.downcast_ref::<crate::client::blocking_io::http::reqwest::remote::Error>() {
return err.is_spurious();
}
false
}
_ => false,
}
}
}
pub type Error = crate::Error;

/// The return value of [`Http::get()`].
pub struct GetResponse<H, B> {
Expand Down
31 changes: 3 additions & 28 deletions gix-transport/src/client/blocking_io/ssh/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,7 @@ use gix_url::{ArgumentSafety::*, Url};
use crate::{client::blocking_io::file::SpawnProcessOnDemand, Protocol};

/// The error used in [`connect()`].
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error {
#[error("The scheme in \"{}\" is not usable for an ssh connection", .0.to_bstring())]
UnsupportedScheme(gix_url::Url),
#[error("Host name '{host}' could be mistaken for a command-line argument")]
AmbiguousHostName { host: String },
}

impl crate::IsSpuriousError for Error {}
pub type Error = crate::Error;

/// The kind of SSH programs we have built-in support for.
///
Expand All @@ -40,24 +31,8 @@ mod program_kind;

///
pub mod invocation {
use std::ffi::OsString;

/// The error returned when producing ssh invocation arguments based on a selected invocation kind.
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error {
#[error("Username '{user}' could be mistaken for a command-line argument")]
AmbiguousUserName { user: String },
#[error("Host name '{host}' could be mistaken for a command-line argument")]
AmbiguousHostName { host: String },
#[error("The 'Simple' ssh variant doesn't support {function}")]
Unsupported {
/// The simple command that should have been invoked.
command: OsString,
/// The function that was unsupported
function: &'static str,
},
}
pub type Error = crate::Error;
}

///
Expand Down Expand Up @@ -109,7 +84,7 @@ pub fn connect(
trace: bool,
) -> Result<SpawnProcessOnDemand, Error> {
if url.scheme != gix_url::Scheme::Ssh || url.host().is_none() {
return Err(Error::UnsupportedScheme(url));
return Err(Error::UnsupportedScheme(url.scheme));
}
let ssh_cmd = options.ssh_command();
let kind = determine_client_kind(options.kind, ssh_cmd, &url, options.disallow_shell)?;
Expand Down
2 changes: 1 addition & 1 deletion gix-transport/src/client/blocking_io/ssh/program_kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl ProgramKind {
}
ProgramKind::Simple => {
if url.port.is_some() {
return Err(ssh::invocation::Error::Unsupported {
return Err(ssh::invocation::Error::SshUnsupported {
command: ssh_cmd.into(),
function: "setting the port",
});
Expand Down
2 changes: 1 addition & 1 deletion gix-transport/src/client/blocking_io/ssh/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ mod program_kind {
fn simple_cannot_handle_any_arguments() {
assert!(matches!(
try_call(ProgramKind::Simple, "ssh://user@host:42/p", Protocol::V2),
Err(ssh::invocation::Error::Unsupported { .. })
Err(ssh::invocation::Error::SshUnsupported { .. })
));
assert_eq!(
call_args(ProgramKind::Simple, "ssh://user@host/p", Protocol::V2),
Expand Down
34 changes: 8 additions & 26 deletions gix-transport/src/client/capabilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,6 @@ use bstr::{BStr, BString, ByteSlice};
use crate::client;
use crate::Protocol;

/// The error used in [`Capabilities::from_bytes()`] and [`Capabilities::from_lines()`].
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error {
#[error("Capabilities were missing entirely as there was no 0 byte")]
MissingDelimitingNullByte,
#[error("there was not a single capability behind the delimiter")]
NoCapabilities,
#[error("a version line was expected, but none was retrieved")]
MissingVersionLine,
#[error("expected 'version X', got {0:?}")]
MalformattedVersionLine(BString),
#[error("Got unsupported version {actual:?}, expected {}", *desired as u8)]
UnsupportedVersion { desired: Protocol, actual: BString },
#[error("An IO error occurred while reading V2 lines")]
Io(#[from] std::io::Error),
}

/// A structure to represent multiple [capabilities](Capability) or features supported by the server.
///
/// ### Deviation
Expand Down Expand Up @@ -82,10 +64,10 @@ impl Capabilities {
/// Parse capabilities from the given `bytes`.
///
/// Useful in case they are encoded within a `ref` behind a null byte.
pub fn from_bytes(bytes: &[u8]) -> Result<(Capabilities, usize), Error> {
let delimiter_pos = bytes.find_byte(0).ok_or(Error::MissingDelimitingNullByte)?;
pub fn from_bytes(bytes: &[u8]) -> Result<(Capabilities, usize), crate::Error> {
let delimiter_pos = bytes.find_byte(0).ok_or(crate::Error::MissingDelimitingNullByte)?;
if delimiter_pos + 1 == bytes.len() {
return Err(Error::NoCapabilities);
return Err(crate::Error::NoCapabilities);
}
let capabilities = &bytes[delimiter_pos + 1..];
Ok((
Expand All @@ -103,19 +85,19 @@ impl Capabilities {
/// Useful for parsing capabilities from a data sent from a server, and to avoid having to deal with
/// blocking and async traits for as long as possible. There is no value in parsing a few bytes
/// in a non-blocking fashion.
pub fn from_lines(lines_buf: BString) -> Result<Capabilities, Error> {
pub fn from_lines(lines_buf: BString) -> Result<Capabilities, crate::Error> {
let mut lines = <_ as bstr::ByteSlice>::lines(lines_buf.as_slice().trim());
let version_line = lines.next().ok_or(Error::MissingVersionLine)?;
let version_line = lines.next().ok_or(crate::Error::MissingVersionLine)?;
let (name, value) = version_line.split_at(
version_line
.find(b" ")
.ok_or_else(|| Error::MalformattedVersionLine(version_line.to_owned().into()))?,
.ok_or_else(|| crate::Error::MalformattedVersionLine(version_line.to_owned().into()))?,
);
if name != b"version" {
return Err(Error::MalformattedVersionLine(version_line.to_owned().into()));
return Err(crate::Error::MalformattedVersionLine(version_line.to_owned().into()));
}
if value != b" 2" {
return Err(Error::UnsupportedVersion {
return Err(crate::Error::UnsupportedVersion {
desired: Protocol::V2,
actual: value.to_owned().into(),
});
Expand Down
Loading