[ref] refactor

Author: Byron

git-object/tests/immutable/tag.rs

@@ -9,7 +9,10 @@ mod method {
     fn target() -> crate::Result {
         let fixture = fixture_bytes("tag", "signed.txt");
         let tag = Tag::from_bytes(&fixture)?;
-        assert_eq!(tag.target(), hex_to_id("ffa700b4aca13b80cb6b98a078e7c96804f8e0ec"));
+        assert_eq!(
+            tag.target.borrow(),
+            hex_to_id("ffa700b4aca13b80cb6b98a078e7c96804f8e0ec")
+        );
         assert_eq!(tag.target, "ffa700b4aca13b80cb6b98a078e7c96804f8e0ec".as_bytes());
         Ok(())
     }

git-ref/src/mutable.rs

@@ -33,6 +33,7 @@ impl TryFrom<BString> for FullName {
     }
 }
 
+// TODO: remove this one to be consistent with borrowed version
 impl AsRef<BStr> for FullName {
     fn as_ref(&self) -> &BStr {
         self.0.as_bstr()
@@ -59,6 +60,10 @@ impl FullName {
     pub fn into_inner(self) -> BString {
         self.0
     }
+    /// Return ourselves as byte string which is a valid refname
+    pub fn as_bstr(&self) -> &BStr {
+        self.0.as_bstr()
+    }
 }
 
 /// Denotes a ref target, equivalent to [`Kind`][super::Kind], but with mutable data.

git-ref/src/store/file/loose/find.rs

@@ -119,11 +119,12 @@ impl file::Store {
             }
             Some(c) => c,
         };
-        Ok(Some(
-            file::Reference::try_from_path(self, &relative_path, &contents)
+        Ok(Some({
+            let full_name = path_to_name(&relative_path);
+            file::Reference::try_from_path(self, FullName(full_name), &contents)
                 .map(file::loose_then_packed::Reference::Loose)
-                .map_err(|err| Error::ReferenceCreation { err, relative_path })?,
-        ))
+                .map_err(|err| Error::ReferenceCreation { err, relative_path })?
+        }))
     }
 }
 
@@ -256,4 +257,5 @@ mod error {
         }
     }
 }
+use crate::mutable::FullName;
 pub use error::Error;

git-ref/src/store/file/loose/iter.rs

