cli: extract function for figuring out how to update branches on a remote

lib/src/refs.rs

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 use crate::index::IndexRef;
-use crate::op_store::RefTarget;
+use crate::op_store::{BranchTarget, RefTarget};
 use crate::store::CommitId;
 
 pub fn merge_ref_targets(
@@ -107,3 +107,161 @@ fn find_pair_to_remove(
 
     None
 }
+
+#[derive(Debug, PartialEq, Eq, Clone)]
+pub enum BranchPushAction {
+    Update {
+        old_target: Option<CommitId>,
+        new_target: Option<CommitId>,
+    },
+    AlreadyMatches,
+    LocalConflicted,
+    RemoteConflicted,
+}
+
+/// Figure out what changes (if any) need to be made to the remote when pushing
+/// this branch.
+pub fn classify_branch_push_action(
+    branch_target: &BranchTarget,
+    remote_name: &str,
+) -> BranchPushAction {
+    let maybe_remote_target = branch_target.remote_targets.get(remote_name);
+    if branch_target.local_target.as_ref() == maybe_remote_target {
+        return BranchPushAction::AlreadyMatches;
+    }
+
+    match (&maybe_remote_target, &branch_target.local_target) {
+        (_, Some(RefTarget::Conflict { .. })) => BranchPushAction::LocalConflicted,
+        (Some(RefTarget::Conflict { .. }), _) => BranchPushAction::RemoteConflicted,
+        (Some(RefTarget::Normal(old_target)), Some(RefTarget::Normal(new_target))) => {
+            BranchPushAction::Update {
+                old_target: Some(old_target.clone()),
+                new_target: Some(new_target.clone()),
+            }
+        }
+        (Some(RefTarget::Normal(old_target)), None) => BranchPushAction::Update {
+            old_target: Some(old_target.clone()),
+            new_target: None,
+        },
+        (None, Some(RefTarget::Normal(new_target))) => BranchPushAction::Update {
+            old_target: None,
+            new_target: Some(new_target.clone()),
+        },
+        (None, None) => {
+            panic!("Unexpected branch doesn't exist anywhere")
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use maplit::btreemap;
+
+    use super::*;
+
+    #[test]
+    fn test_classify_branch_push_action_unchanged() {
+        let commit_id1 = CommitId::from_hex("11");
+        let branch = BranchTarget {
+            local_target: Some(RefTarget::Normal(commit_id1.clone())),
+            remote_targets: btreemap! {
+                "origin".to_string() => RefTarget::Normal(commit_id1)
+            },
+        };
+        assert_eq!(
+            classify_branch_push_action(&branch, "origin"),
+            BranchPushAction::AlreadyMatches
+        );
+    }
+
+    #[test]
+    fn test_classify_branch_push_action_added() {
+        let commit_id1 = CommitId::from_hex("11");
+        let branch = BranchTarget {
+            local_target: Some(RefTarget::Normal(commit_id1.clone())),
+            remote_targets: btreemap! {},
+        };
+        assert_eq!(
+            classify_branch_push_action(&branch, "origin"),
+            BranchPushAction::Update {
+                old_target: None,
+                new_target: Some(commit_id1),
+            }
+        );
+    }
+
+    #[test]
+    fn test_classify_branch_push_action_removed() {
+        let commit_id1 = CommitId::from_hex("11");
+        let branch = BranchTarget {
+            local_target: None,
+            remote_targets: btreemap! {
+                "origin".to_string() => RefTarget::Normal(commit_id1.clone())
+            },
+        };
+        assert_eq!(
+            classify_branch_push_action(&branch, "origin"),
+            BranchPushAction::Update {
+                old_target: Some(commit_id1),
+                new_target: None,
+            }
+        );
+    }
+
+    #[test]
+    fn test_classify_branch_push_action_updated() {
+        let commit_id1 = CommitId::from_hex("11");
+        let commit_id2 = CommitId::from_hex("22");
+        let branch = BranchTarget {
+            local_target: Some(RefTarget::Normal(commit_id2.clone())),
+            remote_targets: btreemap! {
+                "origin".to_string() => RefTarget::Normal(commit_id1.clone())
+            },
+        };
+        assert_eq!(
+            classify_branch_push_action(&branch, "origin"),
+            BranchPushAction::Update {
+                old_target: Some(commit_id1),
+                new_target: Some(commit_id2),
+            }
+        );
+    }
+
+    #[test]
+    fn test_classify_branch_push_action_local_conflicted() {
+        let commit_id1 = CommitId::from_hex("11");
+        let commit_id2 = CommitId::from_hex("22");
+        let branch = BranchTarget {
+            local_target: Some(RefTarget::Conflict {
+                removes: vec![],
+                adds: vec![commit_id1.clone(), commit_id2],
+            }),
+            remote_targets: btreemap! {
+                "origin".to_string() => RefTarget::Normal(commit_id1)
+            },
+        };
+        assert_eq!(
+            classify_branch_push_action(&branch, "origin"),
+            BranchPushAction::LocalConflicted
+        );
+    }
+
+    #[test]
+    fn test_classify_branch_push_action_remote_conflicted() {
+        let commit_id1 = CommitId::from_hex("11");
+        let commit_id2 = CommitId::from_hex("22");
+        let branch = BranchTarget {
+            local_target: Some(RefTarget::Normal(commit_id1.clone())),
+            remote_targets: btreemap! {
+                "origin".to_string() => RefTarget::Conflict {
+                removes: vec![],
+                adds: vec![commit_id1, commit_id2]
+            }
+            },
+        };
+        assert_eq!(
+            classify_branch_push_action(&branch, "origin"),
+            BranchPushAction::RemoteConflicted
+        );
+    }
+}

src/commands.rs

@@ -45,6 +45,7 @@ use jujutsu_lib::matchers::{EverythingMatcher, FilesMatcher, Matcher};
 use jujutsu_lib::op_heads_store::OpHeadsStore;
 use jujutsu_lib::op_store::{OpStore, OpStoreError, OperationId, RefTarget};
 use jujutsu_lib::operation::Operation;
+use jujutsu_lib::refs::{classify_branch_push_action, BranchPushAction};
 use jujutsu_lib::repo::{
     MutableRepo, ReadonlyRepo, RepoInitError, RepoLoadError, RepoLoader, RepoRef,
 };
@@ -3305,61 +3306,62 @@ fn cmd_git_push(
             branch_name
         )));
     }
