git: on push, update jj's view of remote branches without using import_refs()

This means that the commits previously pinned by remote branches are no longer
abandoned. I think that's more correct since "push" is the operation to
propagate local view to remote, and uninteresting commits should have been
locally abandoned.

CHANGELOG.md

@@ -38,6 +38,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   See [automatic local branch creation](docs/config.md#automatic-local-branch-creation)
   for details.
 
+* Pushing deleted/moved branches no longer abandons the local commits referenced
+  by the remote branches.
+
 ### New features
 
 * `jj workspace add` now takes a `--revision` argument.

cli/src/commands/git.rs

@@ -977,7 +977,7 @@ fn cmd_git_push(
         force_pushed_branches,
     };
     with_remote_callbacks(ui, |cb| {
-        git::push_branches(&git_repo, &remote, &targets, cb)
+        git::push_branches(tx.mut_repo(), &git_repo, &remote, &targets, cb)
     })
     .map_err(|err| match err {
         GitPushError::InternalGitError(err) => map_git_error(err),
@@ -988,9 +988,6 @@ fn cmd_git_push(
         ),
         _ => user_error(err.to_string()),
     })?;
-    // TODO: mark pushed remote branches as tracking
-    let stats = git::import_refs(tx.mut_repo(), &git_repo, &command.settings().git_settings())?;
-    print_git_import_stats(ui, &stats)?;
     tx.finish(ui)?;
     Ok(())
 }

cli/tests/test_git_push.rs

