Merge branch 'head-conversions'

Author: Byron

Cargo.lock

@@ -10,7 +10,9 @@ 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 `peel_to_id_in_place()` was called.
+    /// The fully peeled object to which this reference ultimately points to. 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>,
 }
 

gix-ref/src/raw.rs

@@ -87,7 +87,7 @@ pub(crate) fn url(input: &BStr, protocol_end: usize) -> Result<crate::Url, Error
     let bytes_to_path = input[protocol_end + "://".len()..]
         .iter()
         .filter(|b| !b.is_ascii_whitespace())
-        .skip_while(|b| **b == b'/')
+        .skip_while(|b| **b == b'/' || **b == b'\\')
         .position(|b| *b == b'/')
         .unwrap_or(input.len() - protocol_end);
     if bytes_to_path > MAX_LEN || protocol_end > MAX_LEN {
@@ -169,7 +169,10 @@ pub(crate) fn file_url(input: &BStr, protocol_colon: usize) -> Result<crate::Url
     let input = input_to_utf8(input, UrlKind::Url)?;
     let input_after_protocol = &input[protocol_colon + "://".len()..];
 
-    let Some(first_slash) = input_after_protocol.find('/') else {
+    let Some(first_slash) = input_after_protocol
+        .find('/')
+        .or_else(|| cfg!(windows).then(|| input_after_protocol.find('\\')).flatten())
+    else {
         return Err(Error::MissingRepositoryPath {
             url: input.to_owned().into(),
             kind: UrlKind::Url,

gix-url/src/parse.rs

@@ -1,4 +1,3 @@
-use assert_matches::assert_matches;
 use bstr::ByteSlice;
 use gix_url::Scheme;
 
@@ -93,11 +92,19 @@ fn no_relative_paths_if_protocol() -> crate::Result {
     assert_url_roundtrip("file://../", url(Scheme::File, None, "..", None, b"/"))?;
     assert_url_roundtrip("file://./", url(Scheme::File, None, ".", None, b"/"))?;
     assert_url_roundtrip("file://a/", url(Scheme::File, None, "a", None, b"/"))?;
-    assert_matches!(
+    if cfg!(windows) {
+        assert_eq!(
+            gix_url::parse("file://.\\".into())?,
+            url(Scheme::File, None, ".", None, b"\\"),
+            "we are just as none-sensical as git here due to special handling."
+        );
+    } else {
+        assert_matches::assert_matches!(
             gix_url::parse("file://.\\".into()),
             Err(gix_url::parse::Error::MissingRepositoryPath { .. }),
             "DEVIATION: on windows, this parses with git into something nonesensical Diag: url=file://./ Diag: protocol=file Diag: hostandport=./ Diag: path=//./"
         );
+    }
     Ok(())
 }
 
@@ -122,6 +129,15 @@ mod windows {
     use crate::parse::{assert_url, assert_url_roundtrip, url, url_alternate};
     use gix_url::Scheme;
 
+    #[test]
+    fn reproduce_1063() -> crate::Result {
+        let input = "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmp.vIa4tyjv17";
+        let url_input = "file://C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmp.vIa4tyjv17";
+        assert_url(url_input, url(Scheme::File, None, None, None, input.as_bytes()))?;
+        assert_url(input, url_alternate(Scheme::File, None, None, None, input.as_bytes()))?;
+        Ok(())
+    }
+
     #[test]
     fn url_from_absolute_path() -> crate::Result {
         assert_url(

gix-url/tests/fixtures/fuzzed/very-long5.url

@@ -33,7 +33,6 @@ fn empty_input() {
 fn file_missing_host_path_separator() {
     assert_matches!(parse("file://.."), Err(MissingRepositoryPath { .. }));
     assert_matches!(parse("file://."), Err(MissingRepositoryPath { .. }));
-    assert_matches!(parse("file://.\\"), Err(MissingRepositoryPath { .. }));
     assert_matches!(parse("file://a"), Err(MissingRepositoryPath { .. }));
 }
 

gix-url/tests/parse/file.rs

@@ -139,7 +139,7 @@ mod unknown {
 
 #[test]
 fn fuzzed() {
-    for name in ["very-long4", "very-long3", "very-long2", "very-long"] {
+    for name in ["very-long5", "very-long4", "very-long3", "very-long2", "very-long"] {
         let base = Path::new("tests").join("fixtures").join("fuzzed");
         let location = base.join(Path::new(name).with_extension("url"));
         let url = std::fs::read(&location).unwrap();

gix-url/tests/parse/invalid.rs

@@ -11,8 +11,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
     let mut most_recent_commit_id = None;
     let num_commits = repo
         .head()?
-        .into_fully_peeled_id()
-        .ok_or("Cannot provide meaningful stats on empty repos")??
+        .try_into_peeled_id()?
+        .ok_or("Cannot provide meaningful stats on empty repos")?
         .ancestors()
         .all()?
         .map_while(Result::ok)

gix-url/tests/parse/mod.rs

@@ -87,7 +87,7 @@ pub mod main_worktree {
             let workdir = repo.work_dir().ok_or_else(|| Error::BareRepository {
                 git_dir: repo.git_dir().to_owned(),
             })?;
-            let root_tree = match repo.head()?.peel_to_id_in_place().transpose()? {
+            let root_tree = match repo.head()?.try_peel_to_id_in_place()? {
                 Some(id) => id.object().expect("downloaded from remote").peel_to_tree()?.id,
                 None => {
                     return Ok((

gix/examples/stats.rs

@@ -1,12 +1,14 @@
 use crate::{
     ext::{ObjectIdExt, ReferenceExt},
+    head::Kind,
     Head,
 };
 
 mod error {
     use crate::{object, reference};
 
-    /// The error returned by [`Head::peel_to_id_in_place()`][super::Head::peel_to_id_in_place()] and [`Head::into_fully_peeled_id()`][super::Head::into_fully_peeled_id()].
+    /// The error returned by [`Head::peel_to_id_in_place()`](super::Head::try_peel_to_id_in_place())
+    /// and [`Head::into_fully_peeled_id()`](super::Head::try_into_peeled_id()).
     #[derive(Debug, thiserror::Error)]
     #[allow(missing_docs)]
     pub enum Error {
@@ -19,13 +21,11 @@ mod error {
 
 pub use error::Error;
 
-use crate::head::Kind;
-
 ///
-pub mod to_commit {
+pub mod into_id {
     use crate::object;
 
-    /// The error returned by [`Head::peel_to_commit_in_place()`][super::Head::peel_to_commit_in_place()].
+    /// The error returned by [`Head::into_peeled_id()`](super::Head::into_peeled_id()).
     #[derive(Debug, thiserror::Error)]
     #[allow(missing_docs)]
     pub enum Error {
@@ -38,86 +38,122 @@ pub mod to_commit {
     }
 }
 
+///
+pub mod to_commit {
+    use crate::object;
+
+    /// The error returned by [`Head::peel_to_commit_in_place()`](super::Head::peel_to_commit_in_place()).
+    #[derive(Debug, thiserror::Error)]
+    #[allow(missing_docs)]
+    pub enum Error {
+        #[error(transparent)]
+        PeelToObject(#[from] super::to_object::Error),
+        #[error(transparent)]
+        ObjectKind(#[from] object::try_into::Error),
+    }
+}
+
+///
+pub mod to_object {
+    /// The error returned by [`Head::peel_to_object_in_place()`](super::Head::peel_to_object_in_place()).
+    #[derive(Debug, thiserror::Error)]
+    #[allow(missing_docs)]
+    pub enum Error {
+        #[error(transparent)]
+        Peel(#[from] super::Error),
+        #[error("Branch '{name}' does not have any commits")]
+        Unborn { name: gix_ref::FullName },
+    }
+}
+
 impl<'repo> Head<'repo> {
-    // TODO: tests
-    /// Peel this instance to make obtaining its final target id possible, while returning an error on unborn heads.
-    pub fn peeled(mut self) -> Result<Self, Error> {
-        self.peel_to_id_in_place().transpose()?;
-        Ok(self)
+    /// Peel this instance and consume it to make obtaining its final target id possible, while returning an error on unborn heads.
+    ///
+    /// The final target is obtained by following symbolic references and peeling tags to their final destination, which
+    /// typically is a commit, but can be any object.
+    pub fn into_peeled_id(mut self) -> Result<crate::Id<'repo>, into_id::Error> {
+        self.try_peel_to_id_in_place()?;
+        self.id().ok_or_else(|| match self.kind {
+            Kind::Symbolic(gix_ref::Reference { name, .. }) | Kind::Unborn(name) => into_id::Error::Unborn { name },
+            Kind::Detached { .. } => unreachable!("id can be returned after peeling"),
+        })
+    }
+
+    /// Peel this instance and consume it to make obtaining its final target object possible, while returning an error on unborn heads.
+    ///
+    /// The final target is obtained by following symbolic references and peeling tags to their final destination, which
+    /// typically is a commit, but can be any object as well.
+    pub fn into_peeled_object(mut self) -> Result<crate::Object<'repo>, to_object::Error> {
+        self.peel_to_object_in_place()
+    }
+
+    /// Consume this instance and transform it into the final object that it points to, or `Ok(None)` if the `HEAD`
+    /// reference is yet to be born.
+    ///
+    /// The final target is obtained by following symbolic references and peeling tags to their final destination, which
+    /// typically is a commit, but can be any object.
+    pub fn try_into_peeled_id(mut self) -> Result<Option<crate::Id<'repo>>, Error> {
+        self.try_peel_to_id_in_place()
     }
 
-    // TODO: tests
-    // TODO: Fix this! It's not consistently peeling tags. The whole peeling business should be reconsidered to do what people usually
-    //       want which is to peel references, if present, and then peel objects with control over which object type to end at.
-    //       Finding a good interface for that isn't easy as ideally, it's an iterator that shows the intermediate objects so the user
-    //       can select which tag of a chain to choose.
     /// Follow the symbolic reference of this head until its target object and peel it by following tag objects until there is no
     /// more object to follow, and return that object id.
     ///
-    /// Returns `None` if the head is unborn.
-    pub fn peel_to_id_in_place(&mut self) -> Option<Result<crate::Id<'repo>, Error>> {
-        Some(match &mut self.kind {
-            Kind::Unborn(_name) => return None,
+    /// Returns `Ok(None)` if the head is unborn.
+    ///
+    /// The final target is obtained by following symbolic references and peeling tags to their final destination, which
+    /// typically is a commit, but can be any object.
+    pub fn try_peel_to_id_in_place(&mut self) -> Result<Option<crate::Id<'repo>>, Error> {
+        Ok(Some(match &mut self.kind {
+            Kind::Unborn(_name) => return Ok(None),
             Kind::Detached {
                 peeled: Some(peeled), ..
-            } => Ok((*peeled).attach(self.repo)),
+            } => (*peeled).attach(self.repo),
             Kind::Detached { peeled: None, target } => {
-                match target
-                    .attach(self.repo)
-                    .object()
-                    .map_err(Into::into)
-                    .and_then(|obj| obj.peel_tags_to_end().map_err(Into::into))
-                    .map(|peeled| peeled.id)
-                {
-                    Ok(peeled) => {
-                        self.kind = Kind::Detached {
-                            peeled: Some(peeled),
-                            target: *target,
-                        };
-                        Ok(peeled.attach(self.repo))
+                let id = target.attach(self.repo);
+                if id.header()?.kind() == gix_object::Kind::Commit {
+                    id
+                } else {
+                    match id.object()?.peel_tags_to_end() {
+                        Ok(obj) => {
+                            self.kind = Kind::Detached {
+                                peeled: Some(obj.id),
+                                target: *target,
+                            };
+                            obj.id()
+                        }
+                        Err(err) => return Err(err.into()),
                     }
-                    Err(err) => Err(err),
                 }
             }
             Kind::Symbolic(r) => {
                 let mut nr = r.clone().attach(self.repo);
-                let peeled = nr.peel_to_id_in_place().map_err(Into::into);
+                let peeled = nr.peel_to_id_in_place();
                 *r = nr.detach();
-                peeled
+                peeled?
             }
-        })
+        }))
     }
 
-    // TODO: tests
-    // TODO: something similar in `crate::Reference`
     /// Follow the symbolic reference of this head until its target object and peel it by following tag objects until there is no
     /// more object to follow, transform the id into a commit if possible and return that.
     ///
     /// Returns an error if the head is unborn or if it doesn't point to a commit.
-    pub fn peel_to_commit_in_place(&mut self) -> Result<crate::Commit<'repo>, to_commit::Error> {
-        let id = self.peel_to_id_in_place().ok_or_else(|| to_commit::Error::Unborn {
-            name: self.referent_name().expect("unborn").to_owned(),
-        })??;
+    pub fn peel_to_object_in_place(&mut self) -> Result<crate::Object<'repo>, to_object::Error> {
+        let id = self
+            .try_peel_to_id_in_place()?
+            .ok_or_else(|| to_object::Error::Unborn {
+                name: self.referent_name().expect("unborn").to_owned(),
+            })?;
         id.object()
-            .map_err(|err| to_commit::Error::Peel(Error::FindExistingObject(err)))
-            .and_then(|object| object.try_into_commit().map_err(Into::into))
+            .map_err(|err| to_object::Error::Peel(Error::FindExistingObject(err)))
     }
 
-    /// Consume this instance and transform it into the final object that it points to, or `None` if the `HEAD`
-    /// reference is yet to be born.
-    pub fn into_fully_peeled_id(self) -> Option<Result<crate::Id<'repo>, Error>> {
-        Some(match self.kind {
-            Kind::Unborn(_name) => return None,
-            Kind::Detached {
-                peeled: Some(peeled), ..
-            } => Ok(peeled.attach(self.repo)),
-            Kind::Detached { peeled: None, target } => target
-                .attach(self.repo)
-                .object()
-                .map_err(Into::into)
-                .and_then(|obj| obj.peel_tags_to_end().map_err(Into::into))
-                .map(|obj| obj.id.attach(self.repo)),
-            Kind::Symbolic(r) => r.attach(self.repo).peel_to_id_in_place().map_err(Into::into),
-        })
+    /// Follow the symbolic reference of this head until its target object and peel it by following tag objects until there is no
+    /// more object to follow, transform the id into a commit if possible and return that.
+    ///
+    /// Returns an error if the head is unborn or if it doesn't point to a commit.
+    pub fn peel_to_commit_in_place(&mut self) -> Result<crate::Commit<'repo>, to_commit::Error> {
+        Ok(self.peel_to_object_in_place()?.try_into_commit()?)
     }
 }

gix/src/clone/checkout.rs

@@ -43,9 +43,7 @@ pub mod head_id {
         #[error(transparent)]
         Head(#[from] crate::reference::find::existing::Error),
         #[error(transparent)]
-        PeelToId(#[from] crate::head::peel::Error),
-        #[error("Branch '{name}' does not have any commits")]
-        Unborn { name: gix_ref::FullName },
+        PeelToId(#[from] crate::head::peel::into_id::Error),
     }
 }
 
@@ -69,9 +67,7 @@ pub mod head_tree_id {
     #[allow(missing_docs)]
     pub enum Error {
         #[error(transparent)]
-        Head(#[from] crate::reference::find::existing::Error),
-        #[error(transparent)]
-        PeelToCommit(#[from] crate::head::peel::to_commit::Error),
+        HeadCommit(#[from] crate::reference::head_commit::Error),
         #[error(transparent)]
         DecodeCommit(#[from] gix_object::decode::Error),
     }

gix/src/head/peel.rs

@@ -174,20 +174,16 @@ impl crate::Repository {
         .attach(self))
     }
 
-    /// Resolve the `HEAD` reference, follow and peel its target and obtain its object id.
+    /// Resolve the `HEAD` reference, follow and peel its target and obtain its object id,
+    /// following symbolic references and tags until a commit is found.
     ///
     /// Note that this may fail for various reasons, most notably because the repository
     /// is freshly initialized and doesn't have any commits yet.
     ///
     /// Also note that the returned id is likely to point to a commit, but could also
     /// point to a tree or blob. It won't, however, point to a tag as these are always peeled.
     pub fn head_id(&self) -> Result<crate::Id<'_>, reference::head_id::Error> {
-        let mut head = self.head()?;
-        head.peel_to_id_in_place()
-            .ok_or_else(|| reference::head_id::Error::Unborn {
-                name: head.referent_name().expect("unborn").to_owned(),
-            })?
-            .map_err(Into::into)
+        Ok(self.head()?.into_peeled_id()?)
     }
 
     /// Return the name to the symbolic reference `HEAD` points to, or `None` if the head is detached.
@@ -203,7 +199,8 @@ impl crate::Repository {
         Ok(self.head()?.try_into_referent())
     }
 
-    /// Return the commit object the `HEAD` reference currently points to after peeling it fully.
+    /// Return the commit object the `HEAD` reference currently points to after peeling it fully,
+    /// following symbolic references and tags until a commit is found.
     ///
     /// Note that this may fail for various reasons, most notably because the repository
     /// is freshly initialized and doesn't have any commits yet. It could also fail if the
@@ -212,13 +209,14 @@ impl crate::Repository {
         Ok(self.head()?.peel_to_commit_in_place()?)
     }
 
-    /// Return the tree id the `HEAD` reference currently points to after peeling it fully.
+    /// Return the tree id the `HEAD` reference currently points to after peeling it fully,
+    /// following symbolic references and tags until a commit is found.
     ///
     /// Note that this may fail for various reasons, most notably because the repository
     /// is freshly initialized and doesn't have any commits yet. It could also fail if the
     /// head does not point to a commit.
     pub fn head_tree_id(&self) -> Result<crate::Id<'_>, reference::head_tree_id::Error> {
-        Ok(self.head()?.peel_to_commit_in_place()?.tree_id()?)
+        Ok(self.head_commit()?.tree_id()?)
     }
 
     /// Find the reference with the given partial or full `name`, like `main`, `HEAD`, `heads/branch` or `origin/other`,

gix/src/reference/errors.rs

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:1de90b3e0c6547fda6759fb5df5e4bdf410dbe062ef9ef4f972308452c3224d7
+size 11508