Merge branch 'discover-rel-path'

git-discover/Cargo.toml

@@ -22,6 +22,7 @@ thiserror = "1.0.26"
 
 [dev-dependencies]
 git-testtools = { path = "../tests/tools" }
+serial_test = "0.9.0"
 is_ci = "1.1.1"
 
 [target.'cfg(target_os = "macos")'.dev-dependencies]

git-discover/src/upwards/mod.rs

@@ -141,6 +141,10 @@ pub(crate) mod function {
                     cursor.pop();
                 }
             }
+            if cursor.parent().map_or(false, |p| p.as_os_str().is_empty()) {
+                cursor = cwd.to_path_buf();
+                dir_made_absolute = true;
+            }
             if !cursor.pop() {
                 if dir_made_absolute
                     || matches!(
@@ -151,16 +155,13 @@ pub(crate) mod function {
                     break Err(Error::NoGitRepository { path: dir.into_owned() });
                 } else {
                     dir_made_absolute = true;
-                    cursor = if cursor.as_os_str().is_empty() {
-                        cwd.clone().into_owned()
-                    } else {
-                        // TODO: realpath or absolutize? No test runs into this.
-                        git_path::absolutize(&cursor, cwd.as_ref())
-                            .ok_or_else(|| Error::InvalidInput {
-                                directory: cursor.clone(),
-                            })?
-                            .into_owned()
-                    }
+                    debug_assert!(!cursor.as_os_str().is_empty());
+                    // TODO: realpath or absolutize? No test runs into this.
+                    cursor = git_path::absolutize(&cursor, cwd.as_ref())
+                        .ok_or_else(|| Error::InvalidInput {
+                            directory: cursor.clone(),
+                        })?
+                        .into_owned();
                 }
             }
         }

git-discover/src/upwards/util.rs

@@ -34,18 +34,24 @@ pub(crate) fn shorten_path_with_cwd(cursor: PathBuf, cwd: &Path) -> PathBuf {
 /// Find the number of components parenting the `search_dir` before the first directory in `ceiling_dirs`.
 /// `search_dir` needs to be absolutized, and we absolutize every ceiling as well.
 pub(crate) fn find_ceiling_height(search_dir: &Path, ceiling_dirs: &[PathBuf], cwd: &Path) -> Option<usize> {
+    if ceiling_dirs.is_empty() {
+        return None;
+    }
+
+    let search_realpath;
+    let search_dir = if search_dir.is_absolute() {
+        search_dir
+    } else {
+        search_realpath = git_path::realpath_opts(search_dir, cwd, git_path::realpath::MAX_SYMLINKS).ok()?;
+        search_realpath.as_path()
+    };
     ceiling_dirs
         .iter()
         .filter_map(|ceiling_dir| {
             let mut ceiling_dir = git_path::absolutize(ceiling_dir, cwd)?;
-            match (search_dir.is_absolute(), ceiling_dir.is_absolute()) {
-                (true, false) => ceiling_dir = cwd.join(ceiling_dir.as_ref()).into(),
-                (false, true) => {
-                    let stripped = ceiling_dir.as_ref().strip_prefix(cwd).ok()?.to_owned();
-                    ceiling_dir = stripped.into();
-                }
-                (false, false) | (true, true) => {}
-            };
+            if !ceiling_dir.is_absolute() {
+                ceiling_dir = git_path::absolutize(cwd.join(ceiling_dir.as_ref()), cwd)?;
+            }
             search_dir
                 .strip_prefix(ceiling_dir.as_ref())
                 .ok()

git-discover/tests/fixtures/make_basic_repo.sh

@@ -10,6 +10,7 @@ git commit -q -m c1
 echo hello >> this
 git commit -q -am c2
 
+mkdir subdir
 mkdir -p some/very/deeply/nested/subdir
 
 git clone --bare --shared . bare.git

git-discover/tests/isolated.rs

@@ -0,0 +1,70 @@
+use git_discover::upwards::Options;
+use serial_test::serial;
+use std::path::{Path, PathBuf};
+
+#[test]
+#[serial]
+fn upwards_with_relative_directories_and_optional_ceiling() -> git_testtools::Result {
+    let repo = git_testtools::scripted_fixture_repo_read_only("make_basic_repo.sh")?;
+
+    std::env::set_current_dir(repo.join("subdir"))?;
+    let cwd = std::env::current_dir()?;
+
+    for (search_dir, ceiling_dir_component) in [
+        (".", ".."),
+        (".", "./.."),
+        ("./.", "./.."),
+        (".", "./does-not-exist/../.."),
+    ] {
+        let ceiling_dir = cwd.join(ceiling_dir_component);
+        let (repo_path, _trust) = git_discover::upwards_opts(
+            search_dir,
+            Options {
+                ceiling_dirs: vec![ceiling_dir],
+                ..Default::default()
+            },
+        )
+        .expect("ceiling dir should allow us to discover the repo");
+        assert_repo_is_current_workdir(repo_path, Path::new(".."));
+
+        let (repo_path, _trust) =
+            git_discover::upwards_opts(search_dir, Default::default()).expect("without ceiling dir we see the same");
+        assert_repo_is_current_workdir(repo_path, Path::new(".."));
+
+        let (repo_path, _trust) = git_discover::upwards_opts(
+            search_dir,
+            Options {
+                ceiling_dirs: vec![PathBuf::from("..")],
+                ..Default::default()
+            },
+        )
+        .expect("purely relative ceiling dirs work as well");
+        assert_repo_is_current_workdir(repo_path, Path::new(".."));
+
+        let err = git_discover::upwards_opts(
+            search_dir,
+            Options {
+                ceiling_dirs: vec![PathBuf::from(".")],
+                ..Default::default()
+            },
+        )
+        .unwrap_err();
+
+        assert!(
+            matches!(
+                err,
+                git_discover::upwards::Error::NoGitRepositoryWithinCeiling { ceiling_height: 1, .. }
+            ),
+            "limiting the ceiling to the CWD cannot work as it's just an empty dir"
+        );
+    }
+
+    Ok(())
+}
+
+fn assert_repo_is_current_workdir(path: git_discover::repository::Path, work_dir: &Path) {
+    assert_eq!(
+        path.into_repository_and_work_tree_directories().1.expect("work dir"),
+        work_dir,
+    );
+}

git-path/src/convert.rs

@@ -220,6 +220,11 @@ pub fn to_windows_separators<'a>(path: impl Into<Cow<'a, BStr>>) -> Cow<'a, BStr
 
 /// Resolve relative components virtually without accessing the file system, e.g. turn `a/./b/c/.././..` into `a`,
 /// without keeping intermediate `..` and `/a/../b/..` becomes `/`.
+///
+/// This is particularly useful when manipulating paths that are based on user input, and not resolving intermediate
+/// symlinks keeps the path similar to what the user provided. If that's not desirable, use `[realpath()][crate::realpath()`
+/// instead.
+///
 /// Note that we might access the `current_dir` if we run out of path components to pop off, which is expected to be absolute
 /// as typical return value of `std::env::current_dir()`.
 /// As a `current_dir` like `/c` can be exhausted by paths like `../../r`, `None` will be returned to indicate the inability

git-path/tests/convert/absolutize.rs

@@ -74,6 +74,7 @@ fn trailing_relative_components_are_resolved() {
     let cwd = std::env::current_dir().unwrap();
     for (input, expected) in [
         ("./a/b/./c/../d/..", "./a/b"),
+        ("a/./b/c/.././..", "a"),
         ("/a/b/c/.././../.", "/a"),
         ("./a/..", "."),
         ("a/..", "."),