Skip to content

Commit

Permalink
workspace: use colocated git repo for git::reset_head and git::import…
Browse files Browse the repository at this point in the history
…_head

This makes colocated workspaces independent. You can move @ in a
workspace and JJ will write HEAD = @- to the git worktree in that
workspace. And you can run mutating git commands in a workspace, and
JJ will import the new HEAD only in that workspace.

Previously, any imports and exports were against the git backing repo,
which is sometimes colocated with the default workspace. So colocated
workspaces were not independent, they all read and wrote from the same
HEAD in the backing repo.
  • Loading branch information
cormacrelf committed Oct 19, 2024
1 parent cec636f commit dcb04e9
Show file tree
Hide file tree
Showing 7 changed files with 225 additions and 42 deletions.
11 changes: 5 additions & 6 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -902,8 +902,8 @@ impl WorkspaceCommandHelper {
#[instrument(skip_all)]
pub fn maybe_snapshot(&mut self, ui: &Ui) -> Result<(), CommandError> {
if self.may_update_working_copy {
if self.working_copy_shared_with_git {
self.import_git_head(ui)?;
if let Some(git_repo) = self.open_colocated_git_repo_gix()? {
self.import_git_head(ui, &git_repo)?;
}
// Because the Git refs (except HEAD) aren't imported yet, the ref
// pointing to the new working-copy commit might not be exported.
Expand All @@ -925,11 +925,11 @@ impl WorkspaceCommandHelper {
/// working-copy state will be reset to point to the new Git HEAD. The
/// working-copy contents won't be updated.
#[instrument(skip_all)]
fn import_git_head(&mut self, ui: &Ui) -> Result<(), CommandError> {
fn import_git_head(&mut self, ui: &Ui, git_repo: &gix::Repository) -> Result<(), CommandError> {
assert!(self.may_update_working_copy);
let command = self.env.command.clone();
let mut tx = self.start_transaction();
git::import_head(tx.repo_mut())?;
git::import_head(tx.repo_mut(), git_repo)?;
if !tx.repo().has_changes() {
return Ok(());
}
Expand Down Expand Up @@ -1786,8 +1786,7 @@ See https://martinvonz.github.io/jj/latest/working-copy/#stale-working-copy \
.map(|commit_id| tx.repo().store().get_commit(commit_id))
.transpose()?;

if self.working_copy_shared_with_git {
let git_repo = self.git_backend().unwrap().open_git_repo()?;
if let Some(git_repo) = self.open_colocated_git_repo_git2()? {
if let Some(wc_commit) = &maybe_new_wc_commit {
git::reset_head(tx.repo_mut(), &git_repo, wc_commit)?;
}
Expand Down
3 changes: 2 additions & 1 deletion cli/src/commands/git/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ pub fn cmd_git_import(
_args: &GitImportArgs,
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let git_repo = workspace_command.git_either_colocated_or_backend()?;
let mut tx = workspace_command.start_transaction();
// In non-colocated repo, HEAD@git will never be moved internally by jj.
// That's why cmd_git_export() doesn't export the HEAD ref.
git::import_head(tx.repo_mut())?;
git::import_head(tx.repo_mut(), &git_repo)?;
let stats = git::import_refs(tx.repo_mut(), &command.settings().git_settings())?;
print_git_import_stats(ui, tx.repo(), &stats, true)?;
tx.finish(ui, "import git refs")?;
Expand Down
5 changes: 4 additions & 1 deletion cli/src/commands/git/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,11 @@ pub fn do_init(
workspace_command.maybe_snapshot(ui)?;
maybe_set_repository_level_trunk_alias(ui, &workspace_command)?;
if !workspace_command.working_copy_shared_with_git() {
let git_repo = workspace_command
.git_backend_repo()
.expect("Just initialised with the git backend");
let mut tx = workspace_command.start_transaction();
jj_lib::git::import_head(tx.repo_mut())?;
jj_lib::git::import_head(tx.repo_mut(), &git_repo)?;
if let Some(git_head_id) = tx.repo().view().git_head().as_normal().cloned() {
let git_head_commit = tx.repo().store().get_commit(&git_head_id)?;
tx.check_out(&git_head_commit)?;
Expand Down
174 changes: 171 additions & 3 deletions cli/tests/test_git_colocated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -943,15 +943,34 @@ fn test_colocated_workspace_in_bare_repo() {
// TODO: replace with workspace add, when it can create worktrees
stopgap_workspace_colocate(&test_env, &repo_path, false, "../second", &initial_commit);

// FIXME: this workspace should have its own HEAD, but for now, there is no HEAD
// as we are only reading the backing repo, which is bare.
insta::assert_snapshot!(get_log_output(&test_env, &second_path), @r#"
@ 05530a3e0f9d581260343e273d66c381e76957df second@
@ baf7f13355a30ddd3aa6476317fcbc9c65239b0c second@
│ ○ 0959313c0818b66bb3f4912fe6c33e1f8dd1684d default@
├─╯
○ 046d74c8ab0a4730e58488508a5398b7a91e54a2 HEAD@git 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 HEAD@git commit in second workspace
│ ○ 0959313c0818b66bb3f4912fe6c33e1f8dd1684d 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#""#);
}

#[test]
Expand Down Expand Up @@ -1079,3 +1098,152 @@ fn test_colocated_workspace_invalid_gitdir() {
Hint: You may wish to try `git worktree repair` if you have moved the repo or worktree around.
"#);
}

#[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);
}

// 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,
) -> 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",
"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
}
}
18 changes: 6 additions & 12 deletions cli/tests/test_git_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -906,28 +906,22 @@ fn test_git_init_external_in_worktree_pointing_commondir() {

// The local ".git" repository is related, so commits should be imported,
// specifically from the worktree, not the original repo.
// FIXME: we should read/write the worktree HEAD, instead of always the
// backing git repo's HEAD
let (stdout, stderr) = get_log_output_with_stderr(&test_env, &workspace_root);
insta::assert_snapshot!(stdout, @r#"
@ b9966c2ebcc9
│ ○ 49c630103a76 jj-worktree second commit
├─╯
○ 70618e4d103f master HEAD@git initial commit
@ 58a55009f1c1
○ 49c630103a76 jj-worktree HEAD@git second commit
○ 70618e4d103f master initial commit
◆ 000000000000
"#);
insta::assert_snapshot!(stderr, @"");

// Check that Git HEAD is advanced because this is colocated
// FIXME: we should read/write the worktree HEAD, instead of always the
// backing git repo's HEAD
test_env.jj_cmd_ok(&workspace_root, &["new"]);
let (stdout, stderr) = get_log_output_with_stderr(&test_env, &workspace_root);
insta::assert_snapshot!(stdout, @r#"
@ 2920e96adc59
○ b9966c2ebcc9 HEAD@git
│ ○ 49c630103a76 jj-worktree second commit
├─╯
@ f133c02dde73
○ 58a55009f1c1 HEAD@git
○ 49c630103a76 jj-worktree second commit
○ 70618e4d103f master initial commit
◆ 000000000000
"#);
Expand Down
6 changes: 4 additions & 2 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,10 +546,12 @@ fn remotely_pinned_commit_ids(view: &View) -> Vec<CommitId> {
///
/// Unlike `reset_head()`, this function doesn't move the working-copy commit to
/// the child of the new `HEAD@git` revision.
pub fn import_head(mut_repo: &mut MutableRepo) -> Result<(), GitImportError> {
pub fn import_head(
mut_repo: &mut MutableRepo,
git_repo: &gix::Repository,
) -> Result<(), GitImportError> {
let store = mut_repo.store();
let git_backend = get_git_backend(store).ok_or(GitImportError::UnexpectedBackend)?;
let git_repo = git_backend.git_repo();

let old_git_head = mut_repo.view().git_head();
let new_git_head_id = if let Ok(oid) = git_repo.head_id() {
Expand Down
Loading

0 comments on commit dcb04e9

Please sign in to comment.