-
     let branch_target = maybe_branch_target.unwrap();
-    let maybe_remote_target = branch_target.remote_targets.get(remote_name);
-    if branch_target.local_target.as_ref() == maybe_remote_target {
-        writeln!(
-            ui,
-            "Branch {}@{} already matches {}",
-            branch_name, remote_name, branch_name
-        )?;
-        return Ok(());
-    }
+    let push_action = classify_branch_push_action(branch_target, remote_name);
 
     let mut ref_updates = vec![];
-    if let Some(new_target) = &branch_target.local_target {
-        match new_target {
-            RefTarget::Conflict { .. } => {
-                return Err(CommandError::UserError(format!(
-                    "Branch {} is conflicted",
-                    branch_name
-                )));
-            }
-            RefTarget::Normal(new_target_id) => {
-                let new_target_commit = repo.store().get_commit(new_target_id)?;
+    match push_action {
+        BranchPushAction::AlreadyMatches => {
+            writeln!(
+                ui,
+                "Branch {}@{} already matches {}",
+                branch_name, remote_name, branch_name
+            )?;
+            return Ok(());
+        }
+        BranchPushAction::LocalConflicted => {
+            return Err(CommandError::UserError(format!(
+                "Branch {} is conflicted",
+                branch_name
+            )));
+        }
+        BranchPushAction::RemoteConflicted => {
+            return Err(CommandError::UserError(format!(
+                "Branch {}@{} is conflicted",
+                branch_name, remote_name
+            )));
+        }
+        BranchPushAction::Update {
+            old_target,
+            new_target,
+        } => {
+            let qualified_name = format!("refs/heads/{}", branch_name);
+            if let Some(new_target) = new_target {
+                let new_target_commit = repo.store().get_commit(&new_target)?;
                 if new_target_commit.is_open() {
                     return Err(CommandError::UserError(
                         "Won't push open commit".to_string(),
                     ));
                 }
-                let force = match maybe_remote_target {
+                let force = match old_target {
                     None => false,
-                    Some(RefTarget::Conflict { .. }) => {
-                        return Err(CommandError::UserError(format!(
-                            "Branch {}@{} is conflicted",
-                            branch_name, remote_name
-                        )));
-                    }
-                    Some(RefTarget::Normal(old_target_id)) => {
-                        !repo.index().is_ancestor(old_target_id, new_target_id)
-                    }
+                    Some(old_target) => !repo.index().is_ancestor(&old_target, &new_target),
                 };
-
                 ref_updates.push(GitRefUpdate {
-                    qualified_name: format!("refs/heads/{}", branch_name),
+                    qualified_name,
                     force,
-                    new_target: Some(new_target_id.clone()),
+                    new_target: Some(new_target),
+                });
+            } else {
+                ref_updates.push(GitRefUpdate {
+                    qualified_name,
+                    force: false,
+                    new_target: None,
                 });
             }
         }
-    } else {
-        ref_updates.push(GitRefUpdate {
-            qualified_name: format!("refs/heads/{}", branch_name),
-            force: false,
-            new_target: None,
-        });
     }
+
     let git_repo = get_git_repo(repo.store())?;
     git::push_updates(&git_repo, remote_name, &ref_updates)
         .map_err(|err| CommandError::UserError(err.to_string()))?;