From a302090de9a8e4622f640c43d0346f1656fede07 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 3 Oct 2023 19:45:03 +0900 Subject: [PATCH] git: add hack to unset Git HEAD by using placeholder ref As we can set HEAD to an arbitrary ref by using .reference_symbolic(), we don't have to manage a ref that can also be valid as a branch name. Fixes #1495 --- cli/tests/test_git_colocated.rs | 132 ++++++++++++++++++++++++++++++++ lib/src/git.rs | 21 +++++ lib/tests/test_git.rs | 59 ++++++++++++++ 3 files changed, 212 insertions(+) diff --git a/cli/tests/test_git_colocated.rs b/cli/tests/test_git_colocated.rs index 77d6d9a86..294e0be86 100644 --- a/cli/tests/test_git_colocated.rs +++ b/cli/tests/test_git_colocated.rs @@ -95,6 +95,138 @@ fn test_git_colocated() { ); } +#[test] +fn test_git_colocated_unborn_branch() { + let test_env = TestEnvironment::default(); + let workspace_root = test_env.env_root().join("repo"); + let git_repo = git2::Repository::init(&workspace_root).unwrap(); + + let add_file_to_index = |name: &str, data: &str| { + std::fs::write(workspace_root.join(name), data).unwrap(); + let mut index = git_repo.index().unwrap(); + index.add_path(Path::new(name)).unwrap(); + index.write().unwrap(); + }; + let checkout_index = || { + let mut index = git_repo.index().unwrap(); + index.read(true).unwrap(); // discard in-memory cache + git_repo.checkout_index(Some(&mut index), None).unwrap(); + }; + + // Initially, HEAD isn't set. + test_env.jj_cmd_success(&workspace_root, &["init", "--git-repo", "."]); + assert!(git_repo.head().is_err()); + assert_eq!( + git_repo.find_reference("HEAD").unwrap().symbolic_target(), + Some("refs/heads/master") + ); + insta::assert_snapshot!(get_log_output(&test_env, &workspace_root), @r###" + @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22 + ◉ 0000000000000000000000000000000000000000 + "###); + + // Stage some change, and check out root. This shouldn't clobber the HEAD. + add_file_to_index("file0", ""); + insta::assert_snapshot!( + test_env.jj_cmd_success(&workspace_root, &["checkout", "root()"]), @r###" + Working copy now at: kkmpptxz fcdbbd73 (empty) (no description set) + Parent commit : zzzzzzzz 00000000 (empty) (no description set) + Added 0 files, modified 0 files, removed 1 files + "###); + assert!(git_repo.head().is_err()); + assert_eq!( + git_repo.find_reference("HEAD").unwrap().symbolic_target(), + Some("refs/heads/master") + ); + insta::assert_snapshot!(get_log_output(&test_env, &workspace_root), @r###" + @ fcdbbd731496cae17161cd6be9b6cf1f759655a8 + │ ◉ 1de814dbef9641cc6c5c80d2689b80778edcce09 + ├─╯ + ◉ 0000000000000000000000000000000000000000 + "###); + // Staged change shouldn't persist. + checkout_index(); + insta::assert_snapshot!(test_env.jj_cmd_success(&workspace_root, &["status"]), @r###" + The working copy is clean + Working copy : kkmpptxz fcdbbd73 (empty) (no description set) + Parent commit: zzzzzzzz 00000000 (empty) (no description set) + "###); + + // Stage some change, and create new HEAD. This shouldn't move the default + // branch. + add_file_to_index("file1", ""); + insta::assert_snapshot!(test_env.jj_cmd_success(&workspace_root, &["new"]), @r###" + Working copy now at: royxmykx 76c60bf0 (empty) (no description set) + Parent commit : kkmpptxz f8d5bc77 (no description set) + "###); + assert!(git_repo.head().unwrap().symbolic_target().is_none()); + insta::assert_snapshot!( + git_repo.head().unwrap().peel_to_commit().unwrap().id().to_string(), + @"f8d5bc772d1147351fd6e8cea52a4f935d3b31e7" + ); + insta::assert_snapshot!(get_log_output(&test_env, &workspace_root), @r###" + @ 76c60bf0a66dcbe74d74d58c23848d96f9e86e84 + ◉ f8d5bc772d1147351fd6e8cea52a4f935d3b31e7 HEAD@git + │ ◉ 1de814dbef9641cc6c5c80d2689b80778edcce09 + ├─╯ + ◉ 0000000000000000000000000000000000000000 + "###); + // Staged change shouldn't persist. + checkout_index(); + insta::assert_snapshot!(test_env.jj_cmd_success(&workspace_root, &["status"]), @r###" + The working copy is clean + Working copy : royxmykx 76c60bf0 (empty) (no description set) + Parent commit: kkmpptxz f8d5bc77 (no description set) + "###); + + // Assign the default branch. The branch is no longer "unborn". + test_env.jj_cmd_success(&workspace_root, &["branch", "set", "-r@-", "master"]); + + // Stage some change, and check out root again. This should unset the HEAD. + // https://github.com/martinvonz/jj/issues/1495 + add_file_to_index("file2", ""); + insta::assert_snapshot!( + test_env.jj_cmd_success(&workspace_root, &["checkout", "root()"]), @r###" + Working copy now at: znkkpsqq 10dd328b (empty) (no description set) + Parent commit : zzzzzzzz 00000000 (empty) (no description set) + Added 0 files, modified 0 files, removed 2 files + "###); + assert!(git_repo.head().is_err()); + insta::assert_snapshot!(get_log_output(&test_env, &workspace_root), @r###" + @ 10dd328bb906e15890e55047740eab2812a3b2f7 + │ ◉ 2c576a57d2e6e8494616629cfdbb8fe5e3fea73b + │ ◉ f8d5bc772d1147351fd6e8cea52a4f935d3b31e7 master + ├─╯ + │ ◉ 1de814dbef9641cc6c5c80d2689b80778edcce09 + ├─╯ + ◉ 0000000000000000000000000000000000000000 + "###); + // Staged change shouldn't persist. + checkout_index(); + insta::assert_snapshot!(test_env.jj_cmd_success(&workspace_root, &["status"]), @r###" + The working copy is clean + Working copy : znkkpsqq 10dd328b (empty) (no description set) + Parent commit: zzzzzzzz 00000000 (empty) (no description set) + "###); + + // New snapshot and commit can be created after the HEAD got unset. + std::fs::write(workspace_root.join("file3"), "").unwrap(); + insta::assert_snapshot!(test_env.jj_cmd_success(&workspace_root, &["new"]), @r###" + Working copy now at: wqnwkozp cab23370 (empty) (no description set) + Parent commit : znkkpsqq 8f5b2638 (no description set) + "###); + insta::assert_snapshot!(get_log_output(&test_env, &workspace_root), @r###" + @ cab233704a5c0b21bde070943055f22142fb2043 + ◉ 8f5b263819457712a2937428b9c58a2a84afbb1c HEAD@git + │ ◉ 2c576a57d2e6e8494616629cfdbb8fe5e3fea73b + │ ◉ f8d5bc772d1147351fd6e8cea52a4f935d3b31e7 master + ├─╯ + │ ◉ 1de814dbef9641cc6c5c80d2689b80778edcce09 + ├─╯ + ◉ 0000000000000000000000000000000000000000 + "###); +} + #[test] fn test_git_colocated_export_branches_on_snapshot() { // Checks that we export branches that were changed only because the working diff --git a/lib/src/git.rs b/lib/src/git.rs index db7c7eec2..a729e30e2 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -36,6 +36,8 @@ use crate::view::{RefName, View}; /// Reserved remote name for the backing Git repo. pub const REMOTE_NAME_FOR_LOCAL_GIT_REPO: &str = "git"; +/// Ref name used as a placeholder to unset HEAD without a commit. +const UNBORN_ROOT_REF_NAME: &str = "refs/jj/root"; #[derive(Error, Debug)] pub enum GitImportError { @@ -690,6 +692,25 @@ pub fn reset_head( git_repo.set_head_detached(new_git_commit_id)?; git_repo.reset(new_git_commit.as_object(), git2::ResetType::Mixed, None)?; mut_repo.set_git_head_target(RefTarget::normal(first_parent_id.clone())); + } else { + // Can't detach HEAD without a commit. Use placeholder ref to nullify the HEAD. + // We can't set_head() an arbitrary unborn ref, so use reference_symbolic() + // instead. Git CLI appears to deal with that. It would be nice if Git CLI + // couldn't create a commit without setting a valid branch name. + if mut_repo.git_head().is_present() { + match git_repo.find_reference(UNBORN_ROOT_REF_NAME) { + Ok(mut git_repo_ref) => git_repo_ref.delete()?, + Err(err) if err.code() == git2::ErrorCode::NotFound => {} + Err(err) => return Err(err), + } + git_repo.reference_symbolic("HEAD", UNBORN_ROOT_REF_NAME, true, "unset HEAD by jj")?; + } + // git_reset() of libgit2 requires a commit object. Do that manually. + let mut index = git_repo.index()?; + index.clear()?; // or read empty tree + index.write()?; + git_repo.cleanup_state()?; + mut_repo.set_git_head_target(RefTarget::absent()); } Ok(()) } diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index dcab266b2..95fa2b472 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -35,6 +35,7 @@ use jj_lib::op_store::{BranchTarget, RefTarget}; use jj_lib::repo::{MutableRepo, ReadonlyRepo, Repo}; use jj_lib::settings::{GitSettings, UserSettings}; use jj_lib::view::RefName; +use jj_lib::workspace::Workspace; use maplit::{btreemap, hashset}; use tempfile::TempDir; use testutils::{ @@ -1572,6 +1573,64 @@ fn test_export_reexport_transitions() { ); } +#[test] +fn test_reset_head_to_root() { + // Create colocated workspace + let settings = testutils::user_settings(); + let temp_dir = testutils::new_temp_dir(); + let workspace_root = temp_dir.path().join("repo"); + let git_repo = git2::Repository::init(&workspace_root).unwrap(); + let (_workspace, repo) = + Workspace::init_external_git(&settings, &workspace_root, &workspace_root.join(".git")) + .unwrap(); + + let mut tx = repo.start_transaction(&settings, "test"); + let mut_repo = tx.mut_repo(); + + let root_commit_id = repo.store().root_commit_id(); + let tree_id = repo.store().empty_merged_tree_id(); + let commit1 = mut_repo + .new_commit(&settings, vec![root_commit_id.clone()], tree_id.clone()) + .write() + .unwrap(); + let commit2 = mut_repo + .new_commit(&settings, vec![commit1.id().clone()], tree_id.clone()) + .write() + .unwrap(); + + // Set Git HEAD to commit2's parent (i.e. commit1) + git::reset_head(tx.mut_repo(), &git_repo, &commit2).unwrap(); + assert!(git_repo.head().is_ok()); + assert_eq!( + tx.mut_repo().git_head(), + RefTarget::normal(commit1.id().clone()) + ); + + // Set Git HEAD back to root + git::reset_head(tx.mut_repo(), &git_repo, &commit1).unwrap(); + assert!(git_repo.head().is_err()); + assert!(tx.mut_repo().git_head().is_absent()); + + // Move placeholder ref as if new commit were created by git + git_repo + .reference("refs/jj/root", git_id(&commit1), false, "") + .unwrap(); + git::reset_head(tx.mut_repo(), &git_repo, &commit2).unwrap(); + assert!(git_repo.head().is_ok()); + assert_eq!( + tx.mut_repo().git_head(), + RefTarget::normal(commit1.id().clone()) + ); + assert!(git_repo.find_reference("refs/jj/root").is_ok()); + + // Set Git HEAD back to root + git::reset_head(tx.mut_repo(), &git_repo, &commit1).unwrap(); + assert!(git_repo.head().is_err()); + assert!(tx.mut_repo().git_head().is_absent()); + // The placeholder ref should be deleted + assert!(git_repo.find_reference("refs/jj/root").is_err()); +} + #[test] fn test_init() { let settings = testutils::user_settings();