@@ -1,5 +1,5 @@
-use crate::store::file;
-use bstr::{BString, ByteSlice};
+use crate::{mutable::FullName, store::file};
+use bstr::ByteSlice;
 use git_features::fs::walkdir::DirEntryIter;
 use os_str_bytes::OsStrBytes;
 use std::{
@@ -11,31 +11,20 @@ use std::{
 pub(in crate::store::file) struct SortedLoosePaths {
     base: PathBuf,
     file_walk: DirEntryIter,
-    mode: LoosePathsMode,
-}
-
-enum LoosePathsMode {
-    Paths,
-    PathsAndNames,
 }
 
 impl SortedLoosePaths {
-    pub fn at_root(path: impl AsRef<Path>, base: impl Into<PathBuf>) -> Self {
-        Self::new(path.as_ref(), base.into(), LoosePathsMode::Paths)
-    }
-
     pub fn at_root_with_names(path: impl AsRef<Path>, base: impl Into<PathBuf>) -> Self {
-        Self::new(path.as_ref(), base.into(), LoosePathsMode::PathsAndNames)
-    }
-
-    fn new(path: &Path, base: PathBuf, mode: LoosePathsMode) -> Self {
         let file_walk = git_features::fs::walkdir_sorted_new(path).into_iter();
-        SortedLoosePaths { base, file_walk, mode }
+        SortedLoosePaths {
+            base: base.into(),
+            file_walk,
+        }
     }
 }
 
 impl Iterator for SortedLoosePaths {
-    type Item = std::io::Result<(PathBuf, Option<BString>)>;
+    type Item = std::io::Result<(PathBuf, FullName)>;
 
     fn next(&mut self) -> Option<Self::Item> {
         while let Some(entry) = self.file_walk.next() {
@@ -52,15 +41,11 @@ impl Iterator for SortedLoosePaths {
                     #[cfg(windows)]
                     let full_name: Vec<u8> = full_name.into_owned().replace(b"\\", b"/");
 
-                    use LoosePathsMode::*;
                     if git_validate::reference::name_partial(full_name.as_bstr()).is_ok() {
-                        let name = match self.mode {
-                            Paths => None,
-                            #[cfg(not(windows))]
-                            PathsAndNames => Some(full_name.into_owned().into()),
-                            #[cfg(windows)]
-                            PathsAndNames => Some(full_name.into()),
-                        };
+                        #[cfg(not(windows))]
+                        let name = FullName(full_name.into_owned().into());
+                        #[cfg(windows)]
+                        let name = FullName(full_name.into());
                         return Some(Ok((full_path, name)));
                     } else {
                         continue;
@@ -86,7 +71,7 @@ impl<'a> Loose<'a> {
     pub fn at_root(store: &'a file::Store, root: impl AsRef<Path>, base: impl Into<PathBuf>) -> Self {
         Loose {
             parent: store,
-            ref_paths: SortedLoosePaths::at_root(root, base),
+            ref_paths: SortedLoosePaths::at_root_with_names(root, base),
             buf: Vec::new(),
         }
     }
@@ -97,26 +82,25 @@ impl<'a> Iterator for Loose<'a> {
 
     fn next(&mut self) -> Option<Self::Item> {
         self.ref_paths.next().map(|res| {
-            res.map_err(loose::Error::Traversal)
-                .and_then(|(validated_path, _name)| {
-                    std::fs::File::open(&validated_path)
-                        .and_then(|mut f| {
-                            self.buf.clear();
-                            f.read_to_end(&mut self.buf)
-                        })
-                        .map_err(loose::Error::ReadFileContents)
-                        .and_then(|_| {
-                            let relative_path = validated_path
-                                .strip_prefix(&self.ref_paths.base)
-                                .expect("root contains path");
-                            file::Reference::try_from_path(self.parent, relative_path, &self.buf).map_err(|err| {
-                                loose::Error::ReferenceCreation {
-                                    err,
-                                    relative_path: relative_path.into(),
-                                }
-                            })
+            res.map_err(loose::Error::Traversal).and_then(|(validated_path, name)| {
+                std::fs::File::open(&validated_path)
+                    .and_then(|mut f| {
+                        self.buf.clear();
+                        f.read_to_end(&mut self.buf)
+                    })
+                    .map_err(loose::Error::ReadFileContents)
+                    .and_then(|_| {
+                        let relative_path = validated_path
+                            .strip_prefix(&self.ref_paths.base)
+                            .expect("root contains path");
+                        file::Reference::try_from_path(self.parent, name, &self.buf).map_err(|err| {
+                            loose::Error::ReferenceCreation {
+                                err,
+                                relative_path: relative_path.into(),
+                            }
                         })
-                })
+                    })
+            })
         })
     }
 }

git-ref/src/store/file/mod.rs

@@ -21,9 +21,9 @@ impl Default for WriteReflog {
 pub struct Reference<'a> {
     parent: &'a Store,
     /// The path to uniquely identify this ref within its store.
-    // TODO: make this a FullName(BString) and remove the `relative_path()` method entirely.
-    relative_path: PathBuf,
-    target: mutable::Target,
+    pub name: mutable::FullName,
+    /// The target of the reference, either a symbolic reference by full name or an object by its id.
+    pub target: mutable::Target,
 }
 
 /// A store for reference which uses plain files.

git-ref/src/store/file/overlay/iter.rs

@@ -3,7 +3,6 @@ use crate::{
     file::path_to_name,
     store::{file, packed},
 };
-use bstr::BString;
 use std::{
     cmp::Ordering,
     io::Read,
@@ -27,19 +26,21 @@ impl<'p, 's> LooseThenPacked<'p, 's> {
         })
     }
 
-    fn convert_loose(&mut self, res: std::io::Result<(PathBuf, Option<BString>)>) -> Result<Reference<'p, 's>, Error> {
-        let (refpath, _name) = res.map_err(Error::Traversal)?;
+    fn convert_loose(&mut self, res: std::io::Result<(PathBuf, FullName)>) -> Result<Reference<'p, 's>, Error> {
+        let (refpath, name) = res.map_err(Error::Traversal)?;
         std::fs::File::open(&refpath)
             .and_then(|mut f| {
                 self.buf.clear();
                 f.read_to_end(&mut self.buf)
             })
             .map_err(Error::ReadFileContents)?;
-        let relative_path = refpath.strip_prefix(&self.parent.base).expect("base contains path");
-        file::Reference::try_from_path(self.parent, relative_path, &self.buf)
+        file::Reference::try_from_path(self.parent, name, &self.buf)
             .map_err(|err| Error::ReferenceCreation {
                 err,
-                relative_path: relative_path.into(),
+                relative_path: refpath
+                    .strip_prefix(&self.parent.base)
+                    .expect("base contains path")
+                    .into(),
             })
             .map(Reference::Loose)
     }
@@ -60,8 +61,8 @@ impl<'p, 's> Iterator for LooseThenPacked<'p, 's> {
                 Some(self.convert_loose(res))
             }
             (Some(Ok(loose)), Some(Ok(packed))) => {
-                let loose_name = loose.1.as_ref().expect("name retrieval configured");
-                match loose_name.as_slice().cmp(packed.full_name) {
+                let loose_name = loose.1.as_ref();
+                match loose_name.cmp(packed.full_name) {
                     Ordering::Less => {
                         let res = self.loose.next().expect("name retrieval configured");
                         Some(self.convert_loose(res))
@@ -154,4 +155,5 @@ mod error {
     }
 }
 
+use crate::mutable::FullName;
 pub use error::Error;

git-ref/src/store/file/overlay/reference.rs

@@ -1,16 +1,14 @@
 use super::Reference;
 use crate::{
-    mutable,
+    mutable, name,
     store::{
         file::{log, loose},
         packed,
     },
+    FullName,
 };
 use git_hash::oid;
-use std::{
-    convert::{TryFrom, TryInto},
-    path::Path,
-};
+use std::convert::{TryFrom, TryInto};
 
 impl<'p, 's> TryFrom<Reference<'p, 's>> for crate::file::Reference<'s> {
     type Error = ();
@@ -32,14 +30,6 @@ impl<'p, 's> Reference<'p, 's> {
         }
     }
 
-    /// Return the full name of this reference as path, only applicable if this is a loose reference.
-    pub fn relative_path(&self) -> Option<&Path> {
-        match self {
-            Reference::Loose(r) => Some(r.relative_path()),
-            Reference::Packed(_) => None,
-        }
-    }
-
     /// For details, see [crate::file::Reference::peel_to_id_in_place].
     pub fn peel_to_id_in_place(
         &mut self,
@@ -96,7 +86,7 @@ impl<'p, 's> Reference<'p, 's> {
     pub fn into_target(self) -> mutable::Target {
         match self {
             Reference::Packed(p) => mutable::Target::Peeled(p.object()),
-            Reference::Loose(r) => r.into_target(),
+            Reference::Loose(r) => r.target,
         }
     }
 
@@ -109,18 +99,18 @@ impl<'p, 's> Reference<'p, 's> {
     }
 
     /// Return the full validated name of the reference. Please note that if the reference is packed, validation can fail here.
-    pub fn name(&self) -> Result<mutable::FullName, git_validate::refname::Error> {
+    pub fn name(&self) -> Result<FullName<'_>, name::Error> {
         match self {
             Reference::Packed(p) => p.full_name.try_into(),
-            Reference::Loose(l) => Ok(l.name()),
+            Reference::Loose(l) => Ok(l.name.borrow()),
         }
     }
 
     /// Return the target to which the reference points to.
     pub fn target(&self) -> mutable::Target {
         match self {
             Reference::Packed(p) => mutable::Target::Peeled(p.target()),
-            Reference::Loose(l) => l.target().to_owned(),
+            Reference::Loose(l) => l.target.clone(),
         }
     }
 }

git-ref/src/store/file/reference/decode.rs

@@ -12,10 +12,7 @@ use nom::{
     IResult,
 };
 use quick_error::quick_error;
-use std::{
-    convert::{TryFrom, TryInto},
-    path::PathBuf,
-};
+use std::convert::{TryFrom, TryInto};
 
 enum MaybeUnsafeState {
     Id(ObjectId),
@@ -56,14 +53,10 @@ impl TryFrom<MaybeUnsafeState> for mutable::Target {
 impl<'a> Reference<'a> {
     /// Create a new reference of the given `parent` store with `relative_path` service as unique identifier
     /// at which the `path_contents` was read to obtain the refs value.
-    pub fn try_from_path(
-        parent: &'a Store,
-        relative_path: impl Into<PathBuf>,
-        path_contents: &[u8],
-    ) -> Result<Self, Error> {
+    pub fn try_from_path(parent: &'a Store, name: mutable::FullName, path_contents: &[u8]) -> Result<Self, Error> {
         Ok(Reference {
             parent,
-            relative_path: relative_path.into(),
+            name,
             target: parse(path_contents)
                 .map_err(|_| Error::Parse(path_contents.into()))?
                 .1

git-ref/src/store/file/reference/logiter.rs

@@ -1,8 +1,4 @@
-use crate::{
-    store::file::{log, loose, Reference},
-    FullName,
-};
-use bstr::ByteSlice;
+use crate::store::file::{log, loose, Reference};
 use std::io::Read;
 
 impl<'a> Reference<'a> {
@@ -13,9 +9,7 @@ impl<'a> Reference<'a> {
     /// If the caller needs to know if it's readable, try to read the log instead with a reverse or forward iterator.
     pub fn log_exists(&self) -> Result<bool, loose::reflog::Error> {
         // NOTE: Have to repeat the implementation of store::reflog_iter here as borrow_check believes impl Iterator binds self
-        use os_str_bytes::OsStrBytes;
-        let name = self.relative_path.as_path().to_raw_bytes();
-        Ok(self.parent.reflog_path(FullName(name.as_bstr())).is_file())
+        Ok(self.parent.reflog_path(self.name.borrow()).is_file())
     }
     /// Return a reflog reverse iterator for this ref, reading chunks from the back into the fixed buffer `buf`.
     ///
@@ -26,9 +20,7 @@ impl<'a> Reference<'a> {
         buf: &'b mut [u8],
     ) -> Result<Option<log::iter::Reverse<'b, std::fs::File>>, loose::reflog::Error> {
         // NOTE: Have to repeat the implementation of store::reflog_iter here as borrow_check believes impl Iterator binds self
-        use os_str_bytes::OsStrBytes;
-        let name = self.relative_path.as_path().to_raw_bytes();
-        let file = match std::fs::File::open(self.parent.reflog_path(FullName(name.as_bstr()))) {
+        let file = match std::fs::File::open(self.parent.reflog_path(self.name.borrow())) {
             Ok(file) => file,
             Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None),
             Err(err) => return Err(err.into()),
@@ -46,9 +38,7 @@ impl<'a> Reference<'a> {
     ) -> Result<Option<impl Iterator<Item = Result<log::Line<'b>, log::iter::decode::Error>>>, loose::reflog::Error>
     {
         // NOTE: Have to repeat the implementation of store::reflog_iter here as borrow_check believes impl Iterator binds self
-        use os_str_bytes::OsStrBytes;
-        let name = self.relative_path.as_path().to_raw_bytes();
-        match std::fs::File::open(self.parent.reflog_path(FullName(name.as_bstr()))) {
+        match std::fs::File::open(self.parent.reflog_path(self.name.borrow())) {
             Ok(mut file) => {
                 buf.clear();
                 file.read_to_end(buf)?;