Skip to content
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.

Sign up for GitHub

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

Colocated workspaces, minimal #4644

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+1,033 −67
Open

Colocated workspaces, minimal #4644

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
92bbf56
workspace: Make backend factories aware of the workspace root
cormacrelf Nov 8, 2024
7e34182
workspace: Reload the repo from disk in `workspace add`
cormacrelf Nov 10, 2024
0d72f0b
git: box all `gix::open::Error` as clippy keeps warning about it
cormacrelf Nov 10, 2024
6c62985
git: Add MaybeColocatedGitRepo helper
cormacrelf Nov 8, 2024
899ac30
git: make GitBackend colocation-aware
cormacrelf Sep 11, 2024
64adf64
cli: warn when workspace/.git exists, but is not colocated
cormacrelf Sep 11, 2024
3e4b0c1
workspace: add stopgap test helper to create a colocated workspace
cormacrelf Oct 18, 2024
2923211
workspace: Add worktree support to MaybeColocatedGitRepo
cormacrelf Nov 10, 2024
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
351 changes: 351 additions & 0 deletions cli/tests/test_git_colocated.rs
View file
Open in desktop
cormacrelf marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -975,3 +975,354 @@ fn stopgap_workspace_colocate(
.assert()
.success();
}

#[test]
fn test_colocated_workspace_in_bare_repo() {
// TODO: Remove when this stops requiring git (stopgap_workspace_colocate)
if Command::new("git").arg("--version").status().is_err() {
eprintln!("Skipping because git command might fail to run");
return;
}

let test_env = TestEnvironment::default();
let repo_path = test_env.env_root().join("repo");
let second_path = test_env.env_root().join("second");
//
// git init without --colocate creates a bare repo
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
std::fs::write(repo_path.join("file"), "contents").unwrap();
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "initial commit"]);
let (initial_commit, _) = test_env.jj_cmd_ok(
&repo_path,
&["log", "--no-graph", "-T", "commit_id", "-r", "@-"],
);
// TODO: replace with workspace add, when it can create worktrees
stopgap_workspace_colocate(&test_env, &repo_path, false, "../second", &initial_commit);

insta::assert_snapshot!(get_log_output(&test_env, &second_path), @r#"
@ baf7f13355a30ddd3aa6476317fcbc9c65239b0c second@
│ ○ 45c9d8477181a2b9c077ff1b724694fe0969b301 default@
├─╯
○ 046d74c8ab0a4730e58488508a5398b7a91e54a2 git_head() initial commit
◆ 0000000000000000000000000000000000000000
"#);

test_env.jj_cmd_ok(
&second_path,
&["commit", "-m", "commit in second workspace"],
);
insta::assert_snapshot!(get_log_output(&test_env, &second_path), @r#"
@ fca81879c29229d0097cb7d32fc8a661ee80c6e4 second@
○ 220827d1ceb632ec7dd4cb2f5110b496977d14c2 git_head() commit in second workspace
│ ○ 45c9d8477181a2b9c077ff1b724694fe0969b301 default@
├─╯
○ 046d74c8ab0a4730e58488508a5398b7a91e54a2 initial commit
◆ 0000000000000000000000000000000000000000
"#);

// FIXME: There should still be no git HEAD in the default workspace, which
// is not colocated. However, git_head() is a property of the view. And
// currently, all colocated workspaces read and write from the same
// entry of the common view.
//
// let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--no-graph",
// "-r", "git_head()"]); insta::assert_snapshot!(stdout, @r#""#);

let stdout = test_env.jj_cmd_success(
&second_path,
&["op", "log", "-Tself.description().first_line()"],
);
insta::assert_snapshot!(stdout, @r#"
@ commit baf7f13355a30ddd3aa6476317fcbc9c65239b0c
○ import git head
○ create initial working-copy commit in workspace second
○ add workspace 'second'
○ commit 4e8f9d2be039994f589b4e57ac5e9488703e604d
○ snapshot working copy
○ add workspace 'default'
○
"#);
}

#[test]
fn test_colocated_workspace_moved_original_on_disk() {
if Command::new("git").arg("--version").status().is_err() {
eprintln!("Skipping because git command might fail to run");
return;
}

let test_env = TestEnvironment::default();
let repo_path = test_env.env_root().join("repo");
let second_path = test_env.env_root().join("second");
let new_repo_path = test_env.env_root().join("repo-moved");
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "--colocate", "repo"]);
std::fs::write(repo_path.join("file"), "contents").unwrap();
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "initial commit"]);
let (initial_commit, _) = test_env.jj_cmd_ok(
&repo_path,
&["log", "--no-graph", "-T", "commit_id", "-r", "@-"],
);
// TODO: replace with workspace add, when it can create worktrees
stopgap_workspace_colocate(&test_env, &repo_path, true, "../second", &initial_commit);