@@ -300,13 +300,22 @@ fn test_git_push_multiple() {
       Delete branch branch1 from 45a3aa29e907
       Force branch branch2 from 8476341eb395 to 15dcdaa4f12f
       Add branch my-branch to 15dcdaa4f12f
-    Abandoned 2 commits that are no longer reachable.
     "###);
     let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "list"]);
     insta::assert_snapshot!(stdout, @r###"
     branch2: yqosqzyt 15dcdaa4 (empty) foo
     my-branch: yqosqzyt 15dcdaa4 (empty) foo
     "###);
+    let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-rall()"]);
+    insta::assert_snapshot!(stdout, @r###"
+    @  yqosqzyt test.user@example.com 2001-02-03 04:05:17.000 +07:00 branch2 my-branch 15dcdaa4
+    │  (empty) foo
+    │ ◉  rlzusymt test.user@example.com 2001-02-03 04:05:10.000 +07:00 8476341e
+    ├─╯  (empty) description 2
+    │ ◉  lzmmnrxq test.user@example.com 2001-02-03 04:05:08.000 +07:00 45a3aa29
+    ├─╯  (empty) description 1
+    ◉  zzzzzzzz root() 00000000
+    "###);
 }
 
 #[test]
@@ -588,7 +597,16 @@ fn test_git_push_deleted() {
     insta::assert_snapshot!(stderr, @r###"
     Branch changes to push to origin:
       Delete branch branch1 from 45a3aa29e907
-    Abandoned 1 commits that are no longer reachable.
+    "###);
+    let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-rall()"]);
+    insta::assert_snapshot!(stdout, @r###"
+    ◉  rlzusymt test.user@example.com 2001-02-03 04:05:10.000 +07:00 branch2 8476341e
+    │  (empty) description 2
+    │ ◉  lzmmnrxq test.user@example.com 2001-02-03 04:05:08.000 +07:00 45a3aa29
+    ├─╯  (empty) description 1
+    │ @  yqosqzyt test.user@example.com 2001-02-03 04:05:13.000 +07:00 5b36783c
+    ├─╯  (empty) (no description set)
+    ◉  zzzzzzzz root() 00000000
     "###);
     let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--deleted"]);
     insta::assert_snapshot!(stdout, @"");

docs/design/tracking-branches.md

@@ -197,8 +197,7 @@ In particular, a merge of local and remote targets is
      * ~`tags`~ (not implemented, but should be the same as `branches`)
   2. Pushes diff to the remote Git repo (as well as remote tracking branches
      in the backing Git repo.)
-  3. Sets `remotes[remote].branches[name].state = tracking`
+  3. Updates `remotes[remote]` and `git_refs` reflecting the push.
-  4. Import changes only for `remotes[remote].branches[glob]`
 
 * `jj git export`
   1. Copies local `branches`/`tags` back to `remotes["git"]`.
@@ -288,25 +287,22 @@ Note: desired behavior of `jj branch forget` is to
 
 * Pushing new branch, remote doesn't exist
   1. Pushes `[local, absent] - [absent]` -> `local`
-  2. Sets `remotes[remote].branches[name].state = tracking`
+  2. Sets `remotes[remote].branches[name].target = local`, `.state = tracking`
-  3. `import_refs()` merges `[local, local] - [absent]` -> `local` (noop)
 * Pushing new branch, untracked remote exists
   1. Pushes `[local, remote] - [absent]`
      * Fails if `local` moved backwards or sideways
-  2. Sets `remotes[remote].branches[name].state = tracking`
+  2. Sets `remotes[remote].branches[name].target = local`, `.state = tracking`
-  3. `import_refs()` merges `[local, local] - [remote]` -> `local` (noop)
 * Pushing existing branch to tracking remote
   1. Pushes `[local, remote] - [remote]` -> `local`
      * Fails if `local` moved backwards or sideways, and if `remote` is out of
        sync
-  2. `import_refs()` merges `[local, local] - [remote]` -> `local` (noop)
+  2. Sets `remotes[remote].branches[name].target = local`
 * Pushing existing branch to untracked remote
   * Same as "new branch"
 * Pushing deleted branch to tracking remote
   1. Pushes `[absent, remote] - [remote]` -> `absent`
      * TODO: Fails if `remote` is out of sync?
-  2. `import_refs()` merges `[absent, absent] - [remote]` -> `absent`
+  2. Removes `remotes[remote].branches[name]` (`target` becomes `absent`)
-  3. Removes `remotes[remote].branches[name]` (`target` becomes `absent`)
 * Pushing deleted branch to untracked remote
   * Noop since `[absent, remote] - [absent]` -> `remote`
   * Perhaps, UI will report error

lib/src/git.rs

@@ -1094,7 +1094,9 @@ pub struct GitRefUpdate {
     pub new_target: Option<CommitId>,
 }
 
+/// Pushes the specified branches and updates the repo view accordingly.
 pub fn push_branches(
+    mut_repo: &mut MutableRepo,
     git_repo: &git2::Repository,
     remote_name: &str,
     targets: &GitBranchPushTargets,
@@ -1110,9 +1112,24 @@ pub fn push_branches(
         })
         .collect_vec();
     push_updates(git_repo, remote_name, &ref_updates, callbacks)?;
+
+    // TODO: add support for partially pushed refs? we could update the view
+    // excluding rejected refs, but the transaction would be aborted anyway
+    // if we returned an Err.
+    for (branch_name, update) in &targets.branch_updates {
+        let git_ref_name = format!("refs/remotes/{remote_name}/{branch_name}");
+        let new_remote_ref = RemoteRef {
+            target: RefTarget::resolved(update.new_target.clone()),
+            state: RemoteRefState::Tracking,
+        };
+        mut_repo.set_git_ref_target(&git_ref_name, new_remote_ref.target.clone());
+        mut_repo.set_remote_branch(branch_name, remote_name, new_remote_ref);
+    }
+
     Ok(())
 }
 
+/// Pushes the specified Git refs without updating the repo view.
 pub fn push_updates(
     git_repo: &git2::Repository,
     remote_name: &str,

lib/src/op_store.rs

@@ -83,6 +83,11 @@ impl RefTarget {
         &TARGET
     }
 
+    /// Creates non-conflicting target that optionally points to a commit.
+    pub fn resolved(maybe_id: Option<CommitId>) -> Self {
+        Self::from_merge(Merge::resolved(maybe_id))
+    }
+
     /// Creates non-conflicting target pointing to a commit.
     pub fn normal(id: CommitId) -> Self {
         Self::from_merge(Merge::normal(id))

lib/tests/test_git.rs

@@ -2256,6 +2256,20 @@ fn set_up_push_repos(settings: &UserSettings, temp_dir: &TempDir) -> PushTestSet
         .set_parents(vec![initial_commit.id().clone()])
         .write()
         .unwrap();
+    tx.mut_repo().set_git_ref_target(
+        "refs/remotes/origin/main",
+        RefTarget::normal(initial_commit.id().clone()),
+    );
+    tx.mut_repo().set_remote_branch(
+        "main",
+        "origin",
+        RemoteRef {
+            target: RefTarget::normal(initial_commit.id().clone()),
+            // Caller expects the main branch is tracked. The corresponding local branch will
+            // be created (or left as deleted) by caller.
+            state: RemoteRefState::Tracking,
+        },
+    );
     let jj_repo = tx.commit();
     PushTestSetup {
         source_repo_dir,
@@ -2269,8 +2283,9 @@ fn set_up_push_repos(settings: &UserSettings, temp_dir: &TempDir) -> PushTestSet
 fn test_push_branches_success() {
     let settings = testutils::user_settings();
     let temp_dir = testutils::new_temp_dir();
-    let setup = set_up_push_repos(&settings, &temp_dir);
+    let mut setup = set_up_push_repos(&settings, &temp_dir);
     let clone_repo = get_git_repo(&setup.jj_repo);
+    let mut tx = setup.jj_repo.start_transaction(&settings, "test");
 
     let targets = GitBranchPushTargets {
         branch_updates: vec![(
@@ -2283,6 +2298,7 @@ fn test_push_branches_success() {
         force_pushed_branches: hashset! {},
     };
     let result = git::push_branches(
+        tx.mut_repo(),
         &clone_repo,
         "origin",
         &targets,
@@ -2307,14 +2323,35 @@ fn test_push_branches_success() {
         .unwrap()
         .target();
     assert_eq!(new_target, Some(new_oid));
+
+    // Check that the repo view got updated
+    let view = tx.mut_repo().view();
+    assert_eq!(
+        *view.get_git_ref("refs/remotes/origin/main"),
+        RefTarget::normal(setup.new_commit.id().clone()),
+    );
+    assert_eq!(
+        *view.get_remote_branch("main", "origin"),
+        RemoteRef {
+            target: RefTarget::normal(setup.new_commit.id().clone()),
+            state: RemoteRefState::Tracking,
+        },
+    );
+
+    // Check that the repo view reflects the changes in the Git repo
+    setup.jj_repo = tx.commit();
+    let mut tx = setup.jj_repo.start_transaction(&settings, "test");
+    git::import_refs(tx.mut_repo(), &clone_repo, &GitSettings::default()).unwrap();
+    assert!(!tx.mut_repo().has_changes());
 }
 
 #[test]
 fn test_push_branches_deletion() {
     let settings = testutils::user_settings();
     let temp_dir = testutils::new_temp_dir();
-    let setup = set_up_push_repos(&settings, &temp_dir);
+    let mut setup = set_up_push_repos(&settings, &temp_dir);
     let clone_repo = get_git_repo(&setup.jj_repo);
+    let mut tx = setup.jj_repo.start_transaction(&settings, "test");
 
     let source_repo = git2::Repository::open(&setup.source_repo_dir).unwrap();
     // Test the setup
@@ -2331,6 +2368,7 @@ fn test_push_branches_deletion() {
         force_pushed_branches: hashset! {},
     };
     let result = git::push_branches(
+        tx.mut_repo(),
         &get_git_repo(&setup.jj_repo),
         "origin",
         &targets,
@@ -2347,14 +2385,26 @@ fn test_push_branches_deletion() {
     assert!(clone_repo
         .find_reference("refs/remotes/origin/main")
         .is_err());
+
+    // Check that the repo view got updated
+    let view = tx.mut_repo().view();
+    assert!(view.get_git_ref("refs/remotes/origin/main").is_absent());
+    assert!(view.get_remote_branch("main", "origin").is_absent());
+
+    // Check that the repo view reflects the changes in the Git repo
+    setup.jj_repo = tx.commit();
+    let mut tx = setup.jj_repo.start_transaction(&settings, "test");
+    git::import_refs(tx.mut_repo(), &clone_repo, &GitSettings::default()).unwrap();
+    assert!(!tx.mut_repo().has_changes());
 }
 
 #[test]
 fn test_push_branches_mixed_deletion_and_addition() {
     let settings = testutils::user_settings();
     let temp_dir = testutils::new_temp_dir();
-    let setup = set_up_push_repos(&settings, &temp_dir);
+    let mut setup = set_up_push_repos(&settings, &temp_dir);
     let clone_repo = get_git_repo(&setup.jj_repo);
+    let mut tx = setup.jj_repo.start_transaction(&settings, "test");
 
     let targets = GitBranchPushTargets {
         branch_updates: vec![
@@ -2376,6 +2426,7 @@ fn test_push_branches_mixed_deletion_and_addition() {
         force_pushed_branches: hashset! {},
     };
     let result = git::push_branches(
+        tx.mut_repo(),
         &clone_repo,
         "origin",
         &targets,
@@ -2393,6 +2444,28 @@ fn test_push_branches_mixed_deletion_and_addition() {
 
     // Check that the main ref got deleted in the source repo
     assert!(source_repo.find_reference("refs/heads/main").is_err());
+
+    // Check that the repo view got updated
+    let view = tx.mut_repo().view();
+    assert!(view.get_git_ref("refs/remotes/origin/main").is_absent());
+    assert!(view.get_remote_branch("main", "origin").is_absent());
+    assert_eq!(
+        *view.get_git_ref("refs/remotes/origin/topic"),
+        RefTarget::normal(setup.new_commit.id().clone()),
+    );
+    assert_eq!(
+        *view.get_remote_branch("topic", "origin"),
+        RemoteRef {
+            target: RefTarget::normal(setup.new_commit.id().clone()),
+            state: RemoteRefState::Tracking,
+        },
+    );
+
+    // Check that the repo view reflects the changes in the Git repo
+    setup.jj_repo = tx.commit();
+    let mut tx = setup.jj_repo.start_transaction(&settings, "test");
+    git::import_refs(tx.mut_repo(), &clone_repo, &GitSettings::default()).unwrap();
+    assert!(!tx.mut_repo().has_changes());
 }
 
 #[test]
@@ -2403,6 +2476,7 @@ fn test_push_branches_not_fast_forward() {
     let mut tx = setup.jj_repo.start_transaction(&settings, "test");
     let new_commit = write_random_commit(tx.mut_repo(), &settings);
     setup.jj_repo = tx.commit();
+    let mut tx = setup.jj_repo.start_transaction(&settings, "test");
 
     let targets = GitBranchPushTargets {
         branch_updates: vec![(
@@ -2415,6 +2489,7 @@ fn test_push_branches_not_fast_forward() {
         force_pushed_branches: hashset! {},
     };
     let result = git::push_branches(
+        tx.mut_repo(),
         &get_git_repo(&setup.jj_repo),
         "origin",
         &targets,
@@ -2431,6 +2506,7 @@ fn test_push_branches_not_fast_forward_with_force() {
     let mut tx = setup.jj_repo.start_transaction(&settings, "test");
     let new_commit = write_random_commit(tx.mut_repo(), &settings);
     setup.jj_repo = tx.commit();
+    let mut tx = setup.jj_repo.start_transaction(&settings, "test");
 
     let targets = GitBranchPushTargets {
         branch_updates: vec![(
@@ -2445,6 +2521,7 @@ fn test_push_branches_not_fast_forward_with_force() {
         },
     };
     let result = git::push_branches(
+        tx.mut_repo(),
         &get_git_repo(&setup.jj_repo),
         "origin",
         &targets,