From 1151de44e1950faadb76496cfa52a3c658fcdf4a Mon Sep 17 00:00:00 2001
From: Wesley Hershberger <mggmugginsmc@gmail.com>
Date: Wed, 26 Aug 2020 11:42:22 -0400
Subject: [PATCH] Refactor transactions
Instead of creating one transaction for all operations and calling a
method on that transaction to add installs or removes, the operation
methods are constructors for transactions, and a transaction represents
the temprorary state on the filesystem of an operation on an individual
package or pair of packages.
Probably some convenience methods should be implemented for
Vec<Transaction> for easy comitting, but the idea is to expose the
stateful nature of the transactions to library users so that they can do
all the expensive parts of a large operation and commit all package
changes at once.
---
pkgar/src/bin.rs | 10 +++----
pkgar/src/transaction.rs | 61 ++++++++++++++++++++++----------------
pkgar/tests/transaction.rs | 9 ++----
3 files changed, 43 insertions(+), 37 deletions(-)
diff --git a/pkgar/src/bin.rs b/pkgar/src/bin.rs
index 2296350..2c2f2d5 100644
--- a/pkgar/src/bin.rs
+++ b/pkgar/src/bin.rs
@@ -226,9 +226,8 @@ pub fn extract(
let mut package = PackageFile::new(archive_path, &pkey)?;
- let mut transaction = Transaction::new(base_dir);
- transaction.install(&mut package)?;
- transaction.commit()?;
+ Transaction::install(&mut package, base_dir)?
+ .commit()?;
Ok(())
}
@@ -242,9 +241,8 @@ pub fn remove(
let mut package = PackageFile::new(archive_path, &pkey)?;
- let mut transaction = Transaction::new(base_dir);
- transaction.remove(&mut package)?;
- transaction.commit()?;
+ Transaction::remove(&mut package, base_dir)?
+ .commit()?;
Ok(())
}
diff --git a/pkgar/src/transaction.rs b/pkgar/src/transaction.rs
index b3b8778..620ab02 100644
--- a/pkgar/src/transaction.rs
+++ b/pkgar/src/transaction.rs
@@ -52,32 +52,28 @@ impl Action {
pub struct Transaction {
actions: Vec<Action>,
- basedir: PathBuf,
}
impl Transaction {
- pub fn new(basedir: impl AsRef<Path>) -> Transaction {
- Transaction {
- actions: Vec::new(),
- basedir: basedir.as_ref().to_path_buf(),
- }
- }
-
- pub fn install<Pkg>(&mut self, src: &mut Pkg) -> Result<(), Error>
+ pub fn install<Pkg>(
+ src: &mut Pkg,
+ base_dir: impl AsRef<Path>
+ ) -> Result<Transaction, Error>
where Pkg: PackageSrc<Err = Error> + PackageSrcExt,
{
let mut buf = vec![0; READ_WRITE_HASH_BUF_SIZE];
let entries = src.read_entries()?;
- self.actions.reserve(entries.len());
+ let mut actions = Vec::with_capacity(entries.len());
for entry in entries {
let relative_path = entry.check_path()
.map_err(|e| e.path(src.path()) )?;
- let target_path = self.basedir.join(relative_path);
+ let target_path = base_dir.as_ref().join(relative_path);
//HELP: Under what circumstances could this ever fail?
- assert!(target_path.starts_with(&self.basedir), "target path was not in the base path");
+ assert!(target_path.starts_with(&base_dir),
+ "target path was not in the base path");
let tmp_path = temp_path(&target_path, entry.blake3())?;
@@ -131,61 +127,76 @@ impl Transaction {
entry.verify(entry_data_hash, entry_data_size)
.map_err(|e| e.path(src.path()))?;
- self.actions.push(Action::Rename(tmp_path, target_path))
+ actions.push(Action::Rename(tmp_path, target_path))
}
- Ok(())
+ Ok(Transaction {
+ actions,
+ })
}
- pub fn replace<Pkg>(&mut self, old: &mut Pkg, new: &mut Pkg) -> Result<(), Error>
+ pub fn replace<Pkg>(
+ old: &mut Pkg,
+ new: &mut Pkg,
+ base_dir: impl AsRef<Path>,
+ ) -> Result<Transaction, Error>
where Pkg: PackageSrc<Err = Error> + PackageSrcExt,
{
let old_entries = old.read_entries()?;
let new_entries = new.read_entries()?;
// All the files that are present in old but not in new
- let mut removes = old_entries.iter()
+ let mut actions = old_entries.iter()
.filter(|old_e| new_entries.iter()
.find(|new_e| new_e.blake3() == old_e.blake3() )
.is_none())
.map(|e| {
- let target_path = self.basedir
+ let target_path = base_dir.as_ref()
.join(e.check_path()?);
Ok(Action::Remove(target_path))
})
.collect::<Result<Vec<Action>, Error>>()?;
- self.actions.append(&mut removes);
//TODO: Don't force a re-read of all the entries for the new package
- self.install(new)
+ let mut trans = Transaction::install(new, base_dir)?;
+ trans.actions.append(&mut actions);
+ Ok(trans)
}
- pub fn remove<Pkg>(&mut self, pkg: &mut Pkg) -> Result<(), Error>
+ pub fn remove<Pkg>(
+ pkg: &mut Pkg,
+ base_dir: impl AsRef<Path>
+ ) -> Result<Transaction, Error>
where Pkg: PackageSrc<Err = Error>,
{
let mut buf = vec![0; READ_WRITE_HASH_BUF_SIZE];
let entries = pkg.read_entries()?;
- self.actions.reserve(entries.len());
+ let mut actions = Vec::with_capacity(entries.len());
for entry in entries {
let relative_path = entry.check_path()?;
- let target_path = self.basedir.join(relative_path);
+ let target_path = base_dir.as_ref()
+ .join(relative_path);
// Under what circumstances could this ever fail?
- assert!(target_path.starts_with(&self.basedir), "target path was not in the base path");
+ assert!(target_path.starts_with(&base_dir),
+ "target path was not in the base path");
let candidate = File::open(&target_path)
.map_err(|e| Error::from(e).path(&target_path) )?;
+ // Ensure that the deletion candidate on disk has not been modified
copy_and_hash(candidate, io::sink(), &mut buf)
.map_err(|e| Error::from(e)
.reason(format!("Hashing file for entry: '{}'", relative_path.display()))
.path(&target_path)
)?;
- self.actions.push(Action::Remove(target_path));
+ actions.push(Action::Remove(target_path));
}
- Ok(())
+ Ok(Transaction {
+ actions
+ })
}
pub fn commit(&mut self) -> Result<usize, Error> {
diff --git a/pkgar/tests/transaction.rs b/pkgar/tests/transaction.rs
index d1bebaf..474edbe 100644
--- a/pkgar/tests/transaction.rs
+++ b/pkgar/tests/transaction.rs
@@ -54,8 +54,7 @@ fn build_install_update_remove() -> Result<(), Box<dyn Error>> {
let mut src_pkg = PackageFile::new(tmp.file("pkgar-src-1.pkgar"), &pkey_file.pkey)?;
println!("Install archive");
- let mut install = Transaction::new(tmp.dir("installroot"));
- install.install(&mut src_pkg)?;
+ let mut install = Transaction::install(&mut src_pkg, tmp.dir("installroot"))?;
install.commit()?;
println!("Modify build");
@@ -70,13 +69,11 @@ fn build_install_update_remove() -> Result<(), Box<dyn Error>> {
let mut src2_pkg = PackageFile::new(tmp.file("pkgar-src-2.pkgar"), &pkey_file.pkey)?;
println!("Upgrade archive");
- let mut update = Transaction::new(tmp.dir("installroot"));
- update.replace(&mut src_pkg, &mut src2_pkg)?;
+ let mut update = Transaction::replace(&mut src_pkg, &mut src2_pkg, tmp.dir("installroot"))?;
update.commit()?;
println!("Uninstall archive");
- let mut remove = Transaction::new(tmp.dir("installroot"));
- remove.remove(&mut src2_pkg)?;
+ let mut remove = Transaction::remove(&mut src2_pkg, tmp.dir("installroot"))?;
remove.commit()?;
assert_eq!(fs::read_dir(tmp.dir("installroot"))?.count(), 0);
--
GitLab