From 2f00fcb6b407894a673608bc8707843539574a3f Mon Sep 17 00:00:00 2001
From: Ilya Grigoriev <ilyagr@users.noreply.github.com>
Date: Sun, 18 Dec 2022 18:25:04 -0800
Subject: [PATCH] jj git push: Make sure each branch is processed at most once

---
 lib/src/refs.rs        |  2 +-
 src/commands.rs        | 14 ++++++++++++++
 tests/test_git_push.rs | 29 +++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/lib/src/refs.rs b/lib/src/refs.rs
index 38e0b038b..34711c91e 100644
--- a/lib/src/refs.rs
+++ b/lib/src/refs.rs
@@ -108,7 +108,7 @@ fn find_pair_to_remove(
     None
 }
 
-#[derive(Debug, PartialEq, Eq, Clone)]
+#[derive(Debug, PartialEq, Eq, Clone, Hash)]
 pub struct BranchPushUpdate {
     pub old_target: Option<CommitId>,
     pub new_target: Option<CommitId>,
diff --git a/src/commands.rs b/src/commands.rs
index 13f41d0ea..05d4baf09 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -3868,8 +3868,12 @@ fn cmd_git_push(
 
     let mut tx;
     let mut branch_updates = vec![];
+    let mut seen_branches = hashset! {};
     if !args.branch.is_empty() {
         for branch_name in &args.branch {
+            if !seen_branches.insert(branch_name.clone()) {
+                continue;
+            }
             if let Some(update) = branch_updates_for_push(
                 workspace_command.repo().as_repo_ref(),
                 &args.remote,
@@ -3912,6 +3916,9 @@ fn cmd_git_push(
                 ui.settings().push_branch_prefix(),
                 commit.change_id().hex()
             );
+            if !seen_branches.insert(branch_name.clone()) {
+                continue;
+            }
             if workspace_command
                 .repo()
                 .view()
@@ -3942,6 +3949,9 @@ fn cmd_git_push(
     } else if args.all {
         // TODO: Is it useful to warn about conflicted branches?
         for (branch_name, branch_target) in workspace_command.repo().view().branches() {
+            if !seen_branches.insert(branch_name.clone()) {
+                continue;
+            }
             let push_action = classify_branch_push_action(branch_target, &args.remote);
             match push_action {
                 BranchPushAction::AlreadyMatches => {}
@@ -3992,6 +4002,9 @@ fn cmd_git_push(
                     }
                 }
                 for (branch_name, branch_target) in branches {
+                    if !seen_branches.insert(branch_name.clone()) {
+                        continue;
+                    }
                     let push_action = classify_branch_push_action(branch_target, &args.remote);
                     match push_action {
                         BranchPushAction::AlreadyMatches => {}
@@ -4012,6 +4025,7 @@ fn cmd_git_push(
             &args.remote
         ));
     }
+    drop(seen_branches);
 
     if branch_updates.is_empty() {
         writeln!(ui, "Nothing changed.")?;
diff --git a/tests/test_git_push.rs b/tests/test_git_push.rs
index ca8f2ccf9..ca7ba4984 100644
--- a/tests/test_git_push.rs
+++ b/tests/test_git_push.rs
@@ -168,6 +168,25 @@ fn test_git_push_multiple() {
       Add branch my-branch to afc3e612e744
     Dry-run requested, not pushing.
     "###);
+    // Dry run requesting two specific branches twice
+    let stdout = test_env.jj_cmd_success(
+        &workspace_root,
+        &[
+            "git",
+            "push",
+            "-b=branch1",
+            "-b=my-branch",
+            "-b=branch1",
+            "-b=my-branch",
+            "--dry-run",
+        ],
+    );
+    insta::assert_snapshot!(stdout, @r###"
+    Branch changes to push to origin:
+      Delete branch branch1 from 828a683493c6
+      Add branch my-branch to afc3e612e744
+    Dry-run requested, not pushing.
+    "###);
     let stdout = test_env.jj_cmd_success(&workspace_root, &["git", "push", "--all"]);
     insta::assert_snapshot!(stdout, @r###"
     Branch changes to push to origin:
@@ -214,6 +233,16 @@ fn test_git_push_changes() {
       Force branch push-<CHANGE_ID> from ccebc2439094 to 1624f122b2b1
       Add branch push-<CHANGE_ID> to 0a736fed65c0
     "###);
+    // specifying the same change twice doesn't break things
+    std::fs::write(workspace_root.join("file"), "modified3").unwrap();
+    let stdout = test_env.jj_cmd_success(
+        &workspace_root,
+        &["git", "push", "--change", "@", "--change", "@"],
+    );
+    insta::assert_snapshot!(replace_changeid(&stdout), @r###"
+    Branch changes to push to origin:
+      Force branch push-<CHANGE_ID> from 1624f122b2b1 to bae35833f32d
+    "###);
 }
 
 #[test]