cli: Ignore immutable() in jj git push conflict/desc/user checks

Fixes jj-vcs#3029.

CHANGELOG.md

@@ -57,6 +57,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed bugs
 
+* `jj git push` now ignores immutable commits when checking whether a
+  to-be-pushed commit has conflicts, or has no description / committer / author
+  set. [#3029](https://github.com/martinvonz/jj/issues/3029)
+
 ## [0.18.0] - 2024-06-05
 
 ### Breaking changes

cli/src/commands/git/push.rs

@@ -25,7 +25,7 @@ use jj_lib::refs::{
     classify_branch_push_action, BranchPushAction, BranchPushUpdate, LocalAndRemoteRef,
 };
 use jj_lib::repo::Repo;
-use jj_lib::revset::{self, RevsetExpression, RevsetIteratorExt as _};
+use jj_lib::revset::RevsetExpression;
 use jj_lib::settings::{ConfigResultExt as _, UserSettings};
 use jj_lib::str_util::StringPattern;
 use jj_lib::view::View;
@@ -37,6 +37,7 @@ use crate::cli_util::{
 use crate::command_error::{user_error, user_error_with_hint, CommandError};
 use crate::commands::git::{get_single_remote, map_git_error};
 use crate::git_util::{get_git_repo, with_remote_git_callbacks, GitSidebandProgressMessageWriter};
+use crate::revset_util;
 use crate::ui::Ui;
 
 /// Push to a Git remote
@@ -275,9 +276,16 @@ pub fn cmd_git_push(
     if old_heads.is_empty() {
         old_heads.push(repo.store().root_commit_id().clone());
     }
-    for commit in revset::walk_revs(repo.as_ref(), &new_heads, &old_heads)?
-        .iter()
-        .commits(repo.store())
+    // (old_heads | immutable())..new_heads
+    let commits_to_push = RevsetExpression::commits(old_heads)
+        .union(&revset_util::parse_immutable_expression(
+            &tx.base_workspace_helper().revset_parse_context(),
+        )?)
+        .range(&RevsetExpression::commits(new_heads));
+    for commit in tx
+        .base_workspace_helper()
+        .attach_revset_evaluator(commits_to_push)?
+        .evaluate_to_commits()?
     {
         let commit = commit?;
         let mut reasons = vec![];

cli/tests/test_git_push.rs

@@ -835,6 +835,28 @@ fn test_git_push_no_description() {
     );
 }
 
+#[test]
+fn test_git_push_no_description_in_immutable() {
+    let (test_env, workspace_root) = set_up();
+    test_env.jj_cmd_ok(&workspace_root, &["branch", "create", "imm"]);
+    test_env.jj_cmd_ok(&workspace_root, &["describe", "-m="]);
+    test_env.add_config(r#"revset-aliases."immutable_heads()" = "imm""#);
+    test_env.jj_cmd_ok(&workspace_root, &["new", "-m", "foo"]);
+    std::fs::write(workspace_root.join("file"), "contents").unwrap();
+    test_env.jj_cmd_ok(&workspace_root, &["branch", "create", "my-branch"]);
+
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &workspace_root,
+        &["git", "push", "--branch=my-branch", "--dry-run"],
+    );
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Branch changes to push to origin:
+      Add branch my-branch to ea7373507ad9
+    Dry-run requested, not pushing.
+    "###);
+}
+
 #[test]
 fn test_git_push_missing_author() {
     let (test_env, workspace_root) = set_up();
@@ -863,6 +885,36 @@ fn test_git_push_missing_author() {
     "###);
 }
 
+#[test]
+fn test_git_push_missing_author_in_immutable() {
+    let (test_env, workspace_root) = set_up();
+    let run_without_var = |var: &str, args: &[&str]| {
+        test_env
+            .jj_cmd(&workspace_root, args)
+            .env_remove(var)
+            .assert()
+            .success();
+    };
+    run_without_var("JJ_USER", &["new", "root()", "-m=no author name"]);
+    run_without_var("JJ_EMAIL", &["new", "-m=no author email"]);
+    test_env.jj_cmd_ok(&workspace_root, &["branch", "create", "imm"]);
+    test_env.add_config(r#"revset-aliases."immutable_heads()" = "imm""#);
+    test_env.jj_cmd_ok(&workspace_root, &["new", "-m", "foo"]);
+    std::fs::write(workspace_root.join("file"), "contents").unwrap();
+    test_env.jj_cmd_ok(&workspace_root, &["branch", "create", "my-branch"]);
+
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &workspace_root,
+        &["git", "push", "--branch=my-branch", "--dry-run"],
+    );
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Branch changes to push to origin:
+      Add branch my-branch to 68fdae89de4f
+    Dry-run requested, not pushing.
+    "###);
+}
+
 #[test]
 fn test_git_push_missing_committer() {
     let (test_env, workspace_root) = set_up();
@@ -899,6 +951,37 @@ fn test_git_push_missing_committer() {
     "###);
 }
 
+#[test]
+fn test_git_push_missing_committer_in_immutable() {
+    let (test_env, workspace_root) = set_up();
+    let run_without_var = |var: &str, args: &[&str]| {
+        test_env
+            .jj_cmd(&workspace_root, args)
+            .env_remove(var)
+            .assert()
+            .success();
+    };
+    run_without_var("JJ_USER", &["describe", "-m=no committer name"]);
+    test_env.jj_cmd_ok(&workspace_root, &["new"]);
+    run_without_var("JJ_EMAIL", &["describe", "-m=no committer email"]);
+    test_env.jj_cmd_ok(&workspace_root, &["branch", "create", "imm"]);
+    test_env.add_config(r#"revset-aliases."immutable_heads()" = "imm""#);
+    test_env.jj_cmd_ok(&workspace_root, &["new", "-m", "foo"]);
+    std::fs::write(workspace_root.join("file"), "contents").unwrap();
+    test_env.jj_cmd_ok(&workspace_root, &["branch", "create", "my-branch"]);
+
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &workspace_root,
+        &["git", "push", "--branch=my-branch", "--dry-run"],
+    );
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Branch changes to push to origin:
+      Add branch my-branch to c79f85e90b4a
+    Dry-run requested, not pushing.
+    "###);
+}
+
 #[test]
 fn test_git_push_deleted() {
     let (test_env, workspace_root) = set_up();