From cdc7a0c24264e4f1127d9fd885e6a5e16fae2c08 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Sat, 5 Mar 2022 09:28:18 -0800
Subject: [PATCH] cli: when pushing all branches (the default), skip open
 commit

Open commits are work-in-progress and `jj git push --branch <name>`
therefore errors out if the branch points to an open commit. However,
we don't do the same check if you run `jj git push` to push all
branches. This patch introduces such a check. Rather than error out,
we skip such branches instead.

I didn't make it check for open commits in ancestors. It's quite
unusual (at least in my workflow) to have a closed commit on top of an
open one. We can revisit if users get surprised by it.
---
 src/commands.rs        | 12 ++++++++++++
 tests/test_git_push.rs | 25 ++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/src/commands.rs b/src/commands.rs
index 5033c21fc..44312d272 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -4364,6 +4364,18 @@ fn cmd_git_push(
                 BranchPushAction::LocalConflicted => {}
                 BranchPushAction::RemoteConflicted => {}
                 BranchPushAction::Update(update) => {
+                    if let Some(new_target) = &update.new_target {
+                        let new_target_commit = repo.store().get_commit(new_target)?;
+                        // TODO: Should we also skip branches that have open commits as ancestors?
+                        if new_target_commit.is_open() {
+                            writeln!(
+                                ui,
+                                "Skipping branch '{}' since it points to an open commit.",
+                                branch_name
+                            )?;
+                            continue;
+                        }
+                    }
                     branch_updates.insert(branch_name, update);
                 }
             }
diff --git a/tests/test_git_push.rs b/tests/test_git_push.rs
index 6f8aed1d7..f936e4942 100644
--- a/tests/test_git_push.rs
+++ b/tests/test_git_push.rs
@@ -37,6 +37,29 @@ fn test_git_push() {
     insta::assert_snapshot!(get_stdout_string(&assert), @"Nothing changed.
 ");
 
+    // When pushing everything, won't push an open commit even if there's a branch
+    // on it
+    test_env
+        .jj_cmd(&workspace_root, &["branch", "my-branch"])
+        .assert()
+        .success();
+    let assert = test_env
+        .jj_cmd(&workspace_root, &["git", "push"])
+        .assert()
+        .success();
+    insta::assert_snapshot!(get_stdout_string(&assert), @r###"
+    Skipping branch 'my-branch' since it points to an open commit.
+    Nothing changed.
+    "###);
+
+    // When pushing a specific branch, won't push it if it points to an open commit
+    let assert = test_env
+        .jj_cmd(&workspace_root, &["git", "push", "--branch", "my-branch"])
+        .assert()
+        .failure();
+    insta::assert_snapshot!(get_stdout_string(&assert), @"Error: Won't push open commit
+");
+
     // Try pushing a conflict
     std::fs::write(workspace_root.join("file"), "first").unwrap();
     test_env
@@ -65,6 +88,6 @@ fn test_git_push() {
         .jj_cmd(&workspace_root, &["git", "push"])
         .assert()
         .failure();
-    insta::assert_snapshot!(get_stdout_string(&assert), @"Error: Won't push commit 56e09a8ca383 since it has conflicts
+    insta::assert_snapshot!(get_stdout_string(&assert), @"Error: Won't push commit 28b5642cb786 since it has conflicts
 ");
 }