From f920d663e3cf3e65ed5ddf675a0e65ccf4105efc Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 25 Jan 2024 19:41:42 +0900 Subject: [PATCH] cli: reorder import of git head/refs and snapshot Since import_git_refs() may check out new working-copy commit, it shouldn't be triggered before the snapshot. Fixes #2876 --- CHANGELOG.md | 4 +++ cli/src/cli_util.rs | 9 ++++++- cli/tests/test_git_colocated.rs | 48 +++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8432652a30..40009769a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Fixed snapshots of symlinks in `gitignore`-d directory. [#2878](https://github.com/martinvonz/jj/issues/2878) +* Fixed data loss in dirty working copy when checked-out branch is rebased or + abandoned by Git. + [#2876](https://github.com/martinvonz/jj/issues/2876) + ## [0.13.0] - 2024-01-03 diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 37ed405209..bf07a5ba25 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -813,9 +813,16 @@ impl WorkspaceCommandHelper { if self.may_update_working_copy { if self.working_copy_shared_with_git { self.import_git_head(ui)?; - self.import_git_refs(ui)?; } + // Because the Git refs (except HEAD) aren't imported yet, the ref + // pointing to the new working-copy commit might not be exported. + // In that situation, the ref would be conflicted anyway, so export + // failure is okay. self.snapshot_working_copy(ui)?; + // import_git_refs() can rebase the working-copy commit. + if self.working_copy_shared_with_git { + self.import_git_refs(ui)?; + } } Ok(()) } diff --git a/cli/tests/test_git_colocated.rs b/cli/tests/test_git_colocated.rs index 55245fa71c..6342156bd0 100644 --- a/cli/tests/test_git_colocated.rs +++ b/cli/tests/test_git_colocated.rs @@ -520,6 +520,54 @@ fn test_git_colocated_fetch_deleted_or_moved_branch() { "###); } +#[test] +fn test_git_colocated_rebase_dirty_working_copy() { + 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_ok(&repo_path, &["init", "--git-repo=."]); + + std::fs::write(repo_path.join("file"), "base").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new"]); + std::fs::write(repo_path.join("file"), "old").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "feature"]); + + // Make the working-copy dirty, delete the checked out branch. + std::fs::write(repo_path.join("file"), "new").unwrap(); + git_repo + .find_reference("refs/heads/feature") + .unwrap() + .delete() + .unwrap(); + + // Because the working copy is dirty, the new working-copy commit will be + // diverged. Therefore, the feature branch has change-delete conflict. + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["status"]); + insta::assert_snapshot!(stdout, @r###" + Working copy changes: + M file + Working copy : rlvkpnrz d6c5e664 feature?? | (no description set) + Parent commit: qpvuntsm 5973d373 (no description set) + These branches have conflicts: + feature + Use `jj branch list` to see details. Use `jj branch set -r ` to resolve. + "###); + insta::assert_snapshot!(stderr, @r###" + Failed to export some branches: + feature: Modified ref had been deleted in Git + Done importing changes from the underlying Git repo. + "###); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ d6c5e66473426f5ed3a24ecce8ce8b44ff23cf81 feature?? + ◉ 5973d3731aba9dd86c00b4a765fbc4cc13f1e14b HEAD@git + ◉ 0000000000000000000000000000000000000000 + "###); + + // The working-copy content shouldn't be lost. + insta::assert_snapshot!( + std::fs::read_to_string(repo_path.join("file")).unwrap(), @"new"); +} + #[test] fn test_git_colocated_external_checkout() { let test_env = TestEnvironment::default();