Skip to content

Commit

Permalink
fix!: improve naming and change Target::Peeled to Target::Object.
Browse files Browse the repository at this point in the history
This also renames `TargetRef::Peeled` to `TargetRef::Object` to make clear
that it's not necessarily the peeled object that is contained.

Previously these terms were confusing due to the incorrect usage of the
word `peeled`.
  • Loading branch information
Byron committed Aug 21, 2024
1 parent 98bcb14 commit 43d8096
Show file tree
Hide file tree
Showing 20 changed files with 138 additions and 105 deletions.
12 changes: 6 additions & 6 deletions gix-ref/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ pub struct Namespace(BString);
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub enum Kind {
/// A ref that points to an object id
Peeled,
/// A ref that points to an object id directly.
Object,
/// A ref that points to another reference, adding a level of indirection.
///
/// It can be resolved to an id using the [`peel_in_place_to_id()`][`crate::file::ReferenceExt::peel_to_id_in_place()`] method.
Expand Down Expand Up @@ -203,8 +203,8 @@ pub enum Category<'a> {
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub enum Target {
/// A ref that points to an object id
Peeled(ObjectId),
/// A ref that points directly to an object id.
Object(ObjectId),
/// A ref that points to another reference by its validated name, adding a level of indirection.
///
/// Note that this is an extension of gitoxide which will be helpful in logging all reference changes.
Expand All @@ -214,8 +214,8 @@ pub enum Target {
/// Denotes a ref target, equivalent to [`Kind`], but with immutable data.
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
pub enum TargetRef<'a> {
/// A ref that points to an object id
Peeled(&'a oid),
/// A ref that points directly to an object id.
Object(&'a oid),
/// A ref that points to another reference by its validated name, adding a level of indirection.
Symbolic(&'a FullNameRef),
}
5 changes: 3 additions & 2 deletions gix-ref/src/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ pub struct Reference {
pub name: FullName,
/// The target of the reference, either a symbolic reference by full name or a possibly intermediate object by its id.
pub target: Target,
/// The fully peeled object to which this reference ultimately points to. Only guaranteed to be set after
/// The fully peeled object to which this reference ultimately points to after following all symbolic refs and all annotated
/// tags. Only guaranteed to be set after
/// [`Reference::peel_to_id_in_place()`](crate::file::ReferenceExt) was called or if this reference originated
/// from a packed ref.
pub peeled: Option<ObjectId>,
Expand Down Expand Up @@ -48,7 +49,7 @@ mod convert {
fn from(value: packed::Reference<'p>) -> Self {
Reference {
name: value.name.into(),
target: Target::Peeled(value.target()),
target: Target::Object(value.target()),
peeled: value
.object
.map(|hex| ObjectId::from_hex(hex).expect("parser validation")),
Expand Down
2 changes: 1 addition & 1 deletion gix-ref/src/store/file/loose/reference/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ impl TryFrom<MaybeUnsafeState> for Target {

fn try_from(v: MaybeUnsafeState) -> Result<Self, Self::Error> {
Ok(match v {
MaybeUnsafeState::Id(id) => Target::Peeled(id),
MaybeUnsafeState::Id(id) => Target::Object(id),
MaybeUnsafeState::UnvalidatedPath(name) => {
Target::Symbolic(match gix_validate::reference::name(name.as_ref()) {
Ok(_) => FullName(name),
Expand Down
11 changes: 6 additions & 5 deletions gix-ref/src/store/file/raw_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ pub trait ReferenceExt: Sealed {
fn log_exists(&self, store: &file::Store) -> bool;

/// 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.
/// to the end of the tag-chain, returning the first non-tag object the annotated tag points to,
/// using `objects` to access them.
///
/// This is useful to learn where this reference is ultimately pointing to.
fn peel_to_id_in_place(
Expand Down Expand Up @@ -89,7 +90,7 @@ impl ReferenceExt for Reference {
) -> Result<ObjectId, peel::to_id::Error> {
match self.peeled {
Some(peeled) => {
self.target = Target::Peeled(peeled.to_owned());
self.target = Target::Object(peeled.to_owned());
Ok(peeled)
}
None => {
Expand Down Expand Up @@ -136,7 +137,7 @@ impl ReferenceExt for Reference {
};
};
self.peeled = Some(peeled_id);
self.target = Target::Peeled(peeled_id);
self.target = Target::Object(peeled_id);
Ok(peeled_id)
}
}
Expand All @@ -161,11 +162,11 @@ impl ReferenceExt for Reference {
match self.peeled {
Some(peeled) => Some(Ok(Reference {
name: self.name.clone(),
target: Target::Peeled(peeled),
target: Target::Object(peeled),
peeled: None,
})),
None => match &self.target {
Target::Peeled(_) => None,
Target::Object(_) => None,
Target::Symbolic(full_name) => match store.try_find_packed(full_name.as_ref(), packed) {
Ok(Some(next)) => Some(Ok(next)),
Ok(None) => Some(Err(file::find::existing::Error::NotFound {
Expand Down
10 changes: 5 additions & 5 deletions gix-ref/src/store/file/transaction/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,17 @@ impl<'s, 'p> Transaction<'s, 'p> {
// Unless, the ref is new and we can obtain a peeled id
// identified by the expectation of what could be there, as is the case when cloning.
match expected {
PreviousValue::ExistingMustMatch(Target::Peeled(oid)) => {
PreviousValue::ExistingMustMatch(Target::Object(oid)) => {
Some((Some(gix_hash::ObjectId::null(oid.kind())), oid))
}
_ => None,
}
}
Target::Peeled(new_oid) => {
Target::Object(new_oid) => {
let previous = match expected {
// Here, this means that the ref already existed, and that it will receive (even transitively)
// the given value
PreviousValue::MustExistAndMatch(Target::Peeled(oid)) => Some(oid.to_owned()),
PreviousValue::MustExistAndMatch(Target::Object(oid)) => Some(oid.to_owned()),
_ => None,
}
.or(change.leaf_referent_previous_oid);
Expand All @@ -88,7 +88,7 @@ impl<'s, 'p> Transaction<'s, 'p> {
// Don't do anything else while keeping the lock after potentially updating the reflog.
// We delay deletion of the reference and dropping the lock to after the packed-refs were
// safely written.
if delete_loose_refs && matches!(new, Target::Peeled(_)) {
if delete_loose_refs && matches!(new, Target::Object(_)) {
change.lock = lock;
continue;
}
Expand Down Expand Up @@ -156,7 +156,7 @@ impl<'s, 'p> Transaction<'s, 'p> {
log: LogChange { mode, .. },
new,
..
} => delete_loose_refs && *mode == RefLog::AndReference && matches!(new, Target::Peeled(_)),
} => delete_loose_refs && *mode == RefLog::AndReference && matches!(new, Target::Object(_)),
Change::Delete { log: mode, .. } => *mode == RefLog::AndReference,
};
if take_lock_and_delete {
Expand Down
12 changes: 6 additions & 6 deletions gix-ref/src/store/file/transaction/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ impl<'s, 'p> Transaction<'s, 'p> {
| (PreviousValue::MustExist, Some(_))
| (PreviousValue::MustNotExist | PreviousValue::ExistingMustMatch(_), None) => {}
(PreviousValue::MustExist, None) => {
let expected = Target::Peeled(store.object_hash.null());
let expected = Target::Object(store.object_hash.null());
let full_name = change.name();
return Err(Error::MustExist { full_name, expected });
}
Expand Down Expand Up @@ -163,9 +163,9 @@ impl<'s, 'p> Transaction<'s, 'p> {

fn new_would_change_existing(new: &Target, existing: &Target) -> (bool, bool) {
match (new, existing) {
(Target::Peeled(new), Target::Peeled(old)) => (old != new, false),
(Target::Object(new), Target::Object(old)) => (old != new, false),
(Target::Symbolic(new), Target::Symbolic(old)) => (old != new, true),
(Target::Peeled(_), _) => (true, false),
(Target::Object(_), _) => (true, false),
(Target::Symbolic(_), _) => (true, true),
}
}
Expand All @@ -182,7 +182,7 @@ impl<'s, 'p> Transaction<'s, 'p> {
let mut lock = lock.take().map_or_else(obtain_lock, Ok)?;

lock.with_mut(|file| match new {
Target::Peeled(oid) => write!(file, "{oid}"),
Target::Object(oid) => write!(file, "{oid}"),
Target::Symbolic(name) => writeln!(file, "ref: {}", name.0),
})?;
Some(lock.close()?)
Expand Down Expand Up @@ -277,7 +277,7 @@ impl<'s, 'p> Transaction<'s, 'p> {
};
if let Some(ref mut num_updates) = maybe_updates_for_packed_refs {
if let Change::Update {
new: Target::Peeled(_), ..
new: Target::Object(_), ..
} = edit.update.change
{
edits_for_packed_transaction.push(RefEdit {
Expand Down Expand Up @@ -390,7 +390,7 @@ impl<'s, 'p> Transaction<'s, 'p> {

// traverse parent chain from leaf/peeled ref and set the leaf previous oid accordingly
// to help with their reflog entries
if let (Some(crate::TargetRef::Peeled(oid)), Some(parent_idx)) =
if let (Some(crate::TargetRef::Object(oid)), Some(parent_idx)) =
(change.update.change.previous_value(), change.parent_index)
{
let oid = oid.to_owned();
Expand Down
4 changes: 2 additions & 2 deletions gix-ref/src/store/packed/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl packed::Transaction {
let mut buf = Vec::new();
for edit in &mut edits {
if let Change::Update {
new: Target::Peeled(new),
new: Target::Object(new),
..
} = edit.inner.change
{
Expand Down Expand Up @@ -235,7 +235,7 @@ fn write_edit(out: &mut dyn std::io::Write, edit: &Edit, lines_written: &mut i32
match edit.inner.change {
Change::Delete { .. } => {}
Change::Update {
new: Target::Peeled(target_oid),
new: Target::Object(target_oid),
..
} => {
write!(out, "{target_oid} ")?;
Expand Down
34 changes: 17 additions & 17 deletions gix-ref/src/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,28 @@ impl<'a> TargetRef<'a> {
pub fn kind(&self) -> Kind {
match self {
TargetRef::Symbolic(_) => Kind::Symbolic,
TargetRef::Peeled(_) => Kind::Peeled,
TargetRef::Object(_) => Kind::Object,
}
}
/// Interpret this target as object id which maybe `None` if it is symbolic.
pub fn try_id(&self) -> Option<&oid> {
match self {
TargetRef::Symbolic(_) => None,
TargetRef::Peeled(oid) => Some(oid),
TargetRef::Object(oid) => Some(oid),
}
}
/// Interpret this target as object id or **panic** if it is symbolic.
pub fn id(&self) -> &oid {
match self {
TargetRef::Symbolic(_) => panic!("BUG: tries to obtain object id from symbolic target"),
TargetRef::Peeled(oid) => oid,
TargetRef::Object(oid) => oid,
}
}
/// Interpret this target as name of the reference it points to which maybe `None` if it an object id.
pub fn try_name(&self) -> Option<&FullNameRef> {
match self {
TargetRef::Symbolic(name) => Some(name),
TargetRef::Peeled(_) => None,
TargetRef::Object(_) => None,
}
}
/// Convert this instance into an owned version, without consuming it.
Expand All @@ -44,22 +44,22 @@ impl Target {
pub fn kind(&self) -> Kind {
match self {
Target::Symbolic(_) => Kind::Symbolic,
Target::Peeled(_) => Kind::Peeled,
Target::Object(_) => Kind::Object,
}
}

/// Return true if this is a peeled target with a null hash
pub fn is_null(&self) -> bool {
match self {
Target::Peeled(oid) => oid.is_null(),
Target::Object(oid) => oid.is_null(),
Target::Symbolic(_) => false,
}
}

/// Interpret this owned Target as shared Target
pub fn to_ref(&self) -> TargetRef<'_> {
match self {
Target::Peeled(oid) => TargetRef::Peeled(oid),
Target::Object(oid) => TargetRef::Object(oid),
Target::Symbolic(name) => TargetRef::Symbolic(name.as_ref()),
}
}
Expand All @@ -68,44 +68,44 @@ impl Target {
pub fn try_id(&self) -> Option<&oid> {
match self {
Target::Symbolic(_) => None,
Target::Peeled(oid) => Some(oid),
Target::Object(oid) => Some(oid),
}
}
/// Interpret this target as object id or panic if it is symbolic.
pub fn id(&self) -> &oid {
match self {
Target::Symbolic(_) => panic!("BUG: tries to obtain object id from symbolic target"),
Target::Peeled(oid) => oid,
Target::Object(oid) => oid,
}
}
/// Return the contained object id or panic
pub fn into_id(self) -> ObjectId {
match self {
Target::Symbolic(_) => panic!("BUG: expected peeled reference target but found symbolic one"),
Target::Peeled(oid) => oid,
Target::Object(oid) => oid,
}
}

/// Return the contained object id if the target is peeled or itself if it is not.
pub fn try_into_id(self) -> Result<ObjectId, Self> {
match self {
Target::Symbolic(_) => Err(self),
Target::Peeled(oid) => Ok(oid),
Target::Object(oid) => Ok(oid),
}
}
/// Interpret this target as name of the reference it points to which maybe `None` if it an object id.
pub fn try_name(&self) -> Option<&FullNameRef> {
match self {
Target::Symbolic(name) => Some(name.as_ref()),
Target::Peeled(_) => None,
Target::Object(_) => None,
}
}
}

impl<'a> From<TargetRef<'a>> for Target {
fn from(src: TargetRef<'a>) -> Self {
match src {
TargetRef::Peeled(oid) => Target::Peeled(oid.to_owned()),
TargetRef::Object(oid) => Target::Object(oid.to_owned()),
TargetRef::Symbolic(name) => Target::Symbolic(name.to_owned()),
}
}
Expand All @@ -114,7 +114,7 @@ impl<'a> From<TargetRef<'a>> for Target {
impl<'a> PartialEq<TargetRef<'a>> for Target {
fn eq(&self, other: &TargetRef<'a>) -> bool {
match (self, other) {
(Target::Peeled(lhs), TargetRef::Peeled(rhs)) => lhs == rhs,
(Target::Object(lhs), TargetRef::Object(rhs)) => lhs == rhs,
(Target::Symbolic(lhs), TargetRef::Symbolic(rhs)) => lhs.as_bstr() == rhs.as_bstr(),
_ => false,
}
Expand All @@ -123,7 +123,7 @@ impl<'a> PartialEq<TargetRef<'a>> for Target {

impl From<ObjectId> for Target {
fn from(id: ObjectId) -> Self {
Target::Peeled(id)
Target::Object(id)
}
}

Expand All @@ -132,7 +132,7 @@ impl TryFrom<Target> for ObjectId {

fn try_from(value: Target) -> Result<Self, Self::Error> {
match value {
Target::Peeled(id) => Ok(id),
Target::Object(id) => Ok(id),
Target::Symbolic(_) => Err(value),
}
}
Expand All @@ -147,7 +147,7 @@ impl From<FullName> for Target {
impl fmt::Display for Target {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Target::Peeled(oid) => oid.fmt(f),
Target::Object(oid) => oid.fmt(f),
Target::Symbolic(name) => write!(f, "ref: {}", name.as_bstr()),
}
}
Expand Down
Loading

0 comments on commit 43d8096

Please sign in to comment.