Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

transport err typing #201

Merged
merged 3 commits into from
Sep 19, 2021
Merged
Show file tree
Hide file tree
Changes from all 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 etc/check-package-size.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,4 @@ echo "in root: gitoxide CLI"
(enter git-packetline && indent cargo diet -n --package-size-limit 15KB)
(enter git-repository && indent cargo diet -n --package-size-limit 50KB)
(enter git-transport && indent cargo diet -n --package-size-limit 30KB)
(enter gitoxide-core && indent cargo diet -n --package-size-limit 20KB)
(enter gitoxide-core && indent cargo diet -n --package-size-limit 25KB)
11 changes: 9 additions & 2 deletions git-packetline/src/read/async_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,16 @@ where
return (true, stopped_at, None);
} else if fail_on_err_lines {
if let Some(err) = line.check_error() {
let err = err.0.as_bstr().to_string();
let err = err.0.as_bstr().to_owned();
buf.clear();
return (true, None, Some(Err(io::Error::new(io::ErrorKind::Other, err))));
return (
true,
None,
Some(Err(io::Error::new(
io::ErrorKind::Other,
crate::read::Error { message: err },
))),
);
}
}
let len = line
Expand Down
11 changes: 9 additions & 2 deletions git-packetline/src/read/blocking_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,16 @@ where
return (true, stopped_at, None);
} else if fail_on_err_lines {
if let Some(err) = line.check_error() {
let err = err.0.as_bstr().to_string();
let err = err.0.as_bstr().to_owned();
buf.clear();
return (true, None, Some(Err(io::Error::new(io::ErrorKind::Other, err))));
return (
true,
None,
Some(Err(io::Error::new(
io::ErrorKind::Other,
crate::read::Error { message: err },
))),
);
}
}
let len = line
Expand Down
22 changes: 22 additions & 0 deletions git-packetline/src/read/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,28 @@ type ExhaustiveOutcome<'a> = (
Option<std::io::Result<Result<PacketLineRef<'a>, crate::decode::Error>>>, // actual method result
);

mod error {
use bstr::BString;
use std::fmt::{Debug, Display, Formatter};

/// The error representing an ERR packet line, as possibly wrapped into an `std::io::Error` in
/// [`read_line(…)`][super::StreamingPeekableIter::read_line()].
#[derive(Debug)]
pub struct Error {
/// The contents of the ERR line, with `ERR` portion stripped.
pub message: BString,
}

impl Display for Error {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
Display::fmt(&self.message, f)
}
}

impl std::error::Error for Error {}
}
pub use error::Error;

impl<T> StreamingPeekableIter<T> {
/// Return a new instance from `read` which will stop decoding packet lines when receiving one of the given `delimiters`.
pub fn new(read: T, delimiters: &'static [PacketLineRef<'static>]) -> Self {
Expand Down
9 changes: 7 additions & 2 deletions git-packetline/tests/read/sideband.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,15 @@ async fn handling_of_err_lines() {
let mut buf = [0u8; 2];
let mut reader = rd.as_read();
let res = reader.read(buf.as_mut()).await;
let err = res.unwrap_err();
assert_eq!(err.to_string(), "e", "it respects errors and passes them on");
assert_eq!(
res.unwrap_err().to_string(),
err.into_inner()
.expect("inner err")
.downcast::<git_packetline::read::Error>()
.expect("it's this type")
.message,
"e",
"it respects errors and passes them on"
);
let res = reader.read(buf.as_mut()).await;
assert_eq!(
Expand Down
21 changes: 20 additions & 1 deletion git-protocol/src/fetch/response/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ quick_error! {
pub enum Error {
Io(err: io::Error) {
display("Failed to read from line reader")
from()
source(err)
}
UploadPack(err: git_transport::packetline::read::Error) {
display("Upload pack reported an error")
source(err)
}
Transport(err: client::Error) {
Expand All @@ -33,6 +36,22 @@ quick_error! {
}
}

impl From<std::io::Error> for Error {
fn from(err: io::Error) -> Self {
if err.kind() == io::ErrorKind::Other {
match err.into_inner() {
Some(err) => match err.downcast::<git_transport::packetline::read::Error>() {
Ok(err) => Error::UploadPack(*err),
Err(err) => Error::Io(io::Error::new(io::ErrorKind::Other, err)),
},
None => Error::Io(io::ErrorKind::Other.into()),
}
} else {
Error::Io(err)
}
}
}

/// An 'ACK' line received from the server.
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
Expand Down
16 changes: 16 additions & 0 deletions git-protocol/tests/fetch/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,22 @@ mod v2 {
Ok(())
}

#[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))]
async fn fetch_with_err_response() {
let mut provider = mock_reader("v2/fetch-err-line.response");
provider.fail_on_err_lines(true);
let mut sidebands = provider.as_read_without_sidebands();
match fetch::Response::from_line_reader(Protocol::V2, &mut sidebands).await {
Ok(_) => panic!("need error response"),
Err(err) => match err {
fetch::response::Error::UploadPack(err) => {
assert_eq!(err.message, "segmentation fault\n")
}
err => panic!("we expect upload pack errors, got {:#?}", err),
},
}
}

#[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))]
async fn fetch_acks_and_pack() -> crate::Result {
let mut provider = mock_reader("v2/fetch.response");
Expand Down
1 change: 1 addition & 0 deletions git-protocol/tests/fixtures/v2/fetch-err-line.response
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
001bERR segmentation fault
1 change: 1 addition & 0 deletions git-repository/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
//! * [`interrupt`]
//! * [`protocol`]
//! * [`transport`][protocol::transport]
//! * [`packetline`][protocol::transport::packetline]
//!
#![deny(missing_docs, unsafe_code, rust_2018_idioms)]

Expand Down
2 changes: 2 additions & 0 deletions git-transport/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#![forbid(unsafe_code)]
#![deny(rust_2018_idioms, missing_docs)]

pub use git_packetline as packetline;

/// The version of the way client and server communicate.
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
Expand Down
49 changes: 39 additions & 10 deletions gitoxide-core/src/pack/receive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,17 @@ struct CloneDelegate<W> {
directory: Option<PathBuf>,
refs_directory: Option<PathBuf>,
ref_filter: Option<&'static [&'static str]>,
wanted_refs: Vec<BString>,
}
static FILTER: &[&str] = &["HEAD", "refs/tags", "refs/heads"];

fn remote_supports_ref_in_want(server: &Capabilities) -> bool {
server
.capability("fetch")
.and_then(|cap| cap.supports("ref-in-want"))
.unwrap_or(false)
}

impl<W> protocol::fetch::DelegateBlocking for CloneDelegate<W> {
fn prepare_ls_refs(
&mut self,
Expand All @@ -45,16 +53,26 @@ impl<W> protocol::fetch::DelegateBlocking for CloneDelegate<W> {
if server.contains("ls-refs") {
arguments.extend(FILTER.iter().map(|r| format!("ref-prefix {}", r).into()));
}
Ok(LsRefsAction::Continue)
Ok(if self.wanted_refs.is_empty() {
LsRefsAction::Continue
} else {
LsRefsAction::Skip
})
}

fn prepare_fetch(
&mut self,
version: transport::Protocol,
_server: &Capabilities,
server: &Capabilities,
_features: &mut Vec<(&str, Option<&str>)>,
_refs: &[Ref],
) -> io::Result<Action> {
if !self.wanted_refs.is_empty() && !remote_supports_ref_in_want(server) {
return Err(io::Error::new(
io::ErrorKind::Other,
"Want to get specific refs, but remote doesn't support this capability",
));
}
if version == transport::Protocol::V1 {
self.ref_filter = Some(FILTER);
}
Expand All @@ -67,15 +85,21 @@ impl<W> protocol::fetch::DelegateBlocking for CloneDelegate<W> {
arguments: &mut Arguments,
_previous_response: Option<&Response>,
) -> io::Result<Action> {
for r in refs {
let (path, id) = r.unpack();
match self.ref_filter {
Some(ref_prefixes) => {
if ref_prefixes.iter().any(|prefix| path.starts_with_str(prefix)) {
arguments.want(id);
if self.wanted_refs.is_empty() {
for r in refs {
let (path, id) = r.unpack();
match self.ref_filter {
Some(ref_prefixes) => {
if ref_prefixes.iter().any(|prefix| path.starts_with_str(prefix)) {
arguments.want(id);
}
}
None => arguments.want(id),
}
None => arguments.want(id),
}
} else {
for r in &self.wanted_refs {
arguments.want_ref(r.as_ref())
}
}
Ok(Action::Cancel)
Expand All @@ -87,6 +111,7 @@ mod blocking_io {
use std::{io, io::BufRead, path::PathBuf};

use git_repository::{
bstr::BString,
protocol,
protocol::fetch::{Ref, Response},
Progress,
Expand Down Expand Up @@ -119,6 +144,7 @@ mod blocking_io {
url: &str,
directory: Option<PathBuf>,
refs_directory: Option<PathBuf>,
wanted_refs: Vec<BString>,
progress: P,
ctx: Context<W>,
) -> anyhow::Result<()> {
Expand All @@ -128,6 +154,7 @@ mod blocking_io {
directory,
refs_directory,
ref_filter: None,
wanted_refs,
};
protocol::fetch(
transport,
Expand All @@ -150,7 +177,7 @@ mod async_io {
use async_trait::async_trait;
use futures_io::AsyncBufRead;
use git_repository::{
objs::bstr::{BString, ByteSlice},
bstr::{BString, ByteSlice},
odb::pack,
protocol,
protocol::fetch::{Ref, Response},
Expand Down Expand Up @@ -185,6 +212,7 @@ mod async_io {
url: &str,
directory: Option<PathBuf>,
refs_directory: Option<PathBuf>,
wanted_refs: Vec<BString>,
progress: P,
ctx: Context<W>,
) -> anyhow::Result<()> {
Expand All @@ -194,6 +222,7 @@ mod async_io {
directory,
refs_directory,
ref_filter: None,
wanted_refs,
};
blocking::unblock(move || {
futures_lite::future::block_on(protocol::fetch(
Expand Down
2 changes: 2 additions & 0 deletions src/plumbing/lean/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ pub fn main() -> Result<()> {
protocol,
url,
directory,
refs,
refs_directory,
}) => {
let (_handle, progress) = prepare(verbose, "pack-receive", core::pack::receive::PROGRESS_RANGE);
Expand All @@ -100,6 +101,7 @@ pub fn main() -> Result<()> {
&url,
directory,
refs_directory,
refs.into_iter().map(|s| s.into()).collect(),
DoOrDiscard::from(progress),
core::pack::receive::Context {
thread_limit,
Expand Down
8 changes: 7 additions & 1 deletion src/plumbing/lean/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ pub struct PackReceive {
/// the directory into which to write references. Existing files will be overwritten.
///
/// Note that the directory will be created if needed.
#[argh(option, short = 'r')]
#[argh(option, short = 'd')]
pub refs_directory: Option<PathBuf>,

/// the URLs or path from which to receive the pack.
Expand All @@ -111,6 +111,12 @@ pub struct PackReceive {
#[argh(positional)]
pub url: String,

/// if set once or more times, these references will be fetched instead of all advertised ones.
///
/// Note that this requires a reasonably modern git server.
#[argh(option, long = "reference", short = 'r')]
pub refs: Vec<String>,

/// the directory into which to write the received pack and index.
///
/// If unset, they will be discarded.
Expand Down
2 changes: 2 additions & 0 deletions src/plumbing/pretty/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ pub fn main() -> Result<()> {
protocol,
url,
directory,
refs,
refs_directory,
} => prepare_and_run(
"pack-receive",
Expand All @@ -96,6 +97,7 @@ pub fn main() -> Result<()> {
&url,
directory,
refs_directory,
refs.into_iter().map(|r| r.into()).collect(),
git_features::progress::DoOrDiscard::from(progress),
core::pack::receive::Context {
thread_limit,
Expand Down
8 changes: 7 additions & 1 deletion src/plumbing/pretty/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,20 @@ pub enum Subcommands {
/// the directory into which to write references. Existing files will be overwritten.
///
/// Note that the directory will be created if needed.
#[clap(long, short = 'r')]
#[clap(long, short = 'd')]
refs_directory: Option<PathBuf>,

/// The URLs or path from which to receive the pack.
///
/// See here for a list of supported URLs: <https://www.git-scm.com/docs/git-clone#_git_urls>
url: String,

/// If set once or more times, these references will be fetched instead of all advertised ones.
///
/// Note that this requires a reasonably modern git server.
#[clap(long = "reference", short = 'r')]
refs: Vec<String>,

/// The directory into which to write the received pack and index.
///
/// If unset, they will be discarded.
Expand Down
Loading