Skip to content

Commit

Permalink
cli: warn when workspace/.git exists, but is not colocated
Browse files Browse the repository at this point in the history
We will be glad to have added this when people are using worktrees and
breaking them, as worktrees are a bit more sensitive to things like
moving the repo around on disk.
  • Loading branch information
cormacrelf committed Nov 13, 2024
1 parent fdd7775 commit 3fb42d3
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 13 deletions.
4 changes: 2 additions & 2 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ impl WorkspaceCommandHelper {
let op_summary_template_text = settings.config().get_string("templates.op_summary")?;
let may_update_working_copy =
loaded_at_head && !env.command.global_args().ignore_working_copy;
let working_copy_shared_with_git = is_colocated_git_workspace(&workspace, &repo);
let working_copy_shared_with_git = is_colocated_git_workspace(Some(ui), &workspace, &repo);
let helper = Self {
workspace,
user_repo: ReadonlyUserRepo::new(repo),
Expand Down Expand Up @@ -878,7 +878,7 @@ impl WorkspaceCommandHelper {
}

/// Snapshot the working copy if allowed, and import Git refs if the working
/// copy is collocated with Git.
/// copy is colocated with Git.
#[instrument(skip_all)]
pub fn maybe_snapshot(&mut self, ui: &Ui) -> Result<(), CommandError> {
if self.may_update_working_copy {
Expand Down
4 changes: 3 additions & 1 deletion cli/src/commands/git/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,9 @@ pub fn do_init(
Workspace::init_external_git(command.settings(), workspace_root, git_repo_path)?;
// Import refs first so all the reachable commits are indexed in
// chronological order.
let colocated = is_colocated_git_workspace(&workspace, &repo);
// No UI, because we're about to call this function again in the workspace
// command helper, and warnings can be shown then.
let colocated = is_colocated_git_workspace(None, &workspace, &repo);
let repo = init_git_refs(ui, command, repo, colocated)?;
let mut workspace_command = command.for_workable_repo(ui, workspace, repo)?;
maybe_add_gitignore(&workspace_command)?;
Expand Down
64 changes: 62 additions & 2 deletions cli/src/git_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,72 @@ pub fn get_git_repo(store: &Store) -> Result<git2::Repository, CommandError> {
}
}

pub fn is_colocated_git_workspace(_workspace: &Workspace, repo: &ReadonlyRepo) -> bool {
pub fn is_colocated_git_workspace(
ui: Option<&Ui>,
workspace: &Workspace,
repo: &ReadonlyRepo,
) -> bool {
let Some(git_backend) = repo.store().backend_impl().downcast_ref::<GitBackend>() else {
return false;
};

git_backend.is_colocated()
if git_backend.is_colocated() {
return true;
}

if let Some(ui) = ui {
let workspace_dot_git = workspace.workspace_root().join(".git");
let Ok(meta) = workspace_dot_git.symlink_metadata() else {
return git_backend.is_colocated();
};
// Check symlink first as is_dir follows symlinks
if meta.is_symlink() {
let target = std::fs::read_link(&workspace_dot_git)
.unwrap_or_else(|_| Path::new("<could not read link>").to_path_buf());
writeln!(
ui.warning_default(),
"Workspace has a .git symlink (pointing to {}), that isn't pointing to JJ's git \
repo",
target.display()
)
.ok();
} else if meta.is_dir() {
writeln!(
ui.warning_default(),
"Workspace has a .git directory that is not managed by JJ"
)
.ok();
} else if let Ok(_worktree_repo) = gix::open(workspace.workspace_root()) {
writeln!(
ui.warning_default(),
"Workspace is also a Git worktree that is not managed by JJ",
)
.ok();
} else {
let gitfile_contents = std::fs::read_to_string(&workspace_dot_git)
.unwrap_or_else(|_| "<could not read .git file>".to_string());
let gitdir = gitfile_contents
.strip_prefix("gitdir: ")
.unwrap_or(&gitfile_contents)
.trim();
writeln!(ui.warning_default(), "Workspace is a broken Git worktree").ok();
writeln!(ui.warning_no_heading(), "The .git file points at: {gitdir}").ok();
// work_dir may return Some even if colocated = false, because we may be in some
// secondary workspace, and we may have just opened the colocated primary
// workspace with a .git directory.
let git_repo = git_backend.git_repo();
let repair_dir = git_repo.work_dir().unwrap_or(git_repo.common_dir());
writeln!(
ui.hint_default(),
"If this is meant to be a colocated JJ workspace, you may like to try `git -C {} \
worktree repair`",
repair_dir.display()
)
.ok();
}
}

false
}

fn terminal_get_username(ui: &Ui, url: &str) -> Option<String> {
Expand Down
31 changes: 27 additions & 4 deletions cli/tests/test_git_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,20 @@ fn get_log_output(test_env: &TestEnvironment, workspace_root: &Path) -> String {
test_env.jj_cmd_success(workspace_root, &["log", "-T", template, "-r=all()"])
}

fn get_log_output_with_stderr(
test_env: &TestEnvironment,
workspace_root: &Path,
) -> (String, String) {
let template = r#"
separate(" ",
commit_id.short(),
bookmarks,
if(git_head, "git_head()"),
description,
)"#;
test_env.jj_cmd_ok(workspace_root, &["log", "-T", template, "-r=all()"])
}

fn read_git_target(workspace_root: &Path) -> String {
let mut path = workspace_root.to_path_buf();
path.extend([".jj", "repo", "store", "git_target"]);
Expand Down Expand Up @@ -689,23 +703,32 @@ fn test_git_init_external_but_git_dir_exists() {
&["git", "init", "--git-repo", git_repo_path.to_str().unwrap()],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
insta::assert_snapshot!(stderr, @r#"
Warning: Workspace has a .git directory that is not managed by JJ
Initialized repo in "."
"###);
"#);

// The local ".git" repository is unrelated, so no commits should be imported
insta::assert_snapshot!(get_log_output(&test_env, &workspace_root), @r###"
let (stdout, stderr) = get_log_output_with_stderr(&test_env, &workspace_root);
insta::assert_snapshot!(stdout, @r###"
@ 230dd059e1b0
◆ 000000000000
"###);
insta::assert_snapshot!(stderr, @r###"
Warning: Workspace has a .git directory that is not managed by JJ
"###);

// Check that Git HEAD is not set because this isn't a colocated repo
test_env.jj_cmd_ok(&workspace_root, &["new"]);
insta::assert_snapshot!(get_log_output(&test_env, &workspace_root), @r###"
let (stdout, stderr) = get_log_output_with_stderr(&test_env, &workspace_root);
insta::assert_snapshot!(stdout, @r###"
@ 4db490c88528
○ 230dd059e1b0
◆ 000000000000
"###);
insta::assert_snapshot!(stderr, @r###"
Warning: Workspace has a .git directory that is not managed by JJ
"###);
}

#[test]
Expand Down
15 changes: 11 additions & 4 deletions cli/tests/test_init_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,26 +470,33 @@ fn test_init_git_external_but_git_dir_exists() {
&["init", "--git-repo", git_repo_path.to_str().unwrap()],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
insta::assert_snapshot!(stderr, @r#"
Warning: Workspace has a .git directory that is not managed by JJ
Warning: `--git` and `--git-repo` are deprecated.
Use `jj git init` instead
Initialized repo in "."
"###);
"#);

// The local ".git" repository is unrelated, so no commits should be imported
let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-r", "@-"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["log", "-r", "@-"]);
insta::assert_snapshot!(stdout, @r###"
◆ zzzzzzzz root() 00000000
"###);
insta::assert_snapshot!(stderr, @"
Warning: Workspace has a .git directory that is not managed by JJ
");

// Check that Git HEAD is not set because this isn't a colocated repo
test_env.jj_cmd_ok(&workspace_root, &["new"]);
let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-r", "@-"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["log", "-r", "@-"]);
insta::assert_snapshot!(stdout, @r###"
○ qpvuntsm [email protected] 2001-02-03 08:05:07 230dd059
│ (empty) (no description set)
~
"###);
insta::assert_snapshot!(stderr, @"
Warning: Workspace has a .git directory that is not managed by JJ
");
}

#[test]
Expand Down

0 comments on commit 3fb42d3

Please sign in to comment.