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.

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

git: on external HEAD move, do not abandon old branch #1592

Merged
merged 2 commits into from
May 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* It is no longer allowed to create branches at the root commit.

* `git checkout` (without using `jj`) in colocated repo no longer abandons
the previously checked-out anonymous branch.
[#1042](https://github.com/martinvonz/jj/issues/1042).

## [0.7.0] - 2023-02-16

### Breaking changes
Expand Down
6 changes: 3 additions & 3 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,16 @@ pub fn import_some_refs(
new_git_heads.extend(old_target.adds());
}
}
if let Some(old_git_head) = mut_repo.view().git_head() {
old_git_heads.extend(old_git_head.adds());
}

// TODO: Should this be a separate function? We may not always want to import
// the Git HEAD (and add it to our set of heads).
if let Ok(head_git_commit) = git_repo
.head()
.and_then(|head_ref| head_ref.peel_to_commit())
{
// Add the current HEAD to `new_git_heads` to pin the branch. It's not added
// to `old_git_heads` because HEAD move doesn't automatically mean the old
// HEAD branch has been rewritten.
let head_commit_id = CommitId::from_bytes(head_git_commit.id().as_bytes());
let head_commit = store.get_commit(&head_commit_id).unwrap();
new_git_heads.insert(head_commit_id.clone());
Expand Down
105 changes: 105 additions & 0 deletions lib/tests/test_git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,111 @@ fn test_import_refs_reimport_git_head_counts() {
assert!(tx.mut_repo().view().heads().contains(&jj_id(&commit)));
}

#[test]
fn test_import_refs_reimport_git_head_without_ref() {
// Simulate external `git checkout` in colocated repo, from anonymous branch.
let settings = testutils::user_settings();
let git_settings = GitSettings::default();
let test_repo = TestRepo::init(true);
let repo = &test_repo.repo;
let git_repo = repo.store().git_repo().unwrap();

// First, HEAD points to commit1.
let mut tx = repo.start_transaction(&settings, "test");
let commit1 = write_random_commit(tx.mut_repo(), &settings);
let commit2 = write_random_commit(tx.mut_repo(), &settings);
git_repo.set_head_detached(git_id(&commit1)).unwrap();

// Import HEAD.
git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
assert!(tx.mut_repo().view().heads().contains(commit1.id()));
assert!(tx.mut_repo().view().heads().contains(commit2.id()));

// Move HEAD to commit2 (by e.g. `git checkout` command)
git_repo.set_head_detached(git_id(&commit2)).unwrap();

// Reimport HEAD, which doesn't abandon the old HEAD branch because jj thinks it
// would be moved by `git checkout` command. This isn't always true because the
// detached HEAD commit could be rewritten by e.g. `git commit --amend` command,
// but it should be safer than abandoning old checkout branch.
git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
assert!(tx.mut_repo().view().heads().contains(commit1.id()));
assert!(tx.mut_repo().view().heads().contains(commit2.id()));
}

#[test]
fn test_import_refs_reimport_git_head_with_moved_ref() {
// Simulate external history rewriting in colocated repo.
let settings = testutils::user_settings();
let git_settings = GitSettings::default();
let test_repo = TestRepo::init(true);
let repo = &test_repo.repo;
let git_repo = repo.store().git_repo().unwrap();

// First, both HEAD and main point to commit1.
let mut tx = repo.start_transaction(&settings, "test");
let commit1 = write_random_commit(tx.mut_repo(), &settings);
let commit2 = write_random_commit(tx.mut_repo(), &settings);
git_repo
.reference("refs/heads/main", git_id(&commit1), true, "test")
.unwrap();
git_repo.set_head_detached(git_id(&commit1)).unwrap();

// Import HEAD and main.
git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
assert!(tx.mut_repo().view().heads().contains(commit1.id()));
assert!(tx.mut_repo().view().heads().contains(commit2.id()));

// Move both HEAD and main to commit2 (by e.g. `git commit --amend` command)
git_repo
.reference("refs/heads/main", git_id(&commit2), true, "test")
.unwrap();
git_repo.set_head_detached(git_id(&commit2)).unwrap();

// Reimport HEAD and main, which abandons the old main branch.
git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
assert!(!tx.mut_repo().view().heads().contains(commit1.id()));
assert!(tx.mut_repo().view().heads().contains(commit2.id()));
}

#[test]
fn test_import_refs_reimport_git_head_with_fixed_ref() {
// Simulate external `git checkout` in colocated repo, from named branch.
let settings = testutils::user_settings();
let git_settings = GitSettings::default();
let test_repo = TestRepo::init(true);
let repo = &test_repo.repo;
let git_repo = repo.store().git_repo().unwrap();

// First, both HEAD and main point to commit1.
let mut tx = repo.start_transaction(&settings, "test");
let commit1 = write_random_commit(tx.mut_repo(), &settings);
let commit2 = write_random_commit(tx.mut_repo(), &settings);
git_repo
.reference("refs/heads/main", git_id(&commit1), true, "test")
.unwrap();
git_repo.set_head_detached(git_id(&commit1)).unwrap();

// Import HEAD and main.
git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
assert!(tx.mut_repo().view().heads().contains(commit1.id()));
assert!(tx.mut_repo().view().heads().contains(commit2.id()));

// Move only HEAD to commit2 (by e.g. `git checkout` command)
git_repo.set_head_detached(git_id(&commit2)).unwrap();

// Reimport HEAD, which shouldn't abandon the old HEAD branch.
git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
assert!(tx.mut_repo().view().heads().contains(commit1.id()));
assert!(tx.mut_repo().view().heads().contains(commit2.id()));
}

#[test]
fn test_import_refs_reimport_all_from_root_removed() {
// Test that if a chain of commits all the way from the root gets unreferenced,
Expand Down
41 changes: 41 additions & 0 deletions tests/test_git_colocated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,47 @@ fn test_git_colocated_fetch_deleted_branch() {
"###);
}

#[test]
fn test_git_colocated_external_checkout() {
let test_env = TestEnvironment::default();
let repo_path = test_env.env_root().join("repo");
let git_repo = git2::Repository::init(&repo_path).unwrap();
test_env.jj_cmd_success(&repo_path, &["init", "--git-repo=."]);
test_env.jj_cmd_success(&repo_path, &["ci", "-m=A"]);
test_env.jj_cmd_success(&repo_path, &["new", "-m=B", "root"]);
test_env.jj_cmd_success(&repo_path, &["new"]);

// Checked out anonymous branch
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 53637cd508ff02427dd78eca98f5b2450a6370ce
◉ 66f4d1806ae41bd604f69155dece64062a0056cf
│ ◉ a86754f975f953fa25da4265764adc0c62e9ce6b master
├─╯
◉ 0000000000000000000000000000000000000000
"###);

// Check out another branch by external command
git_repo
.set_head_detached(
git_repo
.find_reference("refs/heads/master")
.unwrap()
.target()
.unwrap(),
)
.unwrap();

// The old working-copy commit gets abandoned, but the whole branch should not
// be abandoned. (#1042)
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 0521ce3b8c4e29aab79f3c750e2845dcbc4c3874
│ ◉ 66f4d1806ae41bd604f69155dece64062a0056cf
◉ │ a86754f975f953fa25da4265764adc0c62e9ce6b master
├─╯
◉ 0000000000000000000000000000000000000000
"###);
}

#[test]
fn test_git_colocated_squash_undo() {
let test_env = TestEnvironment::default();
Expand Down