From a663a5d89c60c5f4fcb9b8835e1c796e533e9036 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Sat, 26 Mar 2022 14:53:51 -0700
Subject: [PATCH] repo: don't abandon empty commit if it has descendants

It's unusual for the current commit to have descendants, but it can
happen. In particular, it can easily happen when you run `jj new`. You
probably don't want to abandon it in those cases.
---
 CHANGELOG.md               |  4 ++++
 lib/src/repo.rs            |  4 ++--
 lib/tests/test_mut_repo.rs | 42 ++++++++++++++++++++++++++++++++++++++
 tests/test_new.rs          |  5 ++---
 4 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e058dc010..9b861054f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,6 +26,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * `jj new` now lets you specify a description with `--message/-m`.
 
+* When you check out a commit, the old commit no longer automatically gets
+  abandoned if it's empty and has descendants, it only gets abandoned if it's
+  empty and does not have descendants.
+
 ## [0.3.3] - 2022-03-16
 
 No changes, only trying to get the automated build to work.
diff --git a/lib/src/repo.rs b/lib/src/repo.rs
index b2e2f7c7e..27d4d0879 100644
--- a/lib/src/repo.rs
+++ b/lib/src/repo.rs
@@ -521,8 +521,8 @@ impl MutableRepo {
         if let Some(current_checkout_id) = maybe_current_checkout_id {
             let current_checkout = self.store().get_commit(&current_checkout_id).unwrap();
             assert!(current_checkout.is_open(), "current checkout is closed");
-            if current_checkout.is_empty() {
-                // Abandon the checkout we're leaving if it's empty.
+            if current_checkout.is_empty() && self.view().heads().contains(current_checkout.id()) {
+                // Abandon the checkout we're leaving if it's empty and a head commit
                 self.record_abandoned_commit(current_checkout_id);
             }
         }
diff --git a/lib/tests/test_mut_repo.rs b/lib/tests/test_mut_repo.rs
index e5a8a4dbe..7147703fb 100644
--- a/lib/tests/test_mut_repo.rs
+++ b/lib/tests/test_mut_repo.rs
@@ -133,6 +133,48 @@ fn test_checkout_previous_empty(use_git: bool) {
     assert!(!mut_repo.view().heads().contains(old_checkout.id()));
 }
 
+#[test_case(false ; "local backend")]
+// #[test_case(true ; "git backend")]
+fn test_checkout_previous_empty_non_head(use_git: bool) {
+    // Test that MutableRepo::check_out() does not abandon the previous commit if it
+    // was empty and is not a head
+    let settings = testutils::user_settings();
+    let test_repo = testutils::init_repo(&settings, use_git);
+    let repo = &test_repo.repo;
+
+    let mut tx = repo.start_transaction("test");
+    let mut_repo = tx.mut_repo();
+    let old_checkout = CommitBuilder::for_open_commit(
+        &settings,
+        repo.store(),
+        repo.store().root_commit_id().clone(),
+        repo.store().empty_tree_id().clone(),
+    )
+    .write_to_repo(mut_repo);
+    let old_child = CommitBuilder::for_open_commit(
+        &settings,
+        repo.store(),
+        old_checkout.id().clone(),
+        old_checkout.tree_id().clone(),
+    )
+    .write_to_repo(mut_repo);
+    let ws_id = WorkspaceId::default();
+    mut_repo.check_out(ws_id.clone(), &settings, &old_checkout);
+    let repo = tx.commit();
+
+    let mut tx = repo.start_transaction("test");
+    let mut_repo = tx.mut_repo();
+    let new_checkout = testutils::create_random_commit(&settings, &repo)
+        .set_open(true)
+        .write_to_repo(mut_repo);
+    mut_repo.check_out(ws_id, &settings, &new_checkout);
+    mut_repo.rebase_descendants(&settings);
+    assert_eq!(
+        *mut_repo.view().heads(),
+        hashset! {old_child.id().clone(), new_checkout.id().clone()}
+    );
+}
+
 #[test_case(false ; "local backend")]
 // #[test_case(true ; "git backend")]
 fn test_checkout_initial(use_git: bool) {
diff --git a/tests/test_new.rs b/tests/test_new.rs
index 115874a95..2f2ca9434 100644
--- a/tests/test_new.rs
+++ b/tests/test_new.rs
@@ -20,7 +20,6 @@ fn test_new_with_message() {
     test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
     let repo_path = test_env.env_root().join("repo");
 
-    std::fs::write(repo_path.join("file"), "contents").unwrap();
     test_env.jj_cmd_success(&repo_path, &["describe", "-m", "add a file"]);
     test_env.jj_cmd_success(&repo_path, &["new", "-m", "a new commit"]);
 
@@ -29,8 +28,8 @@ fn test_new_with_message() {
         .assert()
         .success();
     insta::assert_snapshot!(get_stdout_string(&assert), @r###"
-    @ 588665964c5aaf306d9617095c8a9c4447cdb30c a new commit
-    o f0f3ab56bfa927e3a65c2ac9a513693d438e271b add a file
+    @ 88436dbcdbedc2b8a6ebd0687981906d09ccc68f a new commit
+    o 51e9c5819117991e4a6dc5a4a744283fc74f0746 add a file
     o 0000000000000000000000000000000000000000 
     "###);
 }