fix!(gix-object): 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`.

Change the backing type for `EntryMode` from a `u16` to a `[u8; 6]`.
This way, we can represent `b"40000"` as `b"40000 "` which differs from
`b"040000"`

Add a regression test that ensures `EntryMode` must roundtrip.

Fixes [issue 1887](#1887)

BREAKING CHANGE:
* Sha1 for certain git-objects will change (to match Git's)
* The API had to be updated to decouple callers from the internal data
  representation

Author: pierrechevalier83

gix-object/src/object/convert.rs

@@ -71,7 +71,7 @@ impl From<tree::EntryRef<'_>> for tree::Entry {
     fn from(other: tree::EntryRef<'_>) -> tree::Entry {
         let tree::EntryRef { mode, filename, oid } = other;
         tree::Entry {
-            mode,
+            mode: mode.to_owned(),
             filename: filename.to_owned(),
             oid: oid.into(),
         }
@@ -89,6 +89,25 @@ impl<'a> From<&'a tree::Entry> for tree::EntryRef<'a> {
     }
 }
 
+impl<'a> TryFrom<&'a [u8]> for tree::EntryMode {
+    type Error = &'a [u8];
+
+    fn try_from(mode: &'a [u8]) -> Result<Self, Self::Error> {
+        let len = if let Some(delim) = mode.iter().position(|b| *b == b' ') {
+            delim
+        } else {
+            mode.len().min(6)
+        };
+        let mut git_representation = [b' '; 6];
+        git_representation[..len].copy_from_slice(&mode[..len]);
+        let value = tree::parse_git_mode(&git_representation).ok_or(mode)?;
+        Ok(Self {
+            value,
+            git_representation,
+        })
+    }
+}
+
 impl From<ObjectRef<'_>> for Object {
     fn from(v: ObjectRef<'_>) -> Self {
         match v {

gix-object/src/tree/mod.rs

@@ -1,5 +1,5 @@
 use crate::{
-    bstr::{BStr, BString},
+    bstr::{BStr, BString, ByteSlice},
     tree, Tree, TreeRef,
 };
 use std::cell::RefCell;
@@ -36,19 +36,100 @@ pub struct Editor<'a> {
     tree_buf: Vec<u8>,
 }
 
+/// Parse a valid git mode into a u16
+/// A valid git mode can be represented by a set of 5-6 octal digits. The leftmost octal digit can
+/// be at most one. These conditions guarantee that it can be represented as a `u16`, although that
+/// representation is lossy compared to the byte slice it came from as `"040000"` and `"40000"` will
+/// both be represented as `0o40000`.
+/// Input:
+///     We accept input that contains exactly a valid git mode or a valid git mode followed by a
+///     space, then anything (in case we are just pointing in the memory from the Git Tree
+///     representation)
+/// Return value:
+///     The value (`u16`) given a valid input or `None` otherwise
+pub const fn parse_git_mode(i: &[u8]) -> Option<u16> {
+    let mut mode = 0;
+    let mut idx = 0;
+    // const fn, this is why we can't have nice things (like `.iter().any()`)
+    while idx < i.len() {
+        let b = i[idx];
+        // Delimiter, return what we got
+        if b == b' ' {
+            return Some(mode);
+        }
+        // Not a pure octal input
+        if b < b'0' || b > b'7' {
+            return None;
+        }
+        // More than 6 octal digits we must have hit the delimiter or the input was malformed
+        if idx > 6 {
+            return None;
+        }
+        mode = (mode << 3) + (b - b'0') as u16;
+        idx += 1;
+    }
+    Some(mode)
+}
+
+/// Just a place-holder until the `slice_split_once` feature stabilizes
+fn split_once(slice: &[u8], value: u8) -> Option<(&'_ [u8], &'_ [u8])> {
+    let mut iterator = slice.splitn(2, |b| *b == value);
+    let first = iterator.next();
+    let second = iterator.next();
+    first.and_then(|first| second.map(|second| (first, second)))
+}
+
+/// From the slice we get from a Git Tree representation, extract and parse the "mode" part
+/// Return:
+///   * `Some((mode as u16, (mode as slice, rest as slice)))` if the input slice is valid
+///   * `None` otherwise
+#[allow(clippy::type_complexity)]
+fn extract_git_mode(i: &[u8]) -> Option<(u16, (&[u8], &[u8]))> {
+    if let Some((mode_slice, rest_slice)) = split_once(i, b' ') {
+        if rest_slice.is_empty() {
+            return None;
+        }
+
+        parse_git_mode(mode_slice).map(|mode_num| (mode_num, (mode_slice, rest_slice)))
+    } else {
+        None
+    }
+}
+
+impl TryFrom<u32> for tree::EntryMode {
+    type Error = u32;
+
+    fn try_from(mode: u32) -> Result<Self, Self::Error> {
+        Ok(match mode {
+            0o40000 | 0o120000 | 0o160000 => (mode as u16).into(),
+            blob_mode if blob_mode & 0o100000 == 0o100000 => (mode as u16).into(),
+            _ => return Err(mode),
+        })
+    }
+}
+
 /// The mode of items storable in a tree, similar to the file mode on a unix file system.
 ///
-/// Used in [`mutable::Entry`][crate::tree::Entry] and [`EntryRef`].
+/// Used in [`mutable::Entry`][crate::tree::Entry].
 ///
 /// Note that even though it can be created from any `u16`, it should be preferable to
 /// create it by converting [`EntryKind`] into `EntryMode`.
 #[derive(Clone, Copy, PartialEq, Eq, Ord, PartialOrd, Hash)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
-pub struct EntryMode(pub u16);
+pub struct EntryMode {
+    pub(crate) value: u16,
+    pub(crate) git_representation: [u8; 6],
+}
 
 impl std::fmt::Debug for EntryMode {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "EntryMode({:#o})", self.0)
+        write!(f, "EntryMode(0o{})", self.as_bstr())
+    }
+}
+
+impl std::fmt::Octal for EntryMode {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.as_bstr())
     }
 }
 
@@ -72,9 +153,41 @@ pub enum EntryKind {
     Commit = 0o160000,
 }
 
-impl From<EntryKind> for EntryMode {
-    fn from(value: EntryKind) -> Self {
-        EntryMode(value as u16)
+// Mask away the bottom 12 bits and the top 2 bits
+const IFMT: u16 = 0o170000;
+const fn parse_entry_kind_from_value(mode: u16) -> EntryKind {
+    let etype = mode & IFMT;
+    if etype == 0o100000 {
+        if mode & 0o000100 == 0o000100 {
+            EntryKind::BlobExecutable
+        } else {
+            EntryKind::Blob
+        }
+    } else if etype == EntryKind::Link as u16 {
+        EntryKind::Link
+    } else if etype == EntryKind::Tree as u16 {
+        EntryKind::Tree
+    } else {
+        EntryKind::Commit
+    }
+}
+
+impl From<u16> for EntryKind {
+    fn from(mode: u16) -> Self {
+        parse_entry_kind_from_value(mode)
+    }
+}
+
+impl From<u16> for EntryMode {
+    fn from(value: u16) -> Self {
+        let kind: EntryKind = value.into();
+        Self::from(kind)
+    }
+}
+
+impl From<EntryMode> for u16 {
+    fn from(value: EntryMode) -> Self {
+        value.value
     }
 }
 
@@ -88,70 +201,73 @@ impl From<EntryMode> for EntryKind {
 impl EntryKind {
     /// Return the representation as used in the git internal format.
     pub fn as_octal_str(&self) -> &'static BStr {
+        self.as_octal_bytes().as_bstr()
+    }
+
+    /// Return the representation as used in the git internal format.
+    pub fn as_octal_bytes(&self) -> &'static [u8] {
         use EntryKind::*;
-        let bytes: &[u8] = match self {
+        match self {
             Tree => b"40000",
             Blob => b"100644",
             BlobExecutable => b"100755",
             Link => b"120000",
             Commit => b"160000",
-        };
-        bytes.into()
+        }
+    }
+    /// Return the representation as a human readable description
+    pub fn as_descriptive_str(&self) -> &'static str {
+        use EntryKind::*;
+        match self {
+            Tree => "tree",
+            Blob => "blob",
+            BlobExecutable => "exe",
+            Link => "link",
+            Commit => "commit",
+        }
     }
 }
 
-impl std::ops::Deref for EntryMode {
-    type Target = u16;
-
-    fn deref(&self) -> &Self::Target {
-        &self.0
+impl From<EntryKind> for EntryMode {
+    fn from(value: EntryKind) -> Self {
+        let mut git_representation = [b' '; 6];
+        git_representation[..value.as_octal_str().len()].copy_from_slice(value.as_octal_str());
+        EntryMode {
+            git_representation,
+            value: value as u16,
+        }
     }
 }
 
-const IFMT: u16 = 0o170000;
-
 impl EntryMode {
     /// Discretize the raw mode into an enum with well-known state while dropping unnecessary details.
     pub const fn kind(&self) -> EntryKind {
-        let etype = self.0 & IFMT;
-        if etype == 0o100000 {
-            if self.0 & 0o000100 == 0o000100 {
-                EntryKind::BlobExecutable
-            } else {
-                EntryKind::Blob
-            }
-        } else if etype == EntryKind::Link as u16 {
-            EntryKind::Link
-        } else if etype == EntryKind::Tree as u16 {
-            EntryKind::Tree
-        } else {
-            EntryKind::Commit
-        }
-    }
-
-    /// Return true if this entry mode represents a Tree/directory
-    pub const fn is_tree(&self) -> bool {
-        self.0 & IFMT == EntryKind::Tree as u16
+        parse_entry_kind_from_value(self.value)
     }
 
     /// Return true if this entry mode represents the commit of a submodule.
     pub const fn is_commit(&self) -> bool {
-        self.0 & IFMT == EntryKind::Commit as u16
+        matches!(self.kind(), EntryKind::Commit)
     }
 
     /// Return true if this entry mode represents a symbolic link
     pub const fn is_link(&self) -> bool {
-        self.0 & IFMT == EntryKind::Link as u16
+        matches!(self.kind(), EntryKind::Link)
+    }
+
+    /// Return true if this entry mode represents anything BUT Tree/directory
+    pub const fn is_tree(&self) -> bool {
+        matches!(self.kind(), EntryKind::Tree)
     }
 
     /// Return true if this entry mode represents anything BUT Tree/directory
     pub const fn is_no_tree(&self) -> bool {
-        self.0 & IFMT != EntryKind::Tree as u16
+        !matches!(self.kind(), EntryKind::Tree)
     }
 
     /// Return true if the entry is any kind of blob.
     pub const fn is_blob(&self) -> bool {
-        self.0 & IFMT == 0o100000
+        matches!(self.kind(), EntryKind::Blob | EntryKind::BlobExecutable)
     }
 
     /// Return true if the entry is an executable blob.
@@ -167,37 +283,26 @@ impl EntryMode {
         )
     }
 
-    /// Represent the mode as descriptive string.
-    pub const fn as_str(&self) -> &'static str {
-        use EntryKind::*;
-        match self.kind() {
-            Tree => "tree",
-            Blob => "blob",
-            BlobExecutable => "exe",
-            Link => "link",
-            Commit => "commit",
+    /// How many bytes of the backing representation are significant?
+    #[allow(clippy::len_without_is_empty)]
+    pub fn len(&self) -> usize {
+        if let Some(delim) = self.git_representation.iter().position(|b| *b == b' ') {
+            delim
+        } else {
+            self.git_representation.len()
         }
     }
 
     /// Return the representation as used in the git internal format, which is octal and written
     /// to the `backing` buffer. The respective sub-slice that was written to is returned.
-    pub fn as_bytes<'a>(&self, backing: &'a mut [u8; 6]) -> &'a BStr {
-        if self.0 == 0 {
-            std::slice::from_ref(&b'0')
-        } else {
-            let mut nb = 0;
-            let mut n = self.0;
-            while n > 0 {
-                let remainder = (n % 8) as u8;
-                backing[nb] = b'0' + remainder;
-                n /= 8;
-                nb += 1;
-            }
-            let res = &mut backing[..nb];
-            res.reverse();
-            res
-        }
-        .into()
+    pub fn as_bytes(&self) -> &'_ [u8] {
+        &self.git_representation[..self.len()]
+    }
+
+    /// Return the representation as used in the git internal format, which is octal and written
+    /// to the `backing` buffer. The respective sub-slice that was written to is returned.
+    pub fn as_bstr(&self) -> &'_ BStr {
+        self.as_bytes().as_bstr()
     }
 }