Merge pull request GitoxideLabs#2403 from GitoxideLabs/improvements

fix: Differentiate between `core.bare` being known or not

Author: Byron

gitoxide-core/src/organize.rs

@@ -20,25 +20,50 @@ pub fn find_git_repository_workdirs(
     threads: Option<usize>,
 ) -> impl Iterator<Item = (PathBuf, gix::repository::Kind)> {
     progress.init(None, progress::count("filesystem items"));
-    fn is_repository(path: &Path) -> Option<gix::repository::Kind> {
+
+    #[derive(Debug, Clone, Copy)]
+    struct RepoInfo {
+        kind: gix::repository::Kind,
+        is_bare: bool,
+    }
+
+    fn is_repository(path: &Path) -> Option<RepoInfo> {
         // Can be git dir or worktree checkout (file)
         if path.file_name() != Some(OsStr::new(".git")) && path.extension() != Some(OsStr::new("git")) {
             return None;
         }
 
         if path.is_dir() {
             if path.join("HEAD").is_file() && path.join("config").is_file() {
-                gix::discover::is_git(path).ok().map(Into::into)
+                gix::discover::is_git(path).ok().map(|discovered_kind| {
+                    let is_bare = discovered_kind.is_bare();
+                    let kind = match discovered_kind {
+                        gix::discover::repository::Kind::PossiblyBare => gix::repository::Kind::Common,
+                        gix::discover::repository::Kind::WorkTree { linked_git_dir: None } => {
+                            gix::repository::Kind::Common
+                        }
+                        gix::discover::repository::Kind::WorkTree {
+                            linked_git_dir: Some(_),
+                        } => gix::repository::Kind::LinkedWorkTree,
+                        gix::discover::repository::Kind::WorkTreeGitDir { .. } => gix::repository::Kind::LinkedWorkTree,
+                        gix::discover::repository::Kind::Submodule { .. } => gix::repository::Kind::Submodule,
+                        gix::discover::repository::Kind::SubmoduleGitDir => gix::repository::Kind::Submodule,
+                    };
+                    RepoInfo { kind, is_bare }
+                })
             } else {
                 None
             }
         } else {
-            // git files are always worktrees
-            Some(gix::repository::Kind::WorkTree { is_linked: true })
+            // git files are always linked worktrees
+            Some(RepoInfo {
+                kind: gix::repository::Kind::LinkedWorkTree,
+                is_bare: false,
+            })
         }
     }
-    fn into_workdir(git_dir: PathBuf, kind: &gix::repository::Kind) -> PathBuf {
-        if matches!(kind, gix::repository::Kind::Bare) || gix::discover::is_bare(&git_dir) {
+    fn into_workdir(git_dir: PathBuf, info: &RepoInfo) -> PathBuf {
+        if info.is_bare {
             git_dir
         } else {
             git_dir.parent().expect("git is never in the root").to_owned()
@@ -47,7 +72,7 @@ pub fn find_git_repository_workdirs(
 
     #[derive(Debug, Default)]
     struct State {
-        kind: Option<gix::repository::Kind>,
+        info: Option<RepoInfo>,
     }
 
     let walk = jwalk::WalkDirGeneric::<((), State)>::new(root)
@@ -64,9 +89,9 @@ pub fn find_git_repository_workdirs(
         let mut found_bare_repo = false;
         for entry in siblings.iter_mut().flatten() {
             let path = entry.path();
-            if let Some(kind) = is_repository(&path) {
-                let is_bare = kind.is_bare();
-                entry.client_state = State { kind: kind.into() };
+            if let Some(info) = is_repository(&path) {
+                let is_bare = info.is_bare;
+                entry.client_state = State { info: info.into() };
                 entry.read_children_path = None;
 
                 found_any_repo = true;
@@ -76,17 +101,17 @@ pub fn find_git_repository_workdirs(
         // Only return paths which are repositories are further participating in the traversal
         // Don't let bare repositories cause siblings to be pruned.
         if found_any_repo && !found_bare_repo {
-            siblings.retain(|e| e.as_ref().map(|e| e.client_state.kind.is_some()).unwrap_or(false));
+            siblings.retain(|e| e.as_ref().map(|e| e.client_state.info.is_some()).unwrap_or(false));
         }
     })
     .into_iter()
     .inspect(move |_| progress.inc())
     .filter_map(Result::ok)
     .filter_map(|mut e| {
         e.client_state
-            .kind
+            .info
             .take()
-            .map(|kind| (into_workdir(e.path(), &kind), kind))
+            .map(|info| (into_workdir(e.path(), &info), info.kind))
     })
 }
 
@@ -108,7 +133,8 @@ fn handle(
     canonicalized_destination: &Path,
     progress: &mut impl Progress,
 ) -> anyhow::Result<()> {
-    if let gix::repository::Kind::WorkTree { is_linked: true } = kind {
+    // Skip linked worktrees - we only handle Common and Submodule kinds
+    if matches!(kind, gix::repository::Kind::LinkedWorkTree) {
         return Ok(());
     }
     fn to_relative(path: PathBuf) -> PathBuf {
@@ -173,13 +199,24 @@ fn handle(
             let mut path = gix_url::expand_path(None, url.path.as_bstr())?;
             match kind {
                 gix::repository::Kind::Submodule => {
-                    unreachable!("BUG: We should not try to relocated submodules and not find them the first place")
+                    unreachable!("BUG: We should not try to relocate submodules and not find them the first place")
+                }
+                gix::repository::Kind::LinkedWorkTree => {
+                    unreachable!("BUG: LinkedWorkTree should have been skipped earlier")
                 }
-                gix::repository::Kind::Bare => path,
-                gix::repository::Kind::WorkTree { .. } => {
-                    if let Some(ext) = path.extension() {
-                        if ext == "git" {
-                            path.set_extension("");
+                gix::repository::Kind::Common => {
+                    // For Common kind, check if it's bare
+                    let git_dir = if git_workdir.join(".git").is_dir() {
+                        git_workdir.join(".git")
+                    } else {
+                        git_workdir.to_owned()
+                    };
+                    if !gix::discover::is_bare(&git_dir) {
+                        // Non-bare repository - strip .git extension if present
+                        if let Some(ext) = path.extension() {
+                            if ext == "git" {
+                                path.set_extension("");
+                            }
                         }
                     }
                     path

gix-discover/src/is.rs

@@ -1,6 +1,7 @@
 use std::{borrow::Cow, ffi::OsStr, path::Path};
 
-use crate::{DOT_GIT_DIR, MODULES};
+use crate::path::RepositoryKind;
+use crate::DOT_GIT_DIR;
 
 /// Returns true if the given `git_dir` seems to be a bare repository.
 ///
@@ -10,18 +11,9 @@ pub fn bare(git_dir_candidate: &Path) -> bool {
 }
 
 /// Returns true if `git_dir` is located within a `.git/modules` directory, indicating it's a submodule clone.
+#[deprecated = "use path::repository_kind() instead"]
 pub fn submodule_git_dir(git_dir: &Path) -> bool {
-    let mut last_comp = None;
-    git_dir.file_name() != Some(OsStr::new(DOT_GIT_DIR))
-        && git_dir.components().rev().skip(1).any(|c| {
-            if c.as_os_str() == OsStr::new(DOT_GIT_DIR) {
-                true
-            } else {
-                last_comp = Some(c.as_os_str());
-                false
-            }
-        })
-        && last_comp == Some(OsStr::new(MODULES))
+    crate::path::repository_kind(git_dir).is_some_and(|kind| matches!(kind, RepositoryKind::Submodule))
 }
 
 /// What constitutes a valid git repository, returning the guessed repository kind
@@ -157,18 +149,12 @@ pub(crate) fn git_with_metadata(
             };
             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()) {
+            } else if crate::path::repository_kind(conformed_git_dir.as_ref())
+                .is_some_and(|kind| matches!(kind, RepositoryKind::Submodule))
+            {
                 crate::repository::Kind::SubmoduleGitDir
             } 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

@@ -45,7 +45,9 @@ pub mod is_git {
 }
 
 mod is;
-pub use is::{bare as is_bare, git as is_git, submodule_git_dir as is_submodule_git_dir};
+#[allow(deprecated)]
+pub use is::submodule_git_dir as is_submodule_git_dir;
+pub use is::{bare as is_bare, git as is_git};
 
 ///
 pub mod upwards;

gix-discover/src/path.rs

@@ -1,6 +1,18 @@
+use crate::{DOT_GIT_DIR, MODULES};
+use std::ffi::OsStr;
+use std::path::Path;
 use std::{io::Read, path::PathBuf};
 
-use crate::DOT_GIT_DIR;
+/// The kind of repository by looking exclusively at its `git_dir`.
+#[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
+pub enum RepositoryKind {
+    /// The repository resides in `.git/modules/`.
+    Submodule,
+    /// The repository resides in `.git/worktrees/`.
+    LinkedWorktree,
+    /// The repository is in a `.git` directory.
+    Common,
+}
 
 ///
 pub mod from_gitdir_file {
@@ -32,6 +44,34 @@ fn read_regular_file_content_with_size_limit(path: &std::path::Path) -> std::io:
     Ok(buf)
 }
 
+/// Guess the kind of repository by looking at its `git_dir` path and return it.
+/// Return `None` if `git_dir` isn't called `.git` or isn't within `.git/worktrees` or `.git/modules`, or if it's
+/// a `.git` suffix like in `foo.git`.
+/// The check for markers is case-sensitive under the assumption that nobody meddles with standard directories.
+pub fn repository_kind(git_dir: &Path) -> Option<RepositoryKind> {
+    if git_dir.file_name() == Some(OsStr::new(DOT_GIT_DIR)) {
+        return Some(RepositoryKind::Common);
+    }
+
+    let mut last_comp = None;
+    git_dir.components().rev().skip(1).any(|c| {
+        if c.as_os_str() == OsStr::new(DOT_GIT_DIR) {
+            true
+        } else {
+            last_comp = Some(c.as_os_str());
+            false
+        }
+    });
+    let last_comp = last_comp?;
+    if last_comp == OsStr::new(MODULES) {
+        RepositoryKind::Submodule.into()
+    } else if last_comp == OsStr::new("worktrees") {
+        RepositoryKind::LinkedWorktree.into()
+    } else {
+        None
+    }
+}
+
 /// Reads a plain path from a file that contains it as its only content, with trailing newlines trimmed.
 pub fn from_plain_file(path: &std::path::Path) -> Option<std::io::Result<PathBuf>> {
     use bstr::ByteSlice;

gix-discover/tests/discover/path.rs

@@ -43,3 +43,25 @@ mod from_git_dir_file {
         Ok(file)
     }
 }
+
+#[test]
+fn repository_kind() {
+    use gix_discover::path::{repository_kind, RepositoryKind::*};
+    assert_eq!(repository_kind("hello".as_ref()), None);
+    assert_eq!(repository_kind(".git".as_ref()), Some(Common));
+    assert_eq!(repository_kind("foo/.git".as_ref()), Some(Common));
+    assert_eq!(
+        repository_kind("foo/other.git".as_ref()),
+        None,
+        "it makes no assumption beyond the standard name, nor does it consider suffixes"
+    );
+    assert_eq!(repository_kind(".git/modules".as_ref()), None);
+    assert_eq!(
+        repository_kind(".git/modules/actual-submodule".as_ref()),
+        Some(Submodule)
+    );
+    assert_eq!(
+        repository_kind(".git/worktrees/actual-worktree".as_ref()),
+        Some(LinkedWorktree)
+    );
+}

gix/src/config/cache/access.rs

@@ -18,6 +18,11 @@ use crate::{
 
 /// Access
 impl Cache {
+    /// Returns `true` if the configuration value isn't set, so we assume bare for safety.
+    pub(crate) fn is_bare_but_assume_bare_if_unconfigured(&self) -> bool {
+        self.is_bare.unwrap_or(true)
+    }
+
     #[cfg(feature = "blob-diff")]
     pub(crate) fn diff_algorithm(&self) -> Result<gix_diff::blob::Algorithm, config::diff::algorithm::Error> {
         use crate::config::{cache::util::ApplyLeniencyDefault, diff::algorithm::Error, tree::Diff};

gix/src/config/cache/incubate.rs

@@ -12,7 +12,7 @@ pub(crate) struct StageOne {
     pub git_dir_config: gix_config::File<'static>,
     pub buf: Vec<u8>,
 
-    pub is_bare: bool,
+    pub is_bare: Option<bool>,
     pub lossy: bool,
     pub object_hash: gix_hash::Kind,
     pub reflog: Option<gix_ref::store::WriteReflog>,
@@ -39,9 +39,7 @@ impl StageOne {
             lenient,
         )?;
 
-        // Note that we assume the repo is bare by default unless we are told otherwise. This is relevant if
-        // the repo doesn't have a configuration file.
-        let is_bare = util::config_bool(&config, &Core::BARE, "core.bare", true, lenient)?;
+        let is_bare = util::config_bool_opt(&config, &Core::BARE, "core.bare", lenient)?;
         let repo_format_version = config
             .integer("core.repositoryFormatVersion")
             .map(|version| Core::REPOSITORY_FORMAT_VERSION.try_into_usize(version))

gix/src/config/cache/util.rs

@@ -24,6 +24,7 @@ pub(crate) fn base_options(lossy: bool, lenient: bool) -> gix_config::file::init
     }
 }
 
+/// Also sets the default if it's not set!
 pub(crate) fn config_bool(
     config: &gix_config::File<'_>,
     key: &'static config::tree::keys::Boolean,
@@ -44,6 +45,24 @@ pub(crate) fn config_bool(
         .with_lenient_default(lenient)
 }
 
+pub(crate) fn config_bool_opt(
+    config: &gix_config::File<'_>,
+    key: &'static config::tree::keys::Boolean,
+    key_str: &str,
+    lenient: bool,
+) -> Result<Option<bool>, Error> {
+    use config::tree::Key;
+    debug_assert_eq!(
+        key_str,
+        key.logical_name(),
+        "BUG: key name and hardcoded name must match"
+    );
+    config
+        .boolean(key_str)
+        .map(|res| key.enrich_error(res).map_err(Error::from).with_lenient_default(lenient))
+        .transpose()
+}
+
 pub(crate) fn query_refupdates(
     config: &gix_config::File<'static>,
     lenient_config: bool,

gix/src/config/mod.rs

@@ -590,8 +590,8 @@ pub(crate) struct Cache {
     pub resolved: crate::Config,
     /// The hex-length to assume when shortening object ids. If `None`, it should be computed based on the approximate object count.
     pub hex_len: Option<usize>,
-    /// true if the repository is designated as 'bare', without work tree.
-    pub is_bare: bool,
+    /// `true` if the repository is designated as 'bare', without work tree. If `None`, the value wasn't configured.
+    pub is_bare: Option<bool>,
     /// The type of hash to use.
     pub object_hash: gix_hash::Kind,
     /// If true, multi-pack indices, whether present or not, may be used by the object database.