[fix] Store bytes in EntryMode so Tree roundtrips

Before this change, EntryMode wrapped a `u16` which didn't store enough
information to match the git representation as a `&[u8]` of len 5 or 6.

In particular, the mode that represents a `Tree` could be represented as
`"40000"` or `"040000"`, and the difference would get lost once it was
represented as `0o40000`.

We now have two ways to represent the `EntryMode`:
* `EntryMode` is backed by an owned `BString`.
* `EntryModeRef` is backed by a `&bstr` and bound to the lifetime of the
  owner of these bytes.

This allows call-sites to pick a trade-off between convenience
(`EntryMode` allows to not worry about lifetimes) vs performance
(`EntryModeRef` doesn't allocate) and fits the general paradigm used in
the wider gitoxide project.

Fixes [issue 1887](#1887)

Author: pierrechevalier83

examples/ls-tree.rs

@@ -46,17 +46,17 @@ fn run(args: Args) -> anyhow::Result<()> {
         tree.iter()
             .filter_map(|res| res.ok().map(|entry| entry.inner)) // dropping errors silently
             .filter(|entry| !args.tree_only || (entry.mode.is_tree()))
-            .map(|entry| Entry::new(entry.mode, entry.oid.to_owned(), entry.filename.to_owned()))
+            .map(|entry| Entry::new(entry.mode.to_owned(), entry.oid.to_owned(), entry.filename.to_owned()))
             .collect::<Vec<_>>()
     };
 
     let mut out = stdout().lock();
     for entry in entries {
         writeln!(
             out,
-            "{:06o} {:4} {}    {}",
-            *entry.mode,
-            entry.mode.as_str(),
+            "{:>6o} {:4} {}    {}",
+            entry.mode,
+            entry.mode.kind().as_descriptive_str(),
             entry.hash,
             entry.path
         )?;

gitoxide-core/src/repository/cat.rs

@@ -24,7 +24,9 @@ pub fn display_object(
             match cache.expect("is some") {
                 (BlobFormat::Git, _) => unreachable!("no need for a cache when querying object db"),
                 (BlobFormat::Worktree, cache) => {
-                    let platform = cache.attr_stack.at_entry(path, Some(mode.into()), &repo.objects)?;
+                    let platform = cache
+                        .attr_stack
+                        .at_entry(path, Some(mode.clone().into()), &repo.objects)?;
                     let object = id.object()?;
                     let mut converted = cache.filter.worktree_filter.convert_to_worktree(
                         &object.data,

gitoxide-core/src/repository/diff.rs

@@ -47,19 +47,19 @@ fn write_changes(
                 entry_mode,
                 ..
             } => {
-                writeln!(out, "A: {}", typed_location(location, entry_mode))?;
+                writeln!(out, "A: {}", typed_location(location, &entry_mode))?;
                 writeln!(out, "  {}", id.attach(repo).shorten_or_id())?;
-                writeln!(out, "  -> {:o}", entry_mode)?;
+                writeln!(out, "  -> {entry_mode:o}")?;
             }
             gix::diff::tree_with_rewrites::Change::Deletion {
                 location,
                 id,
                 entry_mode,
                 ..
             } => {
-                writeln!(out, "D: {}", typed_location(location, entry_mode))?;
+                writeln!(out, "D: {}", typed_location(location, &entry_mode))?;
                 writeln!(out, "  {}", id.attach(repo).shorten_or_id())?;
-                writeln!(out, "  {:o} ->", entry_mode)?;
+                writeln!(out, "  {entry_mode:o} ->")?;
             }
             gix::diff::tree_with_rewrites::Change::Modification {
                 location,
@@ -68,15 +68,15 @@ fn write_changes(
                 previous_entry_mode,
                 entry_mode,
             } => {
-                writeln!(out, "M: {}", typed_location(location, entry_mode))?;
+                writeln!(out, "M: {}", typed_location(location, &entry_mode))?;
                 writeln!(
                     out,
                     "  {previous_id} -> {id}",
                     previous_id = previous_id.attach(repo).shorten_or_id(),
                     id = id.attach(repo).shorten_or_id()
                 )?;
                 if previous_entry_mode != entry_mode {
-                    writeln!(out, "  {:o} -> {:o}", previous_entry_mode, entry_mode)?;
+                    writeln!(out, "  {previous_entry_mode:o} -> {entry_mode:o}")?;
                 }
             }
             gix::diff::tree_with_rewrites::Change::Rewrite {
@@ -91,8 +91,8 @@ fn write_changes(
                 writeln!(
                     out,
                     "R: {source} -> {dest}",
-                    source = typed_location(source_location, source_entry_mode),
-                    dest = typed_location(location, entry_mode)
+                    source = typed_location(source_location, &source_entry_mode),
+                    dest = typed_location(location, &entry_mode)
                 )?;
                 writeln!(
                     out,
@@ -101,7 +101,7 @@ fn write_changes(
                     id = id.attach(repo).shorten_or_id()
                 )?;
                 if source_entry_mode != entry_mode {
-                    writeln!(out, "  {:o} -> {:o}", source_entry_mode, entry_mode)?;
+                    writeln!(out, "  {source_entry_mode:o} -> {entry_mode:o}")?;
                 }
             }
         }
@@ -110,7 +110,7 @@ fn write_changes(
     Ok(())
 }
 
-fn typed_location(mut location: BString, mode: EntryMode) -> BString {
+fn typed_location(mut location: BString, mode: &EntryMode) -> BString {
     if mode.is_tree() {
         location.push(b'/');
     }

gix-archive/src/write.rs

@@ -213,7 +213,7 @@ fn append_tar_entry<W: std::io::Write>(
 ) -> Result<(), Error> {
     let mut header = tar::Header::new_gnu();
     header.set_mtime(mtime_seconds_since_epoch as u64);
-    header.set_entry_type(tar_entry_type(entry.mode));
+    header.set_entry_type(tar_entry_type(entry.mode.clone()));
     header.set_mode(if entry.mode.is_executable() { 0o755 } else { 0o644 });
     buf.clear();
     std::io::copy(&mut entry, buf)?;

gix-archive/tests/archive.rs

@@ -213,7 +213,7 @@ mod from_tree {
             noop_pipeline(),
             move |rela_path, mode, attrs| {
                 cache
-                    .at_entry(rela_path, Some(mode.into()), &odb)
+                    .at_entry(rela_path, Some(mode.to_owned().into()), &odb)
                     .map(|entry| entry.matching_attributes(attrs))
                     .map(|_| ())
             },

gix-diff/src/rewrites/tracker.rs

@@ -8,7 +8,7 @@
 use std::ops::Range;
 
 use bstr::{BStr, ByteSlice};
-use gix_object::tree::{EntryKind, EntryMode};
+use gix_object::tree::{EntryKind, EntryMode, EntryModeRef};
 
 use crate::rewrites::tracker::visit::SourceKind;
 use crate::tree::visit::{Action, ChangeId, Relation};
@@ -69,15 +69,15 @@ impl<T: Change> Item<T> {
     fn location<'a>(&self, backing: &'a [u8]) -> &'a BStr {
         backing[self.path.clone()].as_ref()
     }
-    fn entry_mode_compatible(&self, other: EntryMode) -> bool {
+    fn entry_mode_compatible(&self, other: EntryModeRef<'_>) -> bool {
         use EntryKind::*;
         matches!(
             (other.kind(), self.change.entry_mode().kind()),
             (Blob | BlobExecutable, Blob | BlobExecutable) | (Link, Link) | (Tree, Tree)
         )
     }
 
-    fn is_source_for_destination_of(&self, kind: visit::SourceKind, dest_item_mode: EntryMode) -> bool {
+    fn is_source_for_destination_of(&self, kind: visit::SourceKind, dest_item_mode: EntryModeRef<'_>) -> bool {
         self.entry_mode_compatible(dest_item_mode)
             && match kind {
                 visit::SourceKind::Rename => !self.emitted && matches!(self.change.kind(), ChangeKind::Deletion),
@@ -687,7 +687,8 @@ fn find_match<'a, T: Change>(
         let res = items[range.clone()].iter().enumerate().find_map(|(mut src_idx, src)| {
             src_idx += range.start;
             *num_checks += 1;
-            (src_idx != item_idx && src.is_source_for_destination_of(kind, item_mode)).then_some((src_idx, src, None))
+            (src_idx != item_idx && src.is_source_for_destination_of(kind, (&item_mode).into()))
+                .then_some((src_idx, src, None))
         });
         if let Some(src) = res {
             return Ok(Some(src));
@@ -696,11 +697,9 @@ fn find_match<'a, T: Change>(
         let mut has_new = false;
         let percentage = percentage.expect("it's set to something below 1.0 and we assured this");
 
-        for (can_idx, src) in items
-            .iter()
-            .enumerate()
-            .filter(|(src_idx, src)| *src_idx != item_idx && src.is_source_for_destination_of(kind, item_mode))
-        {
+        for (can_idx, src) in items.iter().enumerate().filter(|(src_idx, src)| {
+            *src_idx != item_idx && src.is_source_for_destination_of(kind, (&item_mode).into())
+        }) {
             if !has_new {
                 diff_cache.set_resource(
                     item_id.to_owned(),

gix-diff/src/tree/function.rs

@@ -142,7 +142,7 @@ fn delete_entry_schedule_recursion(
     });
     let is_cancelled = delegate
         .visit(Change::Deletion {
-            entry_mode: entry.mode,
+            entry_mode: entry.mode.to_owned(),
             oid: entry.oid.to_owned(),
             relation,
         })
@@ -174,7 +174,7 @@ fn add_entry_schedule_recursion(
     });
     if delegate
         .visit(Change::Addition {
-            entry_mode: entry.mode,
+            entry_mode: entry.mode.to_owned(),
             oid: entry.oid.to_owned(),
             relation,
         })
@@ -302,9 +302,9 @@ fn handle_lhs_and_rhs_with_equal_filenames(
             if lhs.oid != rhs.oid
                 && delegate
                     .visit(Change::Modification {
-                        previous_entry_mode: lhs.mode,
+                        previous_entry_mode: lhs.mode.to_owned(),
                         previous_oid: lhs.oid.to_owned(),
-                        entry_mode: rhs.mode,
+                        entry_mode: rhs.mode.to_owned(),
                         oid: rhs.oid.to_owned(),
                     })
                     .cancelled()
@@ -321,7 +321,7 @@ fn handle_lhs_and_rhs_with_equal_filenames(
             delegate.push_back_tracked_path_component(lhs.filename);
             if delegate
                 .visit(Change::Deletion {
-                    entry_mode: lhs.mode,
+                    entry_mode: lhs.mode.to_owned(),
                     oid: lhs.oid.to_owned(),
                     relation: None,
                 })
@@ -336,7 +336,7 @@ fn handle_lhs_and_rhs_with_equal_filenames(
             });
             if delegate
                 .visit(Change::Addition {
-                    entry_mode: rhs.mode,
+                    entry_mode: rhs.mode.to_owned(),
                     oid: rhs.oid.to_owned(),
                     relation,
                 })
@@ -354,7 +354,7 @@ fn handle_lhs_and_rhs_with_equal_filenames(
             });
             if delegate
                 .visit(Change::Deletion {
-                    entry_mode: lhs.mode,
+                    entry_mode: lhs.mode.to_owned(),
                     oid: lhs.oid.to_owned(),
                     relation,
                 })
@@ -364,7 +364,7 @@ fn handle_lhs_and_rhs_with_equal_filenames(
             }
             if delegate
                 .visit(Change::Addition {
-                    entry_mode: rhs.mode,
+                    entry_mode: rhs.mode.to_owned(),
                     oid: rhs.oid.to_owned(),
                     relation: None,
                 })
@@ -380,9 +380,9 @@ fn handle_lhs_and_rhs_with_equal_filenames(
             if (lhs.oid != rhs.oid || lhs.mode != rhs.mode)
                 && delegate
                     .visit(Change::Modification {
-                        previous_entry_mode: lhs.mode,
+                        previous_entry_mode: lhs.mode.to_owned(),
                         previous_oid: lhs.oid.to_owned(),
-                        entry_mode: rhs.mode,
+                        entry_mode: rhs.mode.to_owned(),
                         oid: rhs.oid.to_owned(),
                     })
                     .cancelled()

gix-diff/src/tree/visit.rs

@@ -1,5 +1,5 @@
 use gix_hash::ObjectId;
-use gix_object::{tree, tree::EntryMode};
+use gix_object::{tree, tree::EntryModeRef};
 
 /// A way to recognize and associate different [`Change`] instances.
 ///
@@ -63,15 +63,15 @@ impl Change {
         }
     }
     /// Return the current tree entry mode.
-    pub fn entry_mode(&self) -> EntryMode {
+    pub fn entry_mode(&self) -> EntryModeRef<'_> {
         match self {
             Change::Addition { entry_mode, .. }
             | Change::Deletion { entry_mode, .. }
-            | Change::Modification { entry_mode, .. } => *entry_mode,
+            | Change::Modification { entry_mode, .. } => entry_mode.into(),
         }
     }
     /// Return the current object id and tree entry mode of a change.
-    pub fn oid_and_entry_mode(&self) -> (&gix_hash::oid, EntryMode) {
+    pub fn oid_and_entry_mode(&self) -> (&gix_hash::oid, EntryModeRef<'_>) {
         match self {
             Change::Addition {
                 oid,
@@ -83,7 +83,7 @@ impl Change {
                 entry_mode,
                 relation: _,
             }
-            | Change::Modification { oid, entry_mode, .. } => (oid, *entry_mode),
+            | Change::Modification { oid, entry_mode, .. } => (oid, entry_mode.into()),
         }
     }
 }
@@ -139,15 +139,15 @@ mod change_impls {
             match self {
                 Change::Addition { entry_mode, .. }
                 | Change::Deletion { entry_mode, .. }
-                | Change::Modification { entry_mode, .. } => *entry_mode,
+                | Change::Modification { entry_mode, .. } => entry_mode.clone(),
             }
         }
 
         fn id_and_entry_mode(&self) -> (&oid, EntryMode) {
             match self {
                 Change::Addition { entry_mode, oid, .. }
                 | Change::Deletion { entry_mode, oid, .. }
-                | Change::Modification { entry_mode, oid, .. } => (oid, *entry_mode),
+                | Change::Modification { entry_mode, oid, .. } => (oid, entry_mode.clone()),
             }
         }
     }
@@ -161,8 +161,8 @@ mod tests {
     fn size_of_change() {
         let actual = std::mem::size_of::<Change>();
         assert!(
-            actual <= 48,
-            "{actual} <= 48: this type shouldn't grow without us knowing"
+            actual <= 104,
+            "{actual} <= 104: this type shouldn't grow without us knowing"
         );
     }
 }