path::discover() now returns the shortest path. (#301)

If and only if it canonicalized the source path. That way, users will
still get a familiar path. This is due to `parent()` not operating
in the file system, which otherwise would be equivalent to `..`,
but that's not how we work.

Maybe we should overhaul the way this works to use `../` instead
and just 'absoluteize' the path later (std::path::absolute()) is
on the way for that.

Author: Byron

git-path/src/convert.rs

@@ -183,13 +183,15 @@ pub fn to_unix_separators_on_windows<'a>(path: impl Into<Cow<'a, BStr>>) -> Cow<
 /// Replaces windows path separators with slashes, unconditionally.
 ///
 /// **Note** Do not use these and prefer the conditional versions of this method.
+// TODO: use https://lib.rs/crates/path-slash to handle escapes
 pub fn to_unix_separators<'a>(path: impl Into<Cow<'a, BStr>>) -> Cow<'a, BStr> {
     replace(path, b'\\', b'/')
 }
 
 /// Find backslashes and replace them with slashes, which typically resembles a unix path, unconditionally.
 ///
 /// **Note** Do not use these and prefer the conditional versions of this method.
+// TODO: use https://lib.rs/crates/path-slash to handle escapes
 pub fn to_windows_separators<'a>(path: impl Into<Cow<'a, BStr>>) -> Cow<'a, BStr> {
     replace(path, b'/', b'\\')
 }

git-repository/src/path/discover.rs

