Refuse to split an empty commit with jj split.

CHANGELOG.md

@@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * The `commit_summary_no_branches` template is superseded by
   `templates.branch_list`.
 
+* `jj split` will now refuse to split an empty commit.
+
 ### Deprecations
 
 ### New features

cli/src/commands/split.rs

@@ -18,7 +18,7 @@ use jj_lib::repo::Repo;
 use tracing::instrument;
 
 use crate::cli_util::{CommandHelper, RevisionArg};
-use crate::command_error::CommandError;
+use crate::command_error::{user_error_with_hint, CommandError};
 use crate::description_util::{description_template_for_commit, edit_description};
 use crate::ui::Ui;
 
@@ -36,6 +36,9 @@ use crate::ui::Ui;
 /// change description for each commit. If the change did not have a
 /// description, the second part will not get a description, and you will be
 /// asked for a description only for the first part.
+///
+/// Splitting an empty commit is not supported because the same effect can be
+/// achieved with `jj new`.
 #[derive(clap::Args, Clone, Debug)]
 pub(crate) struct SplitArgs {
     /// Interactively choose which parts to split. This is the default if no
@@ -64,6 +67,13 @@ pub(crate) fn cmd_split(
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
     let commit = workspace_command.resolve_single_rev(&args.revision)?;
+    if commit.is_empty(workspace_command.repo().as_ref())? {
+        return Err(user_error_with_hint(
+            format!("Refusing to split empty commit {}.", commit.id().hex()),
+            "Use `jj new` if you want to create another empty commit.",
+        ));
+    }
+
     workspace_command.check_rewritable([commit.id()])?;
     let matcher = workspace_command
         .parse_file_patterns(&args.paths)?

cli/src/commit_templater.rs

@@ -641,13 +641,7 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm
     map.insert("empty", |language, _build_ctx, self_property, function| {
         template_parser::expect_no_arguments(function)?;
         let repo = language.repo;
-        let out_property = self_property.and_then(|commit| {
-            if let [parent] = &commit.parents()[..] {
-                return Ok(parent.tree_id() == commit.tree_id());
-            }
-            let parent_tree = commit.parent_tree(repo)?;
-            Ok(*commit.tree_id() == parent_tree.id())
-        });
+        let out_property = self_property.and_then(|commit| Ok(commit.is_empty(repo)?));
         Ok(L::wrap_boolean(out_property))
     });
     map.insert("root", |language, _build_ctx, self_property, function| {

cli/tests/cli-reference@.md.snap

@@ -1732,6 +1732,8 @@ Starts a [diff editor] on the changes in the revision. Edit the right side of th
 
 If the change you split had a description, you will be asked to enter a change description for each commit. If the change did not have a description, the second part will not get a description, and you will be asked for a description only for the first part.
 
+Splitting an empty commit is not supported because the same effect can be achieved with `jj new`.
+
 **Usage:** `jj split [OPTIONS] [PATHS]...`
 
 ###### **Arguments:**

cli/tests/test_immutable_commits.rs

@@ -121,6 +121,9 @@ fn test_rewrite_immutable_commands() {
     test_env.jj_cmd_ok(&repo_path, &["new", "@-", "-m=c"]);
     std::fs::write(repo_path.join("file"), "c").unwrap();
     test_env.jj_cmd_ok(&repo_path, &["new", "all:visible_heads()", "-m=merge"]);
+    // Create another file to make sure the merge commit isn't empty (to satisfy `jj
+    // split`) and still has a conflict (to satisfy `jj resolve`).
+    std::fs::write(repo_path.join("file2"), "merged").unwrap();
     test_env.jj_cmd_ok(&repo_path, &["branch", "create", "main"]);
     test_env.jj_cmd_ok(&repo_path, &["new", "description(b)"]);
     test_env.add_config(r#"revset-aliases."immutable_heads()" = "main""#);
@@ -131,8 +134,8 @@ fn test_rewrite_immutable_commands() {
     insta::assert_snapshot!(stdout, @r###"
     @  yqosqzyt [email protected] 2001-02-03 08:05:13 3f89addf
     │  (empty) (no description set)
-    │ ◉  mzvwutvl [email protected] 2001-02-03 08:05:11 main 406c181c conflict
-    ╭─┤  (empty) merge
+    │ ◉  mzvwutvl [email protected] 2001-02-03 08:05:12 main 3e025082 conflict
+    ╭─┤  merge
     │ │
     │ ~
     │
@@ -144,71 +147,71 @@ fn test_rewrite_immutable_commands() {
     // abandon
     let stderr = test_env.jj_cmd_failure(&repo_path, &["abandon", "main"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // chmod
     let stderr = test_env.jj_cmd_failure(&repo_path, &["chmod", "-r=main", "x", "file"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // describe
     let stderr = test_env.jj_cmd_failure(&repo_path, &["describe", "main"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // diffedit
     let stderr = test_env.jj_cmd_failure(&repo_path, &["diffedit", "-r=main"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // edit
     let stderr = test_env.jj_cmd_failure(&repo_path, &["edit", "main"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // move --from
     let stderr = test_env.jj_cmd_failure(&repo_path, &["move", "--from=main"]);
     insta::assert_snapshot!(stderr, @r###"
     Warning: `jj move` is deprecated; use `jj squash` instead, which is equivalent
     Warning: `jj move` will be removed in a future version, and this will be a hard error
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // move --to
     let stderr = test_env.jj_cmd_failure(&repo_path, &["move", "--to=main"]);
     insta::assert_snapshot!(stderr, @r###"
     Warning: `jj move` is deprecated; use `jj squash` instead, which is equivalent
     Warning: `jj move` will be removed in a future version, and this will be a hard error
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // new --insert-before
     let stderr = test_env.jj_cmd_failure(&repo_path, &["new", "--insert-before", "main"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // new --insert-after parent_of_main
     let stderr = test_env.jj_cmd_failure(&repo_path, &["new", "--insert-after", "description(b)"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // parallelize
     let stderr = test_env.jj_cmd_failure(&repo_path, &["parallelize", "description(b)", "main"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // rebase -s
     let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase", "-s=main", "-d=@"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // rebase -b
@@ -220,31 +223,31 @@ fn test_rewrite_immutable_commands() {
     // rebase -r
     let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase", "-r=main", "-d=@"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // resolve
     let stderr = test_env.jj_cmd_failure(&repo_path, &["resolve", "-r=description(merge)", "file"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // restore -c
     let stderr = test_env.jj_cmd_failure(&repo_path, &["restore", "-c=main"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // restore --to
     let stderr = test_env.jj_cmd_failure(&repo_path, &["restore", "--to=main"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // split
     let stderr = test_env.jj_cmd_failure(&repo_path, &["split", "-r=main"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // squash -r
@@ -256,19 +259,19 @@ fn test_rewrite_immutable_commands() {
     // squash --from
     let stderr = test_env.jj_cmd_failure(&repo_path, &["squash", "--from=main"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // squash --into
     let stderr = test_env.jj_cmd_failure(&repo_path, &["squash", "--into=main"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
     // unsquash
     let stderr = test_env.jj_cmd_failure(&repo_path, &["unsquash", "-r=main"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Commit 406c181c04d8 is immutable
+    Error: Commit 3e0250828ca5 is immutable
     Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
     "###);
 }

cli/tests/test_split_command.rs

@@ -523,3 +523,18 @@ fn test_split_siblings_with_merge_child() {
     ◉  zzzzzzzzzzzz true
     "###);
 }
+
+// Make sure `jj split` would refuse to split an empty commit.
+#[test]
+fn test_split_empty() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
+    let workspace_path = test_env.env_root().join("repo");
+    test_env.jj_cmd_ok(&workspace_path, &["describe", "--message", "abc"]);
+
+    let stderr = test_env.jj_cmd_failure(&workspace_path, &["split"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Error: Refusing to split empty commit 82b6292b775dc4e5c5e6f402faa599dad02d02a0.
+    Hint: Use `jj new` if you want to create another empty commit.
+    "###);
+}

lib/src/commit.rs

@@ -116,6 +116,16 @@ impl Commit {
         merge_commit_trees(repo, &self.parents())
     }
 
+    /// Returns whether commit's content is empty. Commit description is not
+    /// taken into consideration.
+    pub fn is_empty(&self, repo: &dyn Repo) -> BackendResult<bool> {
+        if let [parent] = &self.parents()[..] {
+            return Ok(parent.tree_id() == self.tree_id());
+        }
+        let parent_tree = self.parent_tree(repo)?;
+        Ok(*self.tree_id() == parent_tree.id())
+    }
+
     pub fn has_conflict(&self) -> BackendResult<bool> {
         if let MergedTreeId::Merge(tree_ids) = self.tree_id() {
             Ok(!tree_ids.is_resolved())