cli: don't abandon non-discardable old wc commit by import_git_head()

Perhaps, the original intent was to abandon non-empty working-copy commit
assuming it was rewritten by git (therefore it should be superseded by the
current working-copy content.) However, there are other reasons that could
make the HEAD out of sync (including concurrent jj operations), and abandoning
non-empty commit can be a disaster. This patch turns it to safer side, and let
user abandon non-empty commit manually.

Fixes #3747
This commit is contained in:
Yuya Nishihara 2024-05-24 15:39:09 +09:00
parent 90565fd2f6
commit 4478055e1d
2 changed files with 41 additions and 16 deletions

View file

@ -571,9 +571,10 @@ impl WorkspaceCommandHelper {
/// Imports new HEAD from the colocated Git repo.
///
/// If the Git HEAD has changed, this function abandons our old checkout and
/// checks out the new Git HEAD. The working-copy state will be reset to
/// point to the new Git HEAD. The working-copy contents won't be updated.
/// If the Git HEAD has changed, this function checks out the new Git HEAD.
/// The old working-copy commit will be abandoned if it's discardable. The
/// 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: &mut Ui) -> Result<(), CommandError> {
assert!(self.may_update_working_copy);
@ -598,10 +599,6 @@ impl WorkspaceCommandHelper {
let new_git_head = tx.mut_repo().view().git_head().clone();
if let Some(new_git_head_id) = new_git_head.as_normal() {
let workspace_id = self.workspace_id().to_owned();
if let Some(old_wc_commit_id) = self.repo().view().get_wc_commit_id(&workspace_id) {
tx.mut_repo()
.record_abandoned_commit(old_wc_commit_id.clone());
}
let new_git_head_commit = tx.mut_repo().store().get_commit(new_git_head_id)?;
tx.mut_repo()
.check_out(workspace_id, &self.settings, &new_git_head_commit)?;

View file

@ -573,6 +573,12 @@ 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();
let git_check_out_ref = |name| {
git_repo
.set_head_detached(git_repo.find_reference(name).unwrap().target().unwrap())
.unwrap()
};
test_env.jj_cmd_ok(&repo_path, &["git", "init", "--git-repo=."]);
test_env.jj_cmd_ok(&repo_path, &["ci", "-m=A"]);
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "-r@-", "master"]);
@ -589,15 +595,7 @@ fn test_git_colocated_external_checkout() {
"###);
// Check out another branch by external command
git_repo
.set_head_detached(
git_repo
.find_reference("refs/heads/master")
.unwrap()
.target()
.unwrap(),
)
.unwrap();
git_check_out_ref("refs/heads/master");
// The old working-copy commit gets abandoned, but the whole branch should not
// be abandoned. (#1042)
@ -612,6 +610,36 @@ fn test_git_colocated_external_checkout() {
insta::assert_snapshot!(stderr, @r###"
Reset the working copy parent to the new Git HEAD.
"###);
// Edit non-head commit
test_env.jj_cmd_ok(&repo_path, &["new", "description(B)"]);
test_env.jj_cmd_ok(&repo_path, &["new", "-m=C", "--no-edit"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
99a813753d6db988d8fc436b0d6b30a54d6b2707 C
@ 81e086b7f9b1dd7fde252e28bdcf4ba4abd86ce5
eccedddfa5152d99fc8ddd1081b375387a8a382a HEAD@git B
a86754f975f953fa25da4265764adc0c62e9ce6b master A
0000000000000000000000000000000000000000
"###);
// Check out another branch by external command
git_check_out_ref("refs/heads/master");
// The old working-copy commit shouldn't be abandoned. (#3747)
let (stdout, stderr) = get_log_output_with_stderr(&test_env, &repo_path);
insta::assert_snapshot!(stdout, @r###"
@ f9f6929eae811496820623f44199a9e04ee402e8
a86754f975f953fa25da4265764adc0c62e9ce6b master HEAD@git A
99a813753d6db988d8fc436b0d6b30a54d6b2707 C
81e086b7f9b1dd7fde252e28bdcf4ba4abd86ce5
eccedddfa5152d99fc8ddd1081b375387a8a382a B
0000000000000000000000000000000000000000
"###);
insta::assert_snapshot!(stderr, @r###"
Reset the working copy parent to the new Git HEAD.
"###);
}
#[test]