fix: properly classify non-bare repositories without worktree.

Such repositories claim they are not bare, but also don't have
a workdir with a .git directory inside of it (which is always needed
unless certain variables are set).

Now they are classified as repository that needs more detailed checking
later, while indicating that it 'usually' would be having a worktree
that is configured via git configuration.

Author: Byron

gix-discover/src/is.rs

@@ -9,107 +9,6 @@ pub fn bare(git_dir_candidate: &Path) -> bool {
     !(git_dir_candidate.join("index").exists() || (git_dir_candidate.file_name() == Some(OsStr::new(DOT_GIT_DIR))))
 }
 
-/// Parse `<git_dir_candidate>/config` quickly to evaluate the value of the `bare` line, or return `true` if the file doesn't exist
-/// similar to what`guess_repository_type` seems to be doing.
-/// Return `None` if the `bare` line can't be found or the value of `bare` can't be determined.
-fn bare_by_config(git_dir_candidate: &Path) -> std::io::Result<Option<bool>> {
-    match std::fs::read(git_dir_candidate.join("config")) {
-        Ok(buf) => Ok(config::parse_bare(&buf)),
-        Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(Some(true)),
-        Err(err) => Err(err),
-    }
-}
-
-// Copied and adapted from `gix-config-value::boolean`.
-mod config {
-    use bstr::{BStr, ByteSlice};
-
-    /// Note that we intentionally turn repositories that have a worktree configuration into bare repos,
-    /// as we don't actually parse the worktree from the config file and expect the caller to do the right
-    /// think when seemingly seeing bare repository.
-    /// The reason we do this is to not incorrectly pretend this is a worktree.
-    pub(crate) fn parse_bare(buf: &[u8]) -> Option<bool> {
-        let mut is_bare = None;
-        let mut has_worktree_configuration = false;
-        for line in buf.lines() {
-            if is_bare.is_none() {
-                if let Some(line) = line.trim().strip_prefix(b"bare") {
-                    is_bare = match line.first() {
-                        None => Some(true),
-                        Some(c) if *c == b'=' => parse_bool(line.get(1..)?.trim_start().as_bstr()),
-                        Some(c) if c.is_ascii_whitespace() => match line.split_once_str(b"=") {
-                            Some((_left, right)) => parse_bool(right.trim_start().as_bstr()),
-                            None => Some(true),
-                        },
-                        Some(_other_char_) => None,
-                    };
-                    continue;
-                }
-            }
-            if line.trim().strip_prefix(b"worktree").is_some() {
-                has_worktree_configuration = true;
-                break;
-            }
-        }
-        is_bare.map(|bare| bare || has_worktree_configuration)
-    }
-
-    fn parse_bool(value: &BStr) -> Option<bool> {
-        Some(if parse_true(value) {
-            true
-        } else if parse_false(value) {
-            false
-        } else {
-            use std::str::FromStr;
-            if let Some(integer) = value.to_str().ok().and_then(|s| i64::from_str(s).ok()) {
-                integer != 0
-            } else {
-                return None;
-            }
-        })
-    }
-
-    fn parse_true(value: &BStr) -> bool {
-        value.eq_ignore_ascii_case(b"yes") || value.eq_ignore_ascii_case(b"on") || value.eq_ignore_ascii_case(b"true")
-    }
-
-    fn parse_false(value: &BStr) -> bool {
-        value.eq_ignore_ascii_case(b"no")
-            || value.eq_ignore_ascii_case(b"off")
-            || value.eq_ignore_ascii_case(b"false")
-            || value.is_empty()
-    }
-
-    #[cfg(test)]
-    mod tests {
-        use super::*;
-
-        #[test]
-        fn various() {
-            for (input, expected) in [
-                ("bare=true", Some(true)),
-                ("bare=1", Some(true)),
-                ("bare =1", Some(true)),
-                ("bare= yes", Some(true)),
-                ("bare=false", Some(false)),
-                ("bare=0", Some(false)),
-                ("bare=blah", None),
-                ("bare=", Some(false)),
-                ("bare=  \n", Some(false)),
-                ("bare = true \n", Some(true)),
-                ("\t bare = false \n", Some(false)),
-                ("\n\tbare=true", Some(true)),
-                ("\n\tbare=true\n\tfoo", Some(true)),
-                ("\n\tbare ", Some(true)),
-                ("\n\tbare", Some(true)),
-                ("not found\nreally", None),
-            ] {
-                assert_eq!(parse_bare(input.as_bytes()), expected, "{input:?}");
-            }
-        }
-    }
-}
-
 /// Returns true if `git_dir` is located within a `.git/modules` directory, indicating it's a submodule clone.
 pub fn submodule_git_dir(git_dir: &Path) -> bool {
     let mut last_comp = None;
@@ -141,12 +40,15 @@ pub fn git(git_dir: &Path) -> Result<crate::repository::Kind, crate::is_git::Err
         source: err,
         path: git_dir.into(),
     })?;
-    git_with_metadata(git_dir, git_dir_metadata)
+    // precompose-unicode can't be known here, so we just default it to false, hoping it won't matter.
+    let cwd = gix_fs::current_dir(false)?;
+    git_with_metadata(git_dir, git_dir_metadata, &cwd)
 }
 
 pub(crate) fn git_with_metadata(
     git_dir: &Path,
     git_dir_metadata: std::fs::Metadata,
+    cwd: &Path,
 ) -> Result<crate::repository::Kind, crate::is_git::Error> {
     #[derive(Eq, PartialEq)]
     enum Kind {
@@ -166,6 +68,8 @@ pub(crate) fn git_with_metadata(
     {
         // Fast-path: avoid doing the complete search if HEAD is already not there.
         // TODO(reftable): use a ref-store to lookup HEAD if ref-tables should be supported, or detect ref-tables beforehand.
+        //                 Actually ref-tables still keep a specially marked `HEAD` around, so nothing might be needed here
+        //                 Even though our head-check later would fail without supporting it.
         if !dot_git.join("HEAD").exists() {
             return Err(crate::is_git::Error::MissingHead);
         }
@@ -236,25 +140,26 @@ pub(crate) fn git_with_metadata(
         },
         Kind::MaybeRepo => {
             let conformed_git_dir = if git_dir == Path::new(".") {
-                gix_path::realpath(git_dir)
+                gix_path::realpath_opts(git_dir, cwd, gix_path::realpath::MAX_SYMLINKS)
                     .map(Cow::Owned)
                     .unwrap_or(Cow::Borrowed(git_dir))
             } else {
-                Cow::Borrowed(git_dir)
+                gix_path::normalize(git_dir.into(), cwd).unwrap_or(Cow::Borrowed(git_dir))
             };
             if bare(conformed_git_dir.as_ref()) || conformed_git_dir.extension() == Some(OsStr::new("git")) {
                 crate::repository::Kind::PossiblyBare
             } else if submodule_git_dir(conformed_git_dir.as_ref()) {
                 crate::repository::Kind::SubmoduleGitDir
-            } else if conformed_git_dir.file_name() == Some(OsStr::new(DOT_GIT_DIR))
-                || !bare_by_config(conformed_git_dir.as_ref())
-                    .map_err(|err| crate::is_git::Error::Metadata {
-                        source: err,
-                        path: conformed_git_dir.join("config"),
-                    })?
-                    .ok_or(crate::is_git::Error::Inconclusive)?
-            {
+            } else if conformed_git_dir.file_name() == Some(OsStr::new(DOT_GIT_DIR)) {
                 crate::repository::Kind::WorkTree { linked_git_dir: None }
+            // } else if !bare_by_config(conformed_git_dir.as_ref())
+            //     .map_err(|err| crate::is_git::Error::Metadata {
+            //         source: err,
+            //         path: conformed_git_dir.join("config"),
+            //     })?
+            //     .ok_or(crate::is_git::Error::Inconclusive)?
+            // {
+            //     crate::repository::Kind::WorktreePossiblyInConfiguration
             } else {
                 crate::repository::Kind::PossiblyBare
             }

gix-discover/src/lib.rs

@@ -39,6 +39,8 @@ pub mod is_git {
         Metadata { source: std::io::Error, path: PathBuf },
         #[error("The repository's config file doesn't exist or didn't have a 'bare' configuration or contained core.worktree without value")]
         Inconclusive,
+        #[error("Could not obtain current directory when conforming repository path")]
+        CurrentDir(#[from] std::io::Error),
     }
 }
 

gix-discover/src/repository.rs

@@ -14,8 +14,10 @@ pub enum Path {
     /// The currently checked out or nascent work tree of a git repository
     WorkTree(PathBuf),
     /// The git repository itself, typically bare and without known worktree.
+    /// It could also be non-bare with a worktree configured using git configuration, or no worktree at all despite
+    /// not being bare (due to mis-configuration for example).
     ///
-    /// Note that it might still have linked work-trees which can be accessed later, weather bare or not, or it might be a
+    /// Note that it might still have linked work-trees which can be accessed later, bare or not, or it might be a
     /// submodule git directory in the `.git/modules/**/<name>` directory of the parent repository.
     Repository(PathBuf),
 }
@@ -112,8 +114,11 @@ pub enum Kind {
     /// Note that this is merely a guess at this point as we didn't read the configuration yet.
     ///
     /// Also note that due to optimizing for performance and *just* making an educated *guess in some situations*,
-    /// we may consider a non-bare repository bare if it it doesn't have an index yet due to be freshly initialized.
-    /// The caller is has to handle this, typically by reading the configuration.
+    /// we may consider a non-bare repository bare if it doesn't have an index yet due to be freshly initialized.
+    /// The caller has to handle this, typically by reading the configuration.
+    ///
+    /// It could also be a directory which is non-bare by configuration, but is *not* named `.git`.
+    /// Unusual, but it's possible that a worktree is configured via `core.worktree`.
     PossiblyBare,
     /// A `git` repository along with checked out files in a work tree.
     WorkTree {

gix-discover/src/upwards/mod.rs

@@ -38,11 +38,11 @@ pub(crate) mod function {
         // Normalize the path so that `Path::parent()` _actually_ gives
         // 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 '..'.)
+        // working with paths that contain '..'.)
         let cwd = current_dir.map_or_else(
             || {
                 // The paths we return are relevant to the repository, but at this time it's impossible to know
-                // what `core.precomposeUnicode` is going to be. Hence the one using these paths will have to
+                // what `core.precomposeUnicode` is going to be. Hence, the one using these paths will have to
                 // transform the paths as needed, because we can't. `false` means to leave the obtained path as is.
                 gix_fs::current_dir(false).map(Cow::Owned)
             },
@@ -130,7 +130,7 @@ pub(crate) mod function {
                     cursor_metadata_backup = cursor_metadata.take();
                 }
                 if let Ok(kind) = match cursor_metadata.take() {
-                    Some(metadata) => is_git_with_metadata(&cursor, metadata),
+                    Some(metadata) => is_git_with_metadata(&cursor, metadata, &cwd),
                     None => is_git(&cursor),
                 } {
                     match filter_by_trust(&cursor)? {

gix-discover/tests/discover/is_git/mod.rs

@@ -58,6 +58,30 @@ fn bare_repo_with_index_file_looks_still_looks_like_bare() -> crate::Result {
     Ok(())
 }
 
+#[test]
+fn non_bare_repo_without_workdir() -> crate::Result {
+    let repo = repo_path()?.join("non-bare-without-worktree");
+    let kind = gix_discover::is_git(&repo)?;
+    assert_eq!(
+        kind,
+        gix_discover::repository::Kind::PossiblyBare,
+        "typically due to misconfiguration, but worktrees could also be configured in Git configuration"
+    );
+    Ok(())
+}
+
+#[test]
+fn non_bare_repo_without_workdir_with_index() -> crate::Result {
+    let repo = repo_path()?.join("non-bare-without-worktree-with-index");
+    let kind = gix_discover::is_git(&repo)?;
+    assert_eq!(
+        kind,
+        gix_discover::repository::Kind::PossiblyBare,
+        "this means it has to be validated later"
+    );
+    Ok(())
+}
+
 #[test]
 fn bare_repo_with_index_file_looks_still_looks_like_bare_if_it_was_renamed() -> crate::Result {
     for repo_name in ["bare-with-index-bare", "bare-with-index-no-config-bare"] {

gix-discover/tests/fixtures/make_basic_repo.sh

@@ -15,6 +15,17 @@ mkdir -p some/very/deeply/nested/subdir
 
 git clone --bare --shared . bare.git
 
+git clone --bare --shared . non-bare-without-worktree
+(cd non-bare-without-worktree
+  git config core.bare false
+)
+
+git clone --bare --shared . non-bare-without-worktree-with-index
+(cd non-bare-without-worktree
+  git config core.bare false
+  cp ../.git/index .
+)
+
 git worktree add worktrees/a
 git worktree add worktrees/b-private-dir-deleted
 rm -R .git/worktrees/b-private-dir-deleted