refs: classify push action based on tracking target

Although this is logically correct, the error message is a bit cryptic. It's
probably better to reject push if non-tracking remote branches exist.

#1136

cli/src/commands/git.rs

@@ -734,11 +734,11 @@ fn cmd_git_push(
                 local_target: repo.view().get_local_branch(branch_name),
                 remote_ref: repo.view().get_remote_branch(branch_name, &remote),
             };
-            if targets.local_target.is_absent() && targets.remote_ref.is_absent() {
-                return Err(user_error(format!("Branch {branch_name} doesn't exist")));
-            }
             match classify_branch_update(branch_name, &remote, targets) {
                 Ok(Some(update)) => branch_updates.push((branch_name.clone(), update)),
+                Ok(None) if targets.local_target.is_absent() => {
+                    return Err(user_error(format!("Branch {branch_name} doesn't exist")));
+                }
                 Ok(None) => writeln!(
                     ui.stderr(),
                     "Branch {branch_name}@{remote} already matches {branch_name}",

cli/tests/test_git_push.rs

@@ -229,6 +229,38 @@ fn test_git_push_not_fast_forward() {
     "###);
 }
 
+#[test]
+fn test_git_push_locally_created_and_rewritten() {
+    let (test_env, workspace_root) = set_up();
+    // Ensure that remote branches aren't tracked automatically
+    test_env.add_config("git.auto-local-branch = false");
+
+    // Push locally-created branch
+    test_env.jj_cmd_ok(&workspace_root, &["new", "root()", "-mlocal 1"]);
+    test_env.jj_cmd_ok(&workspace_root, &["branch", "set", "my"]);
+    let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Branch changes to push to origin:
+      Add branch my to fcc999921ce9
+    "###);
+
+    // Rewrite it and push again, which would fail if the pushed branch weren't
+    // set to "tracking"
+    test_env.jj_cmd_ok(&workspace_root, &["describe", "-mlocal 2"]);
+    let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "list"]);
+    insta::assert_snapshot!(stdout, @r###"
+    branch1: lzmmnrxq 45a3aa29 (empty) description 1
+    branch2: rlzusymt 8476341e (empty) description 2
+    my: vruxwmqv bde1d2e4 (empty) local 2
+      @origin (ahead by 1 commits, behind by 1 commits): vruxwmqv fcc99992 (empty) local 1
+    "###);
+    let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Branch changes to push to origin:
+      Force branch my from fcc999921ce9 to bde1d2e44b2a
+    "###);
+}
+
 #[test]
 fn test_git_push_multiple() {
     let (test_env, workspace_root) = set_up();
@@ -682,6 +714,57 @@ fn test_git_push_conflicting_branches() {
     "###);
 }
 
+#[test]
+fn test_git_push_deleted_untracked() {
+    let (test_env, workspace_root) = set_up();
+
+    // Absent local branch shouldn't be considered "deleted" compared to
+    // non-tracking remote branch.
+    test_env.jj_cmd_ok(&workspace_root, &["branch", "delete", "branch1"]);
+    test_env.jj_cmd_ok(&workspace_root, &["branch", "untrack", "branch1@origin"]);
+    let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--deleted"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Nothing changed.
+    "###);
+    let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--branch=branch1"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Error: Branch branch1 doesn't exist
+    "###);
+}
+
+#[test]
+fn test_git_push_moved_forward_untracked() {
+    let (test_env, workspace_root) = set_up();
+
+    test_env.jj_cmd_ok(&workspace_root, &["new", "branch1", "-mmoved branch1"]);
+    test_env.jj_cmd_ok(&workspace_root, &["branch", "set", "branch1"]);
+    test_env.jj_cmd_ok(&workspace_root, &["branch", "untrack", "branch1@origin"]);
+    let (_stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Branch changes to push to origin:
+      Add branch branch1 to a25f24af0d0e
+    "###);
+}
+
+#[test]
+fn test_git_push_moved_sideways_untracked() {
+    let (test_env, workspace_root) = set_up();
+
+    test_env.jj_cmd_ok(&workspace_root, &["new", "root()", "-mmoved branch1"]);
+    test_env.jj_cmd_ok(
+        &workspace_root,
+        &["branch", "set", "--allow-backwards", "branch1"],
+    );
+    test_env.jj_cmd_ok(&workspace_root, &["branch", "untrack", "branch1@origin"]);
+    let stderr = test_env.jj_cmd_failure(&workspace_root, &["git", "push"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Branch changes to push to origin:
+      Add branch branch1 to cfbae7608334
+    Error: The push conflicts with changes made on the remote (it is not fast-forwardable).
+    Hint: Try fetching from the remote, then make the branch point to where you want it to be, and push again.
+    "###);
+}
+
 #[test]
 fn test_git_push_to_remote_named_git() {
     let (test_env, workspace_root) = set_up();

docs/config.md

@@ -519,11 +519,6 @@ jj branch delete gh-pages
 jj branch untrack gh-pages@upstream
 ```
 
-Note that this setting may make it easier to accidentally delete remote
-branches. Since the local branch isn't created, the remote branch will be
-deleted if you push the branch with `jj git push --branch` or `jj git push
---all`.
-
 ### Prefix for generated branches on push
 
 `jj git push --change` generates branch names with a prefix of "push-" by

lib/src/refs.rs

@@ -138,7 +138,7 @@ pub enum BranchPushAction {
 /// this branch.
 pub fn classify_branch_push_action(targets: TrackingRefPair) -> BranchPushAction {
     let local_target = targets.local_target;
-    let remote_target = &targets.remote_ref.target;
+    let remote_target = targets.remote_ref.tracking_target();
     if local_target == remote_target {
         BranchPushAction::AlreadyMatches
     } else if local_target.has_conflict() {
@@ -159,6 +159,13 @@ mod tests {
     use crate::backend::ObjectId;
     use crate::op_store::RemoteRefState;
 
+    fn new_remote_ref(target: RefTarget) -> RemoteRef {
+        RemoteRef {
+            target,
+            state: RemoteRefState::New,
+        }
+    }
+
     fn tracking_remote_ref(target: RefTarget) -> RemoteRef {
         RemoteRef {
             target,
@@ -228,6 +235,36 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_classify_branch_push_action_removed_untracked() {
+        let commit_id1 = CommitId::from_hex("11");
+        let targets = TrackingRefPair {
+            local_target: RefTarget::absent_ref(),
+            remote_ref: &new_remote_ref(RefTarget::normal(commit_id1.clone())),
+        };
+        assert_eq!(
+            classify_branch_push_action(targets),
+            BranchPushAction::AlreadyMatches
+        );
+    }
+
+    #[test]
+    fn test_classify_branch_push_action_updated_untracked() {
+        let commit_id1 = CommitId::from_hex("11");
+        let commit_id2 = CommitId::from_hex("22");
+        let targets = TrackingRefPair {
+            local_target: &RefTarget::normal(commit_id2.clone()),
+            remote_ref: &new_remote_ref(RefTarget::normal(commit_id1.clone())),
+        };
+        assert_eq!(
+            classify_branch_push_action(targets),
+            BranchPushAction::Update(BranchPushUpdate {
+                old_target: None,
+                new_target: Some(commit_id2),
+            })
+        );
+    }
+
     #[test]
     fn test_classify_branch_push_action_local_conflicted() {
         let commit_id1 = CommitId::from_hex("11");