// Break our worktree by moving the original repo on disk
std::fs::rename(&repo_path, &new_repo_path).unwrap();
// imagine JJ were able to do this
std::fs::write(
second_path.join(".jj/repo"),
new_repo_path
.join(".jj/repo")
.as_os_str()
.as_encoded_bytes(),
)
.unwrap();

let (_, stderr) = test_env.jj_cmd_ok(&second_path, &["status"]);
// hack for windows paths
let gitfile_contents = std::fs::read_to_string(second_path.join(".git"))
.unwrap()
.strip_prefix("gitdir: ")
.unwrap()
.trim()
.to_owned();
let stderr = stderr.replace(&gitfile_contents, "$TEST_ENV/repo/.git/worktrees/second");
insta::assert_snapshot!(stderr, @r"
Warning: Workspace is a broken Git worktree
The .git file points at: $TEST_ENV/repo/.git/worktrees/second
Hint: If this is meant to be a colocated JJ workspace, you may like to try `git -C $TEST_ENV/repo-moved worktree repair`
");

Command::new("git")
.args(["worktree", "repair"])
.current_dir(&new_repo_path)
.assert()
.success();
insta::assert_snapshot!(get_log_output(&test_env, &second_path), @r#"
@ 05530a3e0f9d581260343e273d66c381e76957df second@
│ ○ 45c9d8477181a2b9c077ff1b724694fe0969b301 default@
├─╯
○ 046d74c8ab0a4730e58488508a5398b7a91e54a2 git_head() initial commit
◆ 0000000000000000000000000000000000000000
"#);
}

#[test]
fn test_colocated_workspace_wrong_gitdir() {
// TODO: Remove when this stops requiring git (stopgap_workspace_colocate)
if Command::new("git").arg("--version").status().is_err() {
eprintln!("Skipping because git command might fail to run");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think "Skipping test because Git prerequisite is missing" may be a more appropriate message for CI

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am just copying what's there already, so someone can grep for all uses.

return;
}

let test_env = TestEnvironment::default();
let repo_path = test_env.env_root().join("repo");
let second_path = test_env.env_root().join("second");
let other_path = test_env.env_root().join("other");
let other_second_path = test_env.env_root().join("other_second");
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "--colocate", "repo"]);
std::fs::write(repo_path.join("file"), "contents").unwrap();
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "initial commit"]);
let (initial_commit, _) = test_env.jj_cmd_ok(
&repo_path,
&["log", "--no-graph", "-T", "commit_id", "-r", "@-"],
);
// TODO: replace with workspace add, when it can create worktrees
stopgap_workspace_colocate(&test_env, &repo_path, true, "../second", &initial_commit);

test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "--colocate", "other"]);
std::fs::write(other_path.join("file"), "contents2").unwrap();
test_env.jj_cmd_ok(&other_path, &["commit", "-m", "initial commit"]);
let (ic_other, _) = test_env.jj_cmd_ok(
&other_path,
&["log", "--no-graph", "-T", "commit_id", "-r", "@-"],
);
// TODO: replace with workspace add, when it can create worktrees
stopgap_workspace_colocate(&test_env, &other_path, true, "../other_second", &ic_other);

// Break one of our worktrees
std::fs::copy(other_second_path.join(".git"), second_path.join(".git")).unwrap();

let (_, stderr) = test_env.jj_cmd_ok(&second_path, &["status"]);
insta::assert_snapshot!(stderr, @"Warning: Workspace is also a Git worktree that is not managed by JJ");
}

#[test]
fn test_colocated_workspace_invalid_gitdir() {
// TODO: Remove when this stops requiring git (stopgap_workspace_colocate)
if Command::new("git").arg("--version").status().is_err() {
eprintln!("Skipping because git command might fail to run");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

return;
}

let test_env = TestEnvironment::default();
let repo_path = test_env.env_root().join("repo");
let second_path = test_env.env_root().join("second");
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "--colocate", "repo"]);
std::fs::write(repo_path.join("file"), "contents").unwrap();
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "initial commit"]);
let (initial_commit, _) = test_env.jj_cmd_ok(
&repo_path,
&["log", "--no-graph", "-T", "commit_id", "-r", "@-"],
);
// TODO: replace with workspace add, when it can create worktrees
stopgap_workspace_colocate(&test_env, &repo_path, true, "../second", &initial_commit);

// Break one of our worktrees
std::fs::write(second_path.join(".git"), "invalid").unwrap();

let (_, stderr) = test_env.jj_cmd_ok(&second_path, &["status"]);
insta::assert_snapshot!(stderr, @r"
Warning: Workspace is a broken Git worktree
The .git file points at: invalid
Hint: If this is meant to be a colocated JJ workspace, you may like to try `git -C $TEST_ENV/repo worktree repair`
");
}

#[test]
fn test_colocated_workspace_independent_heads() {
// TODO: Remove when this stops requiring git (stopgap_workspace_colocate)
if Command::new("git").arg("--version").status().is_err() {
eprintln!("Skipping because git command might fail to run");
return;
}

let test_env = TestEnvironment::default();
let repo_path = test_env.env_root().join("repo");
let second_path = test_env.env_root().join("second");
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "--colocate", "repo"]);
// create a commit so that git can have a HEAD
std::fs::write(repo_path.join("file"), "contents").unwrap();
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "initial commit"]);
let (initial_commit, _) = test_env.jj_cmd_ok(
&repo_path,
&["log", "--no-graph", "-T", "commit_id", "-r", "@-"],
);
// TODO: replace with workspace add, when it can create worktrees
stopgap_workspace_colocate(&test_env, &repo_path, true, "../second", &initial_commit);

{
let first_git = git2::Repository::open(&repo_path).unwrap();
assert!(first_git.head_detached().unwrap());
let first_head = first_git.head().unwrap();

let commit = first_head.peel_to_commit().unwrap().id();
assert_eq!(commit.to_string(), initial_commit);

let second_git = git2::Repository::open(&second_path).unwrap();
assert!(second_git.head_detached().unwrap());
let second_head = second_git.head().unwrap();

let commit = second_head.peel_to_commit().unwrap().id();
assert_eq!(commit.to_string(), initial_commit);
}
cormacrelf marked this conversation as resolved.
Show resolved Hide resolved

// now commit again in the second worktree, and make sure the original
// repo's head does not move.
//
// This tests that we are writing HEAD to the corresponding worktree,
// rather than unconditionally to the default workspace.
std::fs::write(repo_path.join("file2"), "contents").unwrap();
test_env.jj_cmd_ok(&second_path, &["commit", "-m", "followup commit"]);
let (followup_commit, _) = test_env.jj_cmd_ok(
&second_path,
&["log", "--no-graph", "-T", "commit_id", "-r", "@-"],
);

{
// git HEAD should not move in the default workspace
let first_git = git2::Repository::open(&repo_path).unwrap();
assert!(first_git.head_detached().unwrap());
let first_head = first_git.head().unwrap();
// still initial
assert_eq!(
first_head.peel_to_commit().unwrap().id().to_string(),
initial_commit,
"default workspace's git HEAD should not have moved from {initial_commit}"
);

let second_git = git2::Repository::open(&second_path).unwrap();
assert!(second_git.head_detached().unwrap());
let second_head = second_git.head().unwrap();
assert_eq!(
second_head.peel_to_commit().unwrap().id().to_string(),
followup_commit,
"second workspace's git HEAD should have advanced to {followup_commit}"
);
}

// Finally, test imports. Test that a commit written to HEAD in one workspace
// does not get imported by the other workspace.

// Write in default, expect second not to import it
let new_commit = test_independent_import(&test_env, &repo_path, &second_path, &followup_commit);
// Write in second, expect default not to import it
test_independent_import(&test_env, &second_path, &repo_path, &new_commit);

fn test_independent_import(
test_env: &TestEnvironment,
commit_in: &Path,
no_import_in_workspace: &Path,
workspace_at: &str,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: workspace_at sounds like a path. Rename to wc_commit or something?

) -> String {
// Commit in one workspace
let mut repo = gix::open(commit_in).unwrap();
{
use gix::config::tree::*;
let mut config = repo.config_snapshot_mut();
let (name, email) = ("JJ test", "[email protected]");
config.set_value(&Author::NAME, name).unwrap();
config.set_value(&Author::EMAIL, email).unwrap();
config.set_value(&Committer::NAME, name).unwrap();
config.set_value(&Committer::EMAIL, email).unwrap();
}
let tree = repo.head_tree_id().unwrap();
let current = repo.head_commit().unwrap().id;
let new_commit = repo
.commit(
"HEAD",
format!("empty commit in {}", commit_in.display()),
tree,
[current],
)
.unwrap()
.to_string();

let (check_git_head, stderr) = test_env.jj_cmd_ok(
no_import_in_workspace,
&["log", "--no-graph", "-r", "git_head()", "-T", "commit_id"],
);
// Asserting stderr is empty => no import occurred
assert_eq!(
stderr,
"",
"Should not have imported HEAD in workspace {}",
no_import_in_workspace.display()
);
// And the commit_id should be pointing to what it was before
assert_eq!(
check_git_head,
workspace_at,
"should still be at {workspace_at} in workspace {}",
no_import_in_workspace.display()
);

// Now we import the new HEAD in the commit_in workspace, so it's up to date.
let (check_git_head, stderr) = test_env.jj_cmd_ok(
commit_in,
&["log", "--no-graph", "-r", "git_head()", "-T", "commit_id"],
);
assert_eq!(
stderr,
"Reset the working copy parent to the new Git HEAD.\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: better to use insta::allow_duplicates!? It's tedious to update the expectation on wording changes.

"should have imported HEAD in workspace {}",
commit_in.display()
);
assert_eq!(
check_git_head,
new_commit,
"should have advanced to {new_commit} in workspace {}",
commit_in.display()
);
new_commit
}
}
Loading
Loading