@@ -8,6 +8,12 @@ pub enum Error {
     InaccessibleDirectory { path: PathBuf },
     #[error("Could find a git repository in '{}' or in any of its parents", .path.display())]
     NoGitRepository { path: PathBuf },
+    #[error("Could find a trusted git repository in '{}' or in any of its parents, candidate at '{}' discarded", .path.display(), .candidate.display())]
+    NoTrustedGitRepository {
+        path: PathBuf,
+        candidate: PathBuf,
+        required: git_sec::Trust,
+    },
     #[error("Could not determine trust level for path '{}'.", .path.display())]
     CheckTrust {
         path: PathBuf,
@@ -37,6 +43,7 @@ impl Default for Options {
 pub(crate) mod function {
     use super::{Error, Options};
     use git_sec::Trust;
+    use std::path::PathBuf;
     use std::{
         borrow::Cow,
         path::{Component, Path},
@@ -57,23 +64,20 @@ pub(crate) mod function {
         // us the parent directory. (`Path::parent` just strips off the last
         // path component, which means it will not do what you expect when
         // working with paths paths that contain '..'.)
-        let directory = maybe_canonicalize(directory.as_ref()).map_err(|_| Error::InaccessibleDirectory {
-            path: directory.as_ref().into(),
-        })?;
-        if !directory.is_dir() {
-            return Err(Error::InaccessibleDirectory {
-                path: directory.into_owned(),
-            });
+        let directory = directory.as_ref();
+        let dir = maybe_canonicalize(directory).map_err(|_| Error::InaccessibleDirectory { path: directory.into() })?;
+        let is_canonicalized = dir.as_ref() != directory;
+        if !dir.is_dir() {
+            return Err(Error::InaccessibleDirectory { path: dir.into_owned() });
         }
 
-        let filter_by_trust =
-            |x: &std::path::Path, kind: crate::path::Kind| -> Result<Option<(crate::Path, git_sec::Trust)>, Error> {
-                let trust =
-                    git_sec::Trust::from_path_ownership(x).map_err(|err| Error::CheckTrust { path: x.into(), err })?;
-                Ok((trust >= required_trust).then(|| (crate::Path::from_dot_git_dir(x, kind), trust)))
-            };
+        let filter_by_trust = |x: &std::path::Path| -> Result<Option<git_sec::Trust>, Error> {
+            let trust =
+                git_sec::Trust::from_path_ownership(x).map_err(|err| Error::CheckTrust { path: x.into(), err })?;
+            Ok((trust >= required_trust).then(|| (trust)))
+        };
 
-        let mut cursor = directory.clone();
+        let mut cursor = dir.clone();
         'outer: loop {
             for append_dot_git in &[false, true] {
                 if *append_dot_git {
@@ -83,11 +87,38 @@ pub(crate) mod function {
                     }
                 }
                 if let Ok(kind) = path::is::git(&cursor) {
-                    match filter_by_trust(&cursor, kind)? {
-                        Some(res) => break 'outer Ok(res),
+                    match filter_by_trust(&cursor)? {
+                        Some(trust) => {
+                            // TODO: test this more
+                            let path = if is_canonicalized {
+                                match std::env::current_dir() {
+                                    Ok(cwd) => {
+                                        let short_path_components = cwd
+                                            .strip_prefix(&cursor.parent().expect(".git appended"))
+                                            .expect("cwd is always within canonicalized candidate")
+                                            .components()
+                                            .count();
+                                        if short_path_components < cursor.components().count() {
+                                            let mut p = PathBuf::new();
+                                            p.extend(std::iter::repeat("..").take(short_path_components));
+                                            p.push(".git");
+                                            p
+                                        } else {
+                                            cursor.into_owned()
+                                        }
+                                    }
+                                    Err(_) => cursor.into_owned(),
+                                }
+                            } else {
+                                cursor.into_owned()
+                            };
+                            break 'outer Ok((crate::Path::from_dot_git_dir(path, kind), trust));
+                        }
                         None => {
-                            break 'outer Err(Error::NoGitRepository {
-                                path: directory.into_owned(),
+                            break 'outer Err(Error::NoTrustedGitRepository {
+                                path: dir.into_owned(),
+                                candidate: cursor.into_owned(),
+                                required: required_trust,
                             })
                         }
                     }
@@ -100,11 +131,7 @@ pub(crate) mod function {
             }
             match cursor.parent() {
                 Some(parent) => cursor = parent.to_owned().into(),
-                None => {
-                    break Err(Error::NoGitRepository {
-                        path: directory.into_owned(),
-                    })
-                }
+                None => break Err(Error::NoGitRepository { path: dir.into_owned() }),
             }
         }
     }

git-repository/src/path/is.rs

@@ -28,6 +28,7 @@ pub fn bare(git_dir_candidate: impl AsRef<Path>) -> bool {
 /// What constitutes a valid git repository, and what's yet to be implemented, returning the guessed repository kind
 /// purely based on the presence of files. Note that the git-config ultimately decides what's bare.
 ///
+/// * [ ] git files
 /// * [x] a valid head
 /// * [ ] git common directory
 ///   * [ ] respect GIT_COMMON_DIR

git-repository/src/path/mod.rs

@@ -27,7 +27,11 @@ impl Path {
     pub fn from_dot_git_dir(dir: impl Into<PathBuf>, kind: Kind) -> Self {
         let dir = dir.into();
         match kind {
-            Kind::WorkTree => Path::WorkTree(dir.parent().expect("this is a sub-directory").to_owned()),
+            Kind::WorkTree => Path::WorkTree(if dir == std::path::Path::new(".git") {
+                PathBuf::from(".")
+            } else {
+                dir.parent().expect("this is a sub-directory").to_owned()
+            }),
             Kind::Bare => Path::Repository(dir),
         }
     }

git-repository/tests/discover/mod.rs

@@ -1,5 +1,5 @@
 mod existing {
-    use std::path::{Component, PathBuf};
+    use std::path::PathBuf;
 
     use git_repository::Kind;
 
@@ -75,12 +75,9 @@ mod existing {
         let (path, trust) = git_repository::path::discover(&dir)?;
         assert_eq!(path.kind(), Kind::WorkTree);
         assert_eq!(
-            path.as_ref()
-                .components()
-                .filter(|c| matches!(c, Component::ParentDir | Component::CurDir))
-                .count(),
-            0,
-            "there are no relative path components anymore"
+            path.as_ref(),
+            std::path::Path::new(".."),
+            "there is only the minimal amount of relative path components to see this worktree"
         );
         assert_ne!(
             path.as_ref().canonicalize()?,