Skip to content

Commit

Permalink
change!: use gix-object::Find trait
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Nov 5, 2023
1 parent 7407fec commit dbb6886
Show file tree
Hide file tree
Showing 10 changed files with 61 additions and 80 deletions.
11 changes: 1 addition & 10 deletions gix-ref/src/peel.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,3 @@
/// A function for use in [`crate::file::ReferenceExt::peel_to_id_in_place()`] to indicate no peeling should happen.
#[allow(clippy::type_complexity)]
pub fn none(
_id: gix_hash::ObjectId,
#[allow(clippy::ptr_arg)] _buf: &mut Vec<u8>,
) -> Result<Option<(gix_object::Kind, &[u8])>, Box<dyn std::error::Error + Send + Sync>> {
Ok(Some((gix_object::Kind::Commit, &[])))
}

///
pub mod to_id {
use std::path::PathBuf;
Expand All @@ -26,7 +17,7 @@ pub mod to_id {
#[error("Refusing to follow more than {max_depth} levels of indirection")]
DepthLimitExceeded { max_depth: usize },
#[error("An error occurred when trying to resolve an object a reference points to")]
Find(#[from] Box<dyn std::error::Error + Send + Sync + 'static>),
Find(#[from] gix_object::find::Error),
#[error("Object {oid} as referred to by {name:?} could not be found")]
NotFound { oid: gix_hash::ObjectId, name: BString },
}
Expand Down
35 changes: 18 additions & 17 deletions gix-ref/src/store/file/raw_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,6 @@ use crate::{
pub trait Sealed {}
impl Sealed for crate::Reference {}

pub type FindObjectFn<'a> = dyn FnMut(
gix_hash::ObjectId,
&mut Vec<u8>,
) -> Result<Option<(gix_object::Kind, &[u8])>, Box<dyn std::error::Error + Send + Sync>>
+ 'a;

/// A trait to extend [Reference][crate::Reference] with functionality requiring a [file::Store].
pub trait ReferenceExt: Sealed {
/// A step towards obtaining forward or reverse iterators on reference logs.
Expand All @@ -26,18 +20,22 @@ pub trait ReferenceExt: Sealed {
/// For details, see [`Reference::log_exists()`].
fn log_exists(&self, store: &file::Store) -> bool;

/// For details, see [`Reference::peel_to_id_in_place()`].
/// Follow all symbolic targets this reference might point to and peel the underlying object
/// to the end of the chain, and return it, using `objects` to access them.
///
/// This is useful to learn where this reference is ultimately pointing to.
fn peel_to_id_in_place(
&mut self,
store: &file::Store,
find: &mut FindObjectFn<'_>,
objects: &dyn gix_object::Find,
) -> Result<ObjectId, peel::to_id::Error>;

/// For details, see [`Reference::peel_to_id_in_place()`], with support for a known stable packed buffer.
/// Like [`ReferenceExt::peel_to_id_in_place()`], but with support for a known stable packed buffer
/// to use for resolving symbolic links.
fn peel_to_id_in_place_packed(
&mut self,
store: &file::Store,
find: &mut FindObjectFn<'_>,
objects: &dyn gix_object::Find,
packed: Option<&packed::Buffer>,
) -> Result<ObjectId, peel::to_id::Error>;

Expand Down Expand Up @@ -75,18 +73,18 @@ impl ReferenceExt for Reference {
fn peel_to_id_in_place(
&mut self,
store: &file::Store,
find: &mut FindObjectFn<'_>,
objects: &dyn gix_object::Find,
) -> Result<ObjectId, peel::to_id::Error> {
let packed = store.assure_packed_refs_uptodate().map_err(|err| {
peel::to_id::Error::Follow(file::find::existing::Error::Find(file::find::Error::PackedOpen(err)))
})?;
self.peel_to_id_in_place_packed(store, find, packed.as_ref().map(|b| &***b))
self.peel_to_id_in_place_packed(store, objects, packed.as_ref().map(|b| &***b))
}

fn peel_to_id_in_place_packed(
&mut self,
store: &file::Store,
find: &mut FindObjectFn<'_>,
objects: &dyn gix_object::Find,
packed: Option<&packed::Buffer>,
) -> Result<ObjectId, peel::to_id::Error> {
match self.peeled {
Expand Down Expand Up @@ -118,10 +116,13 @@ impl ReferenceExt for Reference {
let mut buf = Vec::new();
let mut oid = self.target.try_id().expect("peeled ref").to_owned();
let peeled_id = loop {
let (kind, data) = find(oid, &mut buf)?.ok_or_else(|| peel::to_id::Error::NotFound {
oid,
name: self.name.0.clone(),
})?;
let gix_object::Data { kind, data } =
objects
.try_find(&oid, &mut buf)?
.ok_or_else(|| peel::to_id::Error::NotFound {
oid,
name: self.name.0.clone(),
})?;
match kind {
gix_object::Kind::Tag => {
oid = gix_object::TagRefIter::from_bytes(data).target_id().map_err(|_err| {
Expand Down
14 changes: 2 additions & 12 deletions gix-ref/src/store/file/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,18 @@ use crate::{
transaction::RefEdit,
};

/// A function receiving an object id to resolve, returning its decompressed bytes,
/// used to obtain the peeled object ids for storage in packed-refs files.
///
/// Resolution means to follow tag objects until the end of the chain.
pub type FindObjectFn<'a> = dyn FnMut(
gix_hash::ObjectId,
&mut Vec<u8>,
) -> Result<Option<gix_object::Kind>, Box<dyn std::error::Error + Send + Sync + 'static>>
+ 'a;

/// How to handle packed refs during a transaction
#[derive(Default)]
pub enum PackedRefs<'a> {
/// Only propagate deletions of references. This is the default
#[default]
DeletionsOnly,
/// Propagate deletions as well as updates to references which are peeled, that is contain an object id
DeletionsAndNonSymbolicUpdates(Box<FindObjectFn<'a>>),
DeletionsAndNonSymbolicUpdates(Box<dyn gix_object::Find + 'a>),
/// Propagate deletions as well as updates to references which are peeled, that is contain an object id. Furthermore delete the
/// reference which is originally updated if it exists. If it doesn't, the new value will be written into the packed ref right away.
/// Note that this doesn't affect symbolic references at all, which can't be placed into packed refs.
DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(Box<FindObjectFn<'a>>),
DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(Box<dyn gix_object::Find + 'a>),
}

#[derive(Debug)]
Expand Down
6 changes: 2 additions & 4 deletions gix-ref/src/store/file/transaction/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,12 +338,10 @@ impl<'s, 'p> Transaction<'s, 'p> {
self.packed_transaction = Some(match &mut self.packed_refs {
PackedRefs::DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(f)
| PackedRefs::DeletionsAndNonSymbolicUpdates(f) => {
transaction.prepare(&mut edits_for_packed_transaction.into_iter(), f)?
transaction.prepare(&mut edits_for_packed_transaction.into_iter(), &**f)?
}
PackedRefs::DeletionsOnly => transaction
.prepare(&mut edits_for_packed_transaction.into_iter(), &mut |_, _| {
unreachable!("BUG: deletions never trigger object lookups")
})?,
.prepare(&mut edits_for_packed_transaction.into_iter(), &gix_object::find::Never)?,
});
}
}
Expand Down
7 changes: 4 additions & 3 deletions gix-ref/src/store/packed/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::{fmt::Formatter, io::Write};

use crate::{
file,
store_impl::{file::transaction::FindObjectFn, packed, packed::Edit},
store_impl::{packed, packed::Edit},
transaction::{Change, RefEdit},
Target,
};
Expand Down Expand Up @@ -44,10 +44,11 @@ impl packed::Transaction {
/// Lifecycle
impl packed::Transaction {
/// Prepare the transaction by checking all edits for applicability.
/// Use `objects` to access objects for the purpose of peeling them - this is only used if packed-refs are involved.
pub fn prepare(
mut self,
edits: &mut dyn Iterator<Item = RefEdit>,
find: &mut FindObjectFn<'_>,
objects: &dyn gix_object::Find,
) -> Result<Self, prepare::Error> {
assert!(self.edits.is_none(), "BUG: cannot call prepare(…) more than once");
let buffer = &self.buffer;
Expand Down Expand Up @@ -76,7 +77,7 @@ impl packed::Transaction {
{
let mut next_id = new;
edit.peeled = loop {
let kind = find(next_id, &mut buf)?;
let kind = objects.try_find(&next_id, &mut buf)?.map(|d| d.kind);
match kind {
Some(gix_object::Kind::Tag) => {
next_id = gix_object::TagRefIter::from_bytes(&buf).target_id().map_err(|_| {
Expand Down
14 changes: 14 additions & 0 deletions gix-ref/tests/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,20 @@ fn store_writable(name: &str) -> crate::Result<(gix_testtools::tempfile::TempDir
))
}

struct EmptyCommit;
impl gix_object::Find for EmptyCommit {
fn try_find<'a>(
&self,
_id: &gix_hash::oid,
_buffer: &'a mut Vec<u8>,
) -> Result<Option<gix_object::Data<'a>>, gix_object::find::Error> {
Ok(Some(gix_object::Data {
kind: gix_object::Kind::Commit,
data: &[],
}))
}
}

mod log;
mod reference;
mod store;
Expand Down
19 changes: 8 additions & 11 deletions gix-ref/tests/file/reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ mod reflog {
}

mod peel {
use gix_odb::pack::Find;
use gix_ref::{file::ReferenceExt, peel, Reference};
use gix_ref::{file::ReferenceExt, Reference};

use crate::file::EmptyCommit;
use crate::{file, file::store_with_packed_refs, hex_to_id};

#[test]
Expand Down Expand Up @@ -77,11 +77,11 @@ mod peel {
let store = store_with_packed_refs()?;
let mut head: Reference = store.find_loose("HEAD")?.into();
let expected = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03");
assert_eq!(head.peel_to_id_in_place(&store, &mut peel::none)?, expected);
assert_eq!(head.peel_to_id_in_place(&store, &EmptyCommit)?, expected);
assert_eq!(head.target.try_id().map(ToOwned::to_owned), Some(expected));

let mut head = store.find("dt1")?;
assert_eq!(head.peel_to_id_in_place(&store, &mut peel::none)?, expected);
assert_eq!(head.peel_to_id_in_place(&store, &gix_object::find::Never)?, expected);
assert_eq!(head.target.into_id(), expected);
Ok(())
}
Expand Down Expand Up @@ -119,23 +119,20 @@ mod peel {
assert_eq!(r.kind(), gix_ref::Kind::Symbolic, "there is something to peel");

let commit = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03");
assert_eq!(r.peel_to_id_in_place(&store, &mut peel::none)?, commit);
assert_eq!(r.peel_to_id_in_place(&store, &EmptyCommit)?, commit);
assert_eq!(r.name.as_bstr(), "refs/remotes/origin/multi-link-target3");

let mut r: Reference = store.find_loose("dt1")?.into();
assert_eq!(
r.peel_to_id_in_place(&store, &mut peel::none)?,
r.peel_to_id_in_place(&store, &EmptyCommit)?,
hex_to_id("4c3f4cce493d7beb45012e478021b5f65295e5a3"),
"points to a tag object without actual object lookup"
);

let odb = gix_odb::at(store.git_dir().join("objects"))?;
let mut r: Reference = store.find_loose("dt1")?.into();
assert_eq!(
r.peel_to_id_in_place(&store, &mut |oid, buf| {
odb.try_find(&oid, buf)
.map(|obj| obj.map(|(obj, _)| (obj.kind, obj.data)))
})?,
r.peel_to_id_in_place(&store, &odb)?,
commit,
"points to the commit with lookup"
);
Expand All @@ -151,7 +148,7 @@ mod peel {
assert_eq!(r.name.as_bstr(), "refs/loop-a");

assert!(matches!(
r.peel_to_id_in_place(&store, &mut peel::none).unwrap_err(),
r.peel_to_id_in_place(&store, &gix_object::find::Never).unwrap_err(),
gix_ref::peel::to_id::Error::Cycle { .. }
));
assert_eq!(r.name.as_bstr(), "refs/loop-a", "the ref is not changed on error");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use gix_ref::{
Target,
};

use crate::file::EmptyCommit;
use crate::{
file::transaction::prepare_and_commit::{committer, create_at, create_symbolic_at, delete_at, empty_store},
hex_to_id,
Expand Down Expand Up @@ -69,14 +70,14 @@ fn packed_refs_lock_is_mandatory_for_multiple_ongoing_transactions_even_if_one_d
let _t1 = store
.transaction()
.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(
Box::new(|_, _| Ok(Some(gix_object::Kind::Commit))),
Box::new(EmptyCommit),
))
.prepare([create_at(ref_name)], Fail::Immediately, Fail::Immediately)?;

let t2res = store
.transaction()
.prepare([delete_at(ref_name)], Fail::Immediately, Fail::Immediately);
assert_eq!(&t2res.unwrap_err().to_string()[..51], "The lock for the packed-ref file could not be obtai", "if packed-refs are about to be created, other transactions always acquire a packed-refs lock as to not miss anything");
assert_eq!(&t2res.unwrap_err().to_string()[..54], "The lock for the packed-ref file could not be obtained", "if packed-refs are about to be created, other transactions always acquire a packed-refs lock as to not miss anything");
Ok(())
}

Expand All @@ -86,7 +87,7 @@ fn conflicting_creation_into_packed_refs() -> crate::Result {
store
.transaction()
.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(
Box::new(|_, _| Ok(Some(gix_object::Kind::Commit))),
Box::new(EmptyCommit),
))
.prepare(
[
Expand Down Expand Up @@ -118,7 +119,7 @@ fn conflicting_creation_into_packed_refs() -> crate::Result {
store
.transaction()
.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(
Box::new(|_, _| Ok(Some(gix_object::Kind::Commit))),
Box::new(EmptyCommit),
))
.prepare(
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::convert::TryInto;
use gix_hash::ObjectId;
use gix_lock::acquire::Fail;
use gix_object::bstr::{BString, ByteSlice};
use gix_object::Find;
use gix_ref::{
file::{
transaction::{self, PackedRefs},
Expand All @@ -14,6 +13,7 @@ use gix_ref::{
Target,
};

use crate::file::EmptyCommit;
use crate::{
file::{
store_with_packed_refs, store_writable,
Expand Down Expand Up @@ -713,7 +713,7 @@ fn packed_refs_creation_with_packed_refs_mode_prune_removes_original_loose_refs(
let edits = store
.transaction()
.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(
Box::new(move |oid, buf| odb.try_find(&oid, buf).map(|obj| obj.map(|obj| obj.kind))),
Box::new(odb),
))
.prepare(
store
Expand Down Expand Up @@ -782,9 +782,7 @@ fn packed_refs_creation_with_packed_refs_mode_leave_keeps_original_loose_refs()

let edits = store
.transaction()
.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdates(Box::new(|_, _| {
Ok(Some(gix_object::Kind::Commit))
})))
.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdates(Box::new(EmptyCommit)))
.prepare(edits, Fail::Immediately, Fail::Immediately)?
.commit(committer().to_ref())?;
assert_eq!(
Expand Down
18 changes: 4 additions & 14 deletions gix-ref/tests/file/worktree.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::{cmp::Ordering, path::PathBuf};

use gix_object::Find;
use gix_ref::{file::ReferenceExt, Reference};
use gix_testtools::Creation;

Expand Down Expand Up @@ -61,13 +60,7 @@ fn into_peel(
store: &gix_ref::file::Store,
odb: gix_odb::Handle,
) -> impl Fn(gix_ref::Reference) -> gix_hash::ObjectId + '_ {
move |mut r: gix_ref::Reference| {
r.peel_to_id_in_place(store, &mut |id, buf| {
let data = odb.try_find(&id, buf)?;
Ok(data.map(|d| (d.kind, d.data)))
})
.unwrap()
}
move |mut r: gix_ref::Reference| r.peel_to_id_in_place(store, &odb).unwrap()
}

enum Mode {
Expand Down Expand Up @@ -205,6 +198,7 @@ mod writable {
FullName, FullNameRef, Target,
};

use crate::file::EmptyCommit;
use crate::{
file::{
transaction::prepare_and_commit::committer,
Expand Down Expand Up @@ -232,9 +226,7 @@ mod writable {
let (store, _odb, _tmp) = main_store(packed, Mode::Write)?;
let mut t = store.transaction();
if packed {
t = t.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdates(Box::new(|_, _| {
Ok(Some(gix_object::Kind::Commit))
})));
t = t.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdates(Box::new(EmptyCommit)));
}

let edits = t
Expand Down Expand Up @@ -514,9 +506,7 @@ mod writable {

let mut t = store.transaction();
if packed {
t = t.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdates(Box::new(|_, _| {
Ok(Some(gix_object::Kind::Commit))
})));
t = t.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdates(Box::new(EmptyCommit)));
}

let edits = t
Expand Down

0 comments on commit dbb6886

Please sign in to comment.