Skip to content

Commit 2d82ec0

Browse files
committed
change!: use separate error type for I/O hashing operations
Prepare for hashing becoming fallible.
1 parent 9060ae1 commit 2d82ec0

File tree

24 files changed

+125
-76
lines changed

24 files changed

+125
-76
lines changed

gix-hash/src/hasher/io.rs

+12-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
use crate::{hasher, Hasher};
22

3+
/// The error type for I/O operations that compute hashes.
4+
#[derive(Debug, thiserror::Error)]
5+
#[allow(missing_docs)]
6+
pub enum Error {
7+
#[error(transparent)]
8+
Io(#[from] std::io::Error),
9+
}
10+
311
/// Compute the hash of `kind` for the bytes in the file at `path`, hashing only the first `num_bytes_from_start`
412
/// while initializing and calling `progress`.
513
///
@@ -15,7 +23,7 @@ pub fn bytes_of_file(
1523
kind: crate::Kind,
1624
progress: &mut dyn gix_features::progress::Progress,
1725
should_interrupt: &std::sync::atomic::AtomicBool,
18-
) -> std::io::Result<crate::ObjectId> {
26+
) -> Result<crate::ObjectId, Error> {
1927
bytes(
2028
&mut std::fs::File::open(path)?,
2129
num_bytes_from_start,
@@ -32,7 +40,7 @@ pub fn bytes(
3240
kind: crate::Kind,
3341
progress: &mut dyn gix_features::progress::Progress,
3442
should_interrupt: &std::sync::atomic::AtomicBool,
35-
) -> std::io::Result<crate::ObjectId> {
43+
) -> Result<crate::ObjectId, Error> {
3644
bytes_with_hasher(read, num_bytes_from_start, hasher(kind), progress, should_interrupt)
3745
}
3846

@@ -43,7 +51,7 @@ pub fn bytes_with_hasher(
4351
mut hasher: Hasher,
4452
progress: &mut dyn gix_features::progress::Progress,
4553
should_interrupt: &std::sync::atomic::AtomicBool,
46-
) -> std::io::Result<crate::ObjectId> {
54+
) -> Result<crate::ObjectId, Error> {
4755
let start = std::time::Instant::now();
4856
// init progress before the possibility for failure, as convenience in case people want to recover
4957
progress.init(
@@ -62,7 +70,7 @@ pub fn bytes_with_hasher(
6270
progress.inc_by(out.len());
6371
hasher.update(out);
6472
if should_interrupt.load(std::sync::atomic::Ordering::SeqCst) {
65-
return Err(std::io::Error::new(std::io::ErrorKind::Other, "Interrupted"));
73+
return Err(std::io::Error::new(std::io::ErrorKind::Other, "Interrupted").into());
6674
}
6775
}
6876

gix-index/src/file/init.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
use std::path::{Path, PathBuf};
44

5+
use gix_hash::hasher;
6+
57
use crate::{decode, extension, File, State};
68

79
mod error {
@@ -81,7 +83,10 @@ impl File {
8183
object_hash,
8284
&mut gix_features::progress::Discard,
8385
&Default::default(),
84-
)?
86+
)
87+
.map_err(|err| match err {
88+
hasher::io::Error::Io(err) => Error::Io(err),
89+
})?
8590
.verify(&expected)
8691
.map_err(decode::Error::from)?;
8792
}

gix-index/src/file/verify.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
use std::sync::atomic::AtomicBool;
22

3+
use gix_hash::hasher;
4+
35
use crate::File;
46

57
mod error {
@@ -8,7 +10,7 @@ mod error {
810
#[allow(missing_docs)]
911
pub enum Error {
1012
#[error("Could not read index file to generate hash")]
11-
Io(#[from] std::io::Error),
13+
Io(#[from] gix_hash::hasher::io::Error),
1214
#[error("Index checksum mismatch")]
1315
Verify(#[from] gix_hash::verify::Error),
1416
}
@@ -20,7 +22,8 @@ impl File {
2022
pub fn verify_integrity(&self) -> Result<(), Error> {
2123
let _span = gix_features::trace::coarse!("gix_index::File::verify_integrity()");
2224
if let Some(checksum) = self.checksum {
23-
let num_bytes_to_hash = self.path.metadata()?.len() - checksum.as_bytes().len() as u64;
25+
let num_bytes_to_hash =
26+
self.path.metadata().map_err(hasher::io::Error::from)?.len() - checksum.as_bytes().len() as u64;
2427
let should_interrupt = AtomicBool::new(false);
2528
gix_hash::bytes_of_file(
2629
&self.path,

gix-index/src/file/write.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{write, File, Version};
77
#[allow(missing_docs)]
88
pub enum Error {
99
#[error(transparent)]
10-
Io(#[from] std::io::Error),
10+
Io(#[from] hasher::io::Error),
1111
#[error("Could not acquire lock for index file")]
1212
AcquireLock(#[from] gix_lock::acquire::Error),
1313
#[error("Could not commit lock for index file")]
@@ -21,7 +21,7 @@ impl File {
2121
&self,
2222
mut out: impl std::io::Write,
2323
options: write::Options,
24-
) -> std::io::Result<(Version, gix_hash::ObjectId)> {
24+
) -> Result<(Version, gix_hash::ObjectId), hasher::io::Error> {
2525
let _span = gix_features::trace::detail!("gix_index::File::write_to()", skip_hash = options.skip_hash);
2626
let (version, hash) = if options.skip_hash {
2727
let out: &mut dyn std::io::Write = &mut out;
@@ -49,7 +49,7 @@ impl File {
4949
let (version, digest) = self.write_to(&mut lock, options)?;
5050
match lock.into_inner() {
5151
Ok(lock) => lock.commit()?,
52-
Err(err) => return Err(err.into_error().into()),
52+
Err(err) => return Err(Error::Io(err.into_error().into())),
5353
};
5454
self.state.version = version;
5555
self.checksum = Some(digest);

gix-index/src/write.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ impl State {
6767
extensions,
6868
skip_hash: _,
6969
}: Options,
70-
) -> std::io::Result<Version> {
70+
) -> Result<Version, gix_hash::hasher::io::Error> {
7171
let _span = gix_features::trace::detail!("gix_index::State::write()");
7272
let version = self.detect_required_version();
7373

gix-object/benches/edit_tree.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,10 @@ criterion_main!(benches);
136136

137137
type TreeStore = Rc<RefCell<gix_hashtable::HashMap<ObjectId, Tree>>>;
138138

139-
fn new_inmemory_writes() -> (TreeStore, impl FnMut(&Tree) -> Result<ObjectId, std::io::Error>) {
139+
fn new_inmemory_writes() -> (
140+
TreeStore,
141+
impl FnMut(&Tree) -> Result<ObjectId, gix_hash::hasher::io::Error>,
142+
) {
140143
let store = TreeStore::default();
141144
let write_tree = {
142145
let store = store.clone();

gix-object/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ pub fn compute_stream_hash(
424424
stream_len: u64,
425425
progress: &mut dyn gix_features::progress::Progress,
426426
should_interrupt: &std::sync::atomic::AtomicBool,
427-
) -> std::io::Result<gix_hash::ObjectId> {
427+
) -> Result<gix_hash::ObjectId, gix_hash::hasher::io::Error> {
428428
let hasher = object_hasher(hash_kind, object_kind, stream_len);
429429
gix_hash::bytes_with_hasher(stream, stream_len, hasher, progress, should_interrupt)
430430
}

gix-object/tests/object/tree/editor.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,7 @@ mod utils {
793793

794794
pub(super) fn new_inmemory_writes() -> (
795795
TreeStore,
796-
impl FnMut(&Tree) -> Result<ObjectId, std::io::Error>,
796+
impl FnMut(&Tree) -> Result<ObjectId, gix_hash::hasher::io::Error>,
797797
impl Fn() -> usize,
798798
) {
799799
let store = TreeStore::default();
@@ -805,7 +805,7 @@ mod utils {
805805
move |tree: &Tree| {
806806
buf.clear();
807807
tree.write_to(&mut buf)?;
808-
let id = gix_object::compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Tree, &buf);
808+
let id = gix_object::compute_hash(gix_hash::Kind::Sha1, gix_object::Kind::Tree, &buf)?;
809809
store.borrow_mut().insert(id, tree.clone());
810810
let old = num_writes.get();
811811
num_writes.set(old + 1);

gix-odb/src/store_impls/loose/write.rs

+8-8
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use crate::store_impls::loose;
1414
pub enum Error {
1515
#[error("Could not {message} '{path}'")]
1616
Io {
17-
source: io::Error,
17+
source: hasher::io::Error,
1818
message: &'static str,
1919
path: PathBuf,
2020
},
@@ -31,12 +31,12 @@ impl gix_object::Write for Store {
3131
fn write(&self, object: &dyn WriteTo) -> Result<gix_hash::ObjectId, gix_object::write::Error> {
3232
let mut to = self.dest()?;
3333
to.write_all(&object.loose_header()).map_err(|err| Error::Io {
34-
source: err,
34+
source: err.into(),
3535
message: "write header to tempfile in",
3636
path: self.path.to_owned(),
3737
})?;
3838
object.write_to(&mut to).map_err(|err| Error::Io {
39-
source: err,
39+
source: err.into(),
4040
message: "stream all data into tempfile in",
4141
path: self.path.to_owned(),
4242
})?;
@@ -51,13 +51,13 @@ impl gix_object::Write for Store {
5151
let mut to = self.dest().map_err(Box::new)?;
5252
to.write_all(&gix_object::encode::loose_header(kind, from.len() as u64))
5353
.map_err(|err| Error::Io {
54-
source: err,
54+
source: err.into(),
5555
message: "write header to tempfile in",
5656
path: self.path.to_owned(),
5757
})?;
5858

5959
to.write_all(from).map_err(|err| Error::Io {
60-
source: err,
60+
source: err.into(),
6161
message: "stream all data into tempfile in",
6262
path: self.path.to_owned(),
6363
})?;
@@ -77,14 +77,14 @@ impl gix_object::Write for Store {
7777
let mut to = self.dest().map_err(Box::new)?;
7878
to.write_all(&gix_object::encode::loose_header(kind, size))
7979
.map_err(|err| Error::Io {
80-
source: err,
80+
source: err.into(),
8181
message: "write header to tempfile in",
8282
path: self.path.to_owned(),
8383
})?;
8484

8585
io::copy(&mut from, &mut to)
8686
.map_err(|err| Error::Io {
87-
source: err,
87+
source: err.into(),
8888
message: "stream all data into tempfile in",
8989
path: self.path.to_owned(),
9090
})
@@ -118,7 +118,7 @@ impl Store {
118118
}
119119
Ok(hasher::io::Write::new(
120120
deflate::Write::new(builder.tempfile_in(&self.path).map_err(|err| Error::Io {
121-
source: err,
121+
source: err.into(),
122122
message: "create named temp file in",
123123
path: self.path.to_owned(),
124124
})?),

gix-pack/src/data/input/bytes_to_entries.rs

+11-7
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::{fs, io};
22

33
use gix_features::zlib::Decompress;
4-
use gix_hash::{Hasher, ObjectId};
4+
use gix_hash::{hasher, Hasher, ObjectId};
55

66
use crate::data::input;
77

@@ -52,7 +52,7 @@ where
5252
object_hash: gix_hash::Kind,
5353
) -> Result<BytesToEntriesIter<BR>, input::Error> {
5454
let mut header_data = [0u8; 12];
55-
read.read_exact(&mut header_data)?;
55+
read.read_exact(&mut header_data).map_err(hasher::io::Error::from)?;
5656

5757
let (version, num_objects) = crate::data::header::decode(&header_data)?;
5858
assert_eq!(
@@ -97,7 +97,7 @@ where
9797
}
9898
None => crate::data::Entry::from_read(&mut self.read, self.offset, self.hash_len),
9999
}
100-
.map_err(input::Error::from)?;
100+
.map_err(hasher::io::Error::from)?;
101101

102102
// Decompress object to learn its compressed bytes
103103
let compressed_buf = self.compressed_buf.take().unwrap_or_else(|| Vec::with_capacity(4096));
@@ -114,7 +114,7 @@ where
114114
decompressor: &mut self.decompressor,
115115
};
116116

117-
let bytes_copied = io::copy(&mut decompressed_reader, &mut io::sink())?;
117+
let bytes_copied = io::copy(&mut decompressed_reader, &mut io::sink()).map_err(hasher::io::Error::from)?;
118118
if bytes_copied != entry.decompressed_size {
119119
return Err(input::Error::IncompletePack {
120120
actual: bytes_copied,
@@ -138,7 +138,10 @@ where
138138

139139
let crc32 = if self.compressed.crc32() {
140140
let mut header_buf = [0u8; 12 + gix_hash::Kind::longest().len_in_bytes()];
141-
let header_len = entry.header.write_to(bytes_copied, &mut header_buf.as_mut())?;
141+
let header_len = entry
142+
.header
143+
.write_to(bytes_copied, &mut header_buf.as_mut())
144+
.map_err(hasher::io::Error::from)?;
142145
let state = gix_features::hash::crc32_update(0, &header_buf[..header_len]);
143146
Some(gix_features::hash::crc32_update(state, &compressed))
144147
} else {
@@ -172,7 +175,7 @@ where
172175
let mut id = gix_hash::ObjectId::null(self.object_hash);
173176
if let Err(err) = self.read.read_exact(id.as_mut_slice()) {
174177
if self.mode != input::Mode::Restore {
175-
return Err(err.into());
178+
return Err(input::Error::Io(err.into()));
176179
}
177180
}
178181

@@ -269,7 +272,8 @@ where
269272
impl crate::data::File {
270273
/// Returns an iterator over [`Entries`][crate::data::input::Entry], without making use of the memory mapping.
271274
pub fn streaming_iter(&self) -> Result<BytesToEntriesIter<impl io::BufRead>, input::Error> {
272-
let reader = io::BufReader::with_capacity(4096 * 8, fs::File::open(&self.path)?);
275+
let reader =
276+
io::BufReader::with_capacity(4096 * 8, fs::File::open(&self.path).map_err(hasher::io::Error::from)?);
273277
BytesToEntriesIter::new_from_header(
274278
reader,
275279
input::Mode::Verify,

gix-pack/src/data/input/entries_to_bytes.rs

+15-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
use std::iter::Peekable;
22

3+
use gix_hash::hasher;
4+
35
use crate::data::input;
46

57
/// An implementation of [`Iterator`] to write [encoded entries][input::Entry] to an inner implementation each time
@@ -64,7 +66,7 @@ where
6466
self.trailer
6567
}
6668

67-
fn next_inner(&mut self, entry: input::Entry) -> Result<input::Entry, input::Error> {
69+
fn next_inner(&mut self, entry: input::Entry) -> Result<input::Entry, hasher::io::Error> {
6870
if self.num_entries == 0 {
6971
let header_bytes = crate::data::header::encode(self.data_version, 0);
7072
self.output.write_all(&header_bytes[..])?;
@@ -80,7 +82,7 @@ where
8082
Ok(entry)
8183
}
8284

83-
fn write_header_and_digest(&mut self, last_entry: Option<&mut input::Entry>) -> Result<(), input::Error> {
85+
fn write_header_and_digest(&mut self, last_entry: Option<&mut input::Entry>) -> Result<(), hasher::io::Error> {
8486
let header_bytes = crate::data::header::encode(self.data_version, self.num_entries);
8587
let num_bytes_written = if last_entry.is_some() {
8688
self.output.stream_position()?
@@ -127,21 +129,24 @@ where
127129

128130
match self.input.next() {
129131
Some(res) => Some(match res {
130-
Ok(entry) => self.next_inner(entry).and_then(|mut entry| {
131-
if self.input.peek().is_none() {
132-
self.write_header_and_digest(Some(&mut entry)).map(|_| entry)
133-
} else {
134-
Ok(entry)
135-
}
136-
}),
132+
Ok(entry) => self
133+
.next_inner(entry)
134+
.and_then(|mut entry| {
135+
if self.input.peek().is_none() {
136+
self.write_header_and_digest(Some(&mut entry)).map(|_| entry)
137+
} else {
138+
Ok(entry)
139+
}
140+
})
141+
.map_err(input::Error::from),
137142
Err(err) => {
138143
self.is_done = true;
139144
Err(err)
140145
}
141146
}),
142147
None => match self.write_header_and_digest(None) {
143148
Ok(_) => None,
144-
Err(err) => Some(Err(err)),
149+
Err(err) => Some(Err(err.into())),
145150
},
146151
}
147152
}

gix-pack/src/data/input/entry.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ fn compress_data(obj: &gix_object::Data<'_>) -> Result<Vec<u8>, input::Error> {
5454
let mut out = gix_features::zlib::stream::deflate::Write::new(Vec::new());
5555
if let Err(err) = std::io::copy(&mut &*obj.data, &mut out) {
5656
match err.kind() {
57-
std::io::ErrorKind::Other => return Err(input::Error::Io(err)),
57+
std::io::ErrorKind::Other => return Err(input::Error::Io(err.into())),
5858
err => {
5959
unreachable!("Should never see other errors than zlib, but got {:?}", err)
6060
}

gix-pack/src/data/input/types.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
use std::io;
1+
use gix_hash::hasher;
22

33
/// Returned by [`BytesToEntriesIter::new_from_header()`][crate::data::input::BytesToEntriesIter::new_from_header()] and as part
44
/// of `Item` of [`BytesToEntriesIter`][crate::data::input::BytesToEntriesIter].
55
#[derive(thiserror::Error, Debug)]
66
#[allow(missing_docs)]
77
pub enum Error {
88
#[error("An IO operation failed while streaming an entry")]
9-
Io(#[from] io::Error),
9+
Io(#[from] hasher::io::Error),
1010
#[error(transparent)]
1111
PackParse(#[from] crate::data::header::decode::Error),
1212
#[error("Failed to verify pack checksum in trailer")]

0 commit comments

Comments
 (0)