From 1c9d95e357fcc37b855ba8048a2f774318775a99 Mon Sep 17 00:00:00 2001 From: Kim Altintop Date: Wed, 24 Jun 2020 19:58:36 +0200 Subject: [PATCH] Set libgit2's last_error to the error returned by a custom transport (#576) Previously, the propagated error would always be the default "unknown error" as returned by `Error::last_error` if `git_error_last` returns a null pointer. The attached test case is rather trivial, but can be used to reproduce the old and new behaviour. --- src/transport.rs | 59 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/src/transport.rs b/src/transport.rs index c82eb1b27c..118e6d030f 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -2,7 +2,6 @@ use libc::{c_char, c_int, c_uint, c_void, size_t}; use std::ffi::{CStr, CString}; -use std::io; use std::io::prelude::*; use std::mem; use std::ptr; @@ -249,7 +248,10 @@ extern "C" fn subtransport_action( let transport = &mut *(raw_transport as *mut RawSmartSubtransport); let obj = match transport.obj.action(url, action) { Ok(s) => s, - Err(e) => return e.raw_code() as c_int, + Err(e) => { + set_err(&e); + return e.raw_code() as c_int; + } }; *stream = mem::transmute(Box::new(RawSmartSubtransportStream { raw: raw::git_smart_subtransport_stream { @@ -338,7 +340,7 @@ extern "C" fn stream_write( } } -unsafe fn set_err(e: &io::Error) { +unsafe fn set_err(e: &E) { let s = CString::new(e.to_string()).unwrap(); raw::git_error_set_str(raw::GIT_ERROR_NET as c_int, s.as_ptr()); } @@ -350,3 +352,54 @@ extern "C" fn stream_free(stream: *mut raw::git_smart_subtransport_stream) { mem::transmute::<_, Box>(stream); }); } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ErrorClass, ErrorCode}; + use std::sync::Once; + + struct DummyTransport; + + impl SmartSubtransport for DummyTransport { + fn action( + &self, + _url: &str, + _service: Service, + ) -> Result, Error> { + Err(Error::from_str("it's not working")) + } + + fn close(&self) -> Result<(), Error> { + Ok(()) + } + } + + #[test] + fn transport_error_propagates() { + static INIT: Once = Once::new(); + + unsafe { + INIT.call_once(|| { + register("dummy", move |remote| { + Transport::smart(&remote, true, DummyTransport) + }) + .unwrap(); + }) + } + + let (_td, repo) = crate::test::repo_init(); + t!(repo.remote("origin", "dummy://ball")); + + let mut origin = t!(repo.find_remote("origin")); + + match origin.fetch(&["master"], None, None) { + Ok(()) => unreachable!(), + Err(e) => { + assert_eq!(e.message(), "it's not working"); + assert_eq!(e.code(), ErrorCode::GenericError); + assert_eq!(e.class(), ErrorClass::Net); + } + } + } +}