Skip to content

handle discovery of split worktrees #1068

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix!: make repositories with worktree config work.
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
commit e9295dc7a98eaedddb6941daa39b80412164360d
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