diff --git a/gix-discover/src/is.rs b/gix-discover/src/is.rs index c0172ef9a06..6817a5fe8af 100644 --- a/gix-discover/src/is.rs +++ b/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 `/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> { - 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 { - 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 { - 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 Result { #[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 } diff --git a/gix-discover/src/lib.rs b/gix-discover/src/lib.rs index a035c5cf6bb..df0ca849489 100644 --- a/gix-discover/src/lib.rs +++ b/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), } } diff --git a/gix-discover/src/repository.rs b/gix-discover/src/repository.rs index 7d0e20f1889..92fc633f354 100644 --- a/gix-discover/src/repository.rs +++ b/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/**/` 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 { diff --git a/gix-discover/src/upwards/mod.rs b/gix-discover/src/upwards/mod.rs index b39d216c659..7419e0c7600 100644 --- a/gix-discover/src/upwards/mod.rs +++ b/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)? { diff --git a/gix-discover/tests/is_git/mod.rs b/gix-discover/tests/discover/is_git/mod.rs similarity index 83% rename from gix-discover/tests/is_git/mod.rs rename to gix-discover/tests/discover/is_git/mod.rs index 060720d46f1..09f8576a409 100644 --- a/gix-discover/tests/is_git/mod.rs +++ b/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"] { diff --git a/gix-discover/tests/discover.rs b/gix-discover/tests/discover/main.rs similarity index 100% rename from gix-discover/tests/discover.rs rename to gix-discover/tests/discover/main.rs diff --git a/gix-discover/tests/parse/mod.rs b/gix-discover/tests/discover/parse/mod.rs similarity index 100% rename from gix-discover/tests/parse/mod.rs rename to gix-discover/tests/discover/parse/mod.rs diff --git a/gix-discover/tests/path/mod.rs b/gix-discover/tests/discover/path/mod.rs similarity index 100% rename from gix-discover/tests/path/mod.rs rename to gix-discover/tests/discover/path/mod.rs diff --git a/gix-discover/tests/upwards/ceiling_dirs.rs b/gix-discover/tests/discover/upwards/ceiling_dirs.rs similarity index 100% rename from gix-discover/tests/upwards/ceiling_dirs.rs rename to gix-discover/tests/discover/upwards/ceiling_dirs.rs diff --git a/gix-discover/tests/upwards/mod.rs b/gix-discover/tests/discover/upwards/mod.rs similarity index 100% rename from gix-discover/tests/upwards/mod.rs rename to gix-discover/tests/discover/upwards/mod.rs diff --git a/gix-discover/tests/fixtures/make_basic_repo.sh b/gix-discover/tests/fixtures/make_basic_repo.sh index b0c79baf404..4895aab8295 100755 --- a/gix-discover/tests/fixtures/make_basic_repo.sh +++ b/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 diff --git a/gix/src/object/tree/diff/change.rs b/gix/src/object/tree/diff/change.rs index a40c7100a91..0651d8dbd11 100644 --- a/gix/src/object/tree/diff/change.rs +++ b/gix/src/object/tree/diff/change.rs @@ -1,8 +1,8 @@ +use super::ChangeDetached; use crate::bstr::{BStr, ByteSlice}; use crate::ext::ObjectIdExt; use crate::object::tree::diff::Change; use crate::Repository; -use gix_diff::tree_with_rewrites::Change as ChangeDetached; impl Change<'_, '_, '_> { /// Produce a platform for performing a line-diff no matter whether the underlying [Change] is an addition, modification, diff --git a/gix/src/object/tree/diff/mod.rs b/gix/src/object/tree/diff/mod.rs index 592600055f6..adab1c34504 100644 --- a/gix/src/object/tree/diff/mod.rs +++ b/gix/src/object/tree/diff/mod.rs @@ -12,6 +12,8 @@ pub enum Action { Cancel, } +pub use gix_diff::tree_with_rewrites::Change as ChangeDetached; + /// Represents any possible change in order to turn one tree into another. #[derive(Debug, Clone, Copy)] pub enum Change<'a, 'old, 'new> { diff --git a/gix/src/open/repository.rs b/gix/src/open/repository.rs index 679abd4a7dc..7e369ffa6b9 100644 --- a/gix/src/open/repository.rs +++ b/gix/src/open/repository.rs @@ -1,7 +1,7 @@ #![allow(clippy::result_large_err)] -use std::{borrow::Cow, path::PathBuf}; - use gix_features::threading::OwnShared; +use std::ffi::OsStr; +use std::{borrow::Cow, path::PathBuf}; use super::{Error, Options}; use crate::{ @@ -86,7 +86,7 @@ impl ThreadSafeRepository { } }; - // The be altered later based on `core.precomposeUnicode`. + // To be altered later based on `core.precomposeUnicode`. let cwd = gix_fs::current_dir(false)?; let (git_dir, worktree_dir) = gix_discover::repository::Path::from_dot_git_dir(path, kind, &cwd) .expect("we have sanitized path with is_git()") @@ -284,7 +284,7 @@ impl ThreadSafeRepository { } match worktree_dir { - None if !config.is_bare => { + None if !config.is_bare && refs.git_dir().extension() == Some(OsStr::new(gix_discover::DOT_GIT_DIR)) => { worktree_dir = Some(git_dir.parent().expect("parent is always available").to_owned()); } Some(_) => { diff --git a/gix/tests/fixtures/generated-archives/make_basic_repo.tar b/gix/tests/fixtures/generated-archives/make_basic_repo.tar index 07621a94675..f65e4ce7845 100644 Binary files a/gix/tests/fixtures/generated-archives/make_basic_repo.tar and b/gix/tests/fixtures/generated-archives/make_basic_repo.tar differ diff --git a/gix/tests/fixtures/make_basic_repo.sh b/gix/tests/fixtures/make_basic_repo.sh index 45defa67648..e3cd8024ec7 100755 --- a/gix/tests/fixtures/make_basic_repo.sh +++ b/gix/tests/fixtures/make_basic_repo.sh @@ -36,4 +36,9 @@ git init all-untracked git init empty-core-excludes (cd empty-core-excludes echo $'[core]\n\texcludesFile = ' >> .git/config +) + +git clone --bare . non-bare-without-worktree +(cd non-bare-without-worktree + git config core.bare false ) \ No newline at end of file diff --git a/gix/tests/gix/repository/mod.rs b/gix/tests/gix/repository/mod.rs index 750987bc590..8acf0308529 100644 --- a/gix/tests/gix/repository/mod.rs +++ b/gix/tests/gix/repository/mod.rs @@ -42,6 +42,7 @@ mod dirwalk { ("bare.git".into(), Directory), ("empty-core-excludes".into(), Repository), ("non-bare-repo-without-index".into(), Repository), + ("non-bare-without-worktree".into(), Directory), ("some".into(), Directory), ]; assert_eq!( diff --git a/gix/tests/gix/repository/open.rs b/gix/tests/gix/repository/open.rs index 17a658dbf28..cea3bf8ceca 100644 --- a/gix/tests/gix/repository/open.rs +++ b/gix/tests/gix/repository/open.rs @@ -62,6 +62,30 @@ fn bare_repo_with_index() -> crate::Result { repo.is_bare(), "it's properly classified as it reads the configuration (and has no worktree)" ); + assert_eq!(repo.work_dir(), None); + Ok(()) +} + +#[test] +fn non_bare_non_git_repo_without_worktree() -> crate::Result { + let repo = named_subrepo_opts( + "make_basic_repo.sh", + "non-bare-without-worktree", + gix::open::Options::isolated(), + )?; + assert!(!repo.is_bare()); + assert_eq!(repo.work_dir(), None, "it doesn't assume that workdir exists"); + + let repo = gix::open_opts( + repo.git_dir().join("objects").join(".."), + gix::open::Options::isolated(), + )?; + assert!(!repo.is_bare()); + assert_eq!( + repo.work_dir(), + None, + "it figures this out even if a non-normalized gitdir is used" + ); Ok(()) }