Skip to content

Commit

Permalink
fix!: make repositories with worktree config work.
Browse files Browse the repository at this point in the history
Also rename `repository::Kind::Bare` to `repository::Kind::PossiblyBare`
to better indicate the caller has to scrutinize this value.
  • Loading branch information
Byron committed Oct 16, 2023
1 parent c5a7e66 commit e9295dc
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 27 deletions.
41 changes: 28 additions & 13 deletions gix-discover/src/is.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,34 @@ fn bare_by_config(git_dir_candidate: &Path) -> std::io::Result<Option<bool>> {
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> {
buf.lines().find_map(|line| {
let line = line.trim().strip_prefix(b"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,
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> {
Expand Down Expand Up @@ -233,7 +248,7 @@ pub(crate) fn git_with_metadata(
Cow::Borrowed(git_dir)
};
if bare(conformed_git_dir.as_ref()) || conformed_git_dir.extension() == Some(OsStr::new("git")) {
crate::repository::Kind::Bare
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))
Expand All @@ -246,7 +261,7 @@ pub(crate) fn git_with_metadata(
{
crate::repository::Kind::WorkTree { linked_git_dir: None }
} else {
crate::repository::Kind::Bare
crate::repository::Kind::PossiblyBare
}
}
})
Expand Down
2 changes: 1 addition & 1 deletion gix-discover/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub mod is_git {
GitFile(#[from] crate::path::from_gitdir_file::Error),
#[error("Could not retrieve metadata of \"{path}\"")]
Metadata { source: std::io::Error, path: PathBuf },
#[error("The repository's config file doesn't exist or didn't have a 'bare' configuration")]
#[error("The repository's config file doesn't exist or didn't have a 'bare' configuration or contained core.worktree without value")]
Inconclusive,
}
}
Expand Down
12 changes: 8 additions & 4 deletions gix-discover/src/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ mod path {
Path::WorkTree(work_dir)
}
},
Kind::Bare => Path::Repository(dir),
Kind::PossiblyBare => Path::Repository(dir),
}
.into()
}
Expand All @@ -89,7 +89,7 @@ mod path {
linked_git_dir: Some(git_dir.to_owned()),
},
Path::WorkTree(_) => Kind::WorkTree { linked_git_dir: None },
Path::Repository(_) => Kind::Bare,
Path::Repository(_) => Kind::PossiblyBare,
}
}

Expand All @@ -110,7 +110,11 @@ pub enum Kind {
/// A bare repository does not have a work tree, that is files on disk beyond the `git` repository itself.
///
/// Note that this is merely a guess at this point as we didn't read the configuration yet.
Bare,
///
/// 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.
PossiblyBare,
/// A `git` repository along with checked out files in a work tree.
WorkTree {
/// If set, this is the git dir associated with this _linked_ worktree.
Expand All @@ -135,6 +139,6 @@ pub enum Kind {
impl Kind {
/// Returns true if this is a bare repository, one without a work tree.
pub fn is_bare(&self) -> bool {
matches!(self, Kind::Bare)
matches!(self, Kind::PossiblyBare)
}
}
22 changes: 22 additions & 0 deletions gix-discover/tests/fixtures/make_basic_repo.sh
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,25 @@ git init non-bare-without-index
git commit -m "init"
rm .git/index
)

git --git-dir=repo-with-worktree-in-config-unborn-no-worktreedir --work-tree=does-not-exist-yet init
worktree=repo-with-worktree-in-config-unborn-worktree
git --git-dir=repo-with-worktree-in-config-unborn --work-tree=$worktree init && mkdir $worktree

repo=repo-with-worktree-in-config-unborn-empty-worktreedir
git --git-dir=$repo --work-tree="." init
touch $repo/index
git -C $repo config core.worktree ''

repo=repo-with-worktree-in-config-unborn-worktreedir-missing-value
git --git-dir=$repo init
touch $repo/index
echo " worktree" >> $repo/config

worktree=repo-with-worktree-in-config-worktree
git --git-dir=repo-with-worktree-in-config --work-tree=$worktree init
mkdir $worktree && touch $worktree/file
(cd repo-with-worktree-in-config
git add file
git commit -m "make sure na index exists"
)
27 changes: 24 additions & 3 deletions gix-discover/tests/is_git/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ fn missing_configuration_file_is_not_a_dealbreaker_in_bare_repo() -> crate::Resu
for name in ["bare-no-config-after-init.git", "bare-no-config.git"] {
let repo = repo_path()?.join(name);
let kind = gix_discover::is_git(&repo)?;
assert_eq!(kind, gix_discover::repository::Kind::Bare);
assert_eq!(kind, gix_discover::repository::Kind::PossiblyBare);
}
Ok(())
}
Expand All @@ -54,7 +54,7 @@ fn missing_configuration_file_is_not_a_dealbreaker_in_bare_repo() -> crate::Resu
fn bare_repo_with_index_file_looks_still_looks_like_bare() -> crate::Result {
let repo = repo_path()?.join("bare-with-index.git");
let kind = gix_discover::is_git(&repo)?;
assert_eq!(kind, gix_discover::repository::Kind::Bare);
assert_eq!(kind, gix_discover::repository::Kind::PossiblyBare);
Ok(())
}

Expand All @@ -63,7 +63,7 @@ fn bare_repo_with_index_file_looks_still_looks_like_bare_if_it_was_renamed() ->
for repo_name in ["bare-with-index-bare", "bare-with-index-no-config-bare"] {
let repo = repo_path()?.join(repo_name);
let kind = gix_discover::is_git(&repo)?;
assert_eq!(kind, gix_discover::repository::Kind::Bare);
assert_eq!(kind, gix_discover::repository::Kind::PossiblyBare);
}
Ok(())
}
Expand All @@ -85,3 +85,24 @@ fn missing_configuration_file_is_not_a_dealbreaker_in_nonbare_repo() -> crate::R
}
Ok(())
}

#[test]
fn split_worktree_using_configuration() -> crate::Result {
for name in [
"repo-with-worktree-in-config",
"repo-with-worktree-in-config-unborn",
"repo-with-worktree-in-config-unborn-no-worktreedir",
"repo-with-worktree-in-config-unborn-empty-worktreedir",
"repo-with-worktree-in-config-unborn-worktreedir-missing-value",
] {
let repo = repo_path()?.join(name);
let kind = gix_discover::is_git(&repo)?;
assert_eq!(
kind,
gix_discover::repository::Kind::PossiblyBare,
"{name}: we think these are bare as we don't read the configuration in this case - \
a shortcoming to favor performance which still comes out correct in `gix`"
);
}
Ok(())
}
4 changes: 2 additions & 2 deletions gix-discover/tests/isolated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ fn upwards_bare_repo_with_index() -> gix_testtools::Result {
let (repo_path, _trust) = gix_discover::upwards(".".as_ref())?;
assert_eq!(
repo_path.kind(),
gix_discover::repository::Kind::Bare,
gix_discover::repository::Kind::PossiblyBare,
"bare stays bare, even with index, as it resolves the path as needed in this special case"
);
Ok(())
Expand All @@ -25,7 +25,7 @@ fn in_cwd_upwards_bare_repo_without_index() -> gix_testtools::Result {

let _keep = gix_testtools::set_current_dir(repo.join("bare.git"))?;
let (repo_path, _trust) = gix_discover::upwards(".".as_ref())?;
assert_eq!(repo_path.kind(), gix_discover::repository::Kind::Bare);
assert_eq!(repo_path.kind(), gix_discover::repository::Kind::PossiblyBare);
Ok(())
}

Expand Down
8 changes: 4 additions & 4 deletions gix-discover/tests/upwards/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ fn from_bare_git_dir() -> crate::Result {
let dir = repo_path()?.join("bare.git");
let (path, trust) = gix_discover::upwards(&dir)?;
assert_eq!(path.as_ref(), dir, "the bare .git dir is directly returned");
assert_eq!(path.kind(), Kind::Bare);
assert_eq!(path.kind(), Kind::PossiblyBare);
assert_eq!(trust, expected_trust());
Ok(())
}
Expand All @@ -27,7 +27,7 @@ fn from_bare_with_index() -> crate::Result {
let dir = repo_path()?.join("bare-with-index.git");
let (path, trust) = gix_discover::upwards(&dir)?;
assert_eq!(path.as_ref(), dir, "the bare .git dir is directly returned");
assert_eq!(path.kind(), Kind::Bare);
assert_eq!(path.kind(), Kind::PossiblyBare);
assert_eq!(trust, expected_trust());
Ok(())
}
Expand All @@ -48,7 +48,7 @@ fn from_bare_git_dir_without_config_file() -> crate::Result {
let dir = repo_path()?.join(name);
let (path, trust) = gix_discover::upwards(&dir)?;
assert_eq!(path.as_ref(), dir, "the bare .git dir is directly returned");
assert_eq!(path.kind(), Kind::Bare);
assert_eq!(path.kind(), Kind::PossiblyBare);
assert_eq!(trust, expected_trust());
}
Ok(())
Expand All @@ -64,7 +64,7 @@ fn from_inside_bare_git_dir() -> crate::Result {
git_dir,
"the bare .git dir is found while traversing upwards"
);
assert_eq!(path.kind(), Kind::Bare);
assert_eq!(path.kind(), Kind::PossiblyBare);
assert_eq!(trust, expected_trust());
Ok(())
}
Expand Down

0 comments on commit e9295dc

Please sign in to comment.