view: add stub method to set remote branch target and state, migrate …

…callers

Since set_remote_branch_target() is called while merging refs, its tracking
state shouldn't be reinitialized. The other callers are migrated to new setter
to keep the story simple.

lib/src/git.rs

@@ -28,7 +28,7 @@ use thiserror::Error;
 use crate::backend::{BackendError, CommitId, ObjectId};
 use crate::commit::Commit;
 use crate::git_backend::GitBackend;
-use crate::op_store::{RefTarget, RefTargetOptionExt};
+use crate::op_store::{RefTarget, RefTargetOptionExt, RemoteRef};
 use crate::repo::{MutableRepo, Repo};
 use crate::revset::{self, RevsetExpression};
 use crate::settings::GitSettings;
@@ -261,10 +261,13 @@ pub fn import_some_refs(
         mut_repo.set_git_ref_target(&full_name, new_target);
     }
     for (ref_name, (old_target, new_target)) in &changed_remote_refs {
+        let new_remote_ref = RemoteRef {
+            target: new_target.clone(),
+        };
         if let RefName::RemoteBranch { branch, remote } = ref_name {
             // Remote-tracking branch is the last known state of the branch in the remote.
             // It shouldn't diverge even if we had inconsistent view.
-            mut_repo.set_remote_branch_target(branch, remote, new_target.clone());
+            mut_repo.set_remote_branch(branch, remote, new_remote_ref);
             // If a git remote-tracking branch changed, apply the change to the local branch
             // as well.
             if git_settings.auto_local_branch {
@@ -274,11 +277,7 @@ pub fn import_some_refs(
         } else {
             if let RefName::LocalBranch(branch) = ref_name {
                 // Update Git-tracking branch like the other remote branches.
-                mut_repo.set_remote_branch_target(
-                    branch,
-                    REMOTE_NAME_FOR_LOCAL_GIT_REPO,
-                    new_target.clone(),
-                );
+                mut_repo.set_remote_branch(branch, REMOTE_NAME_FOR_LOCAL_GIT_REPO, new_remote_ref);
             }
             mut_repo.merge_single_ref(ref_name, old_target, new_target);
         }
@@ -574,7 +573,8 @@ fn copy_exportable_local_branches_to_remote_view(
         .map(|(branch, new_target)| (branch.to_owned(), new_target.clone()))
         .collect_vec();
     for (branch, new_target) in new_local_branches {
-        mut_repo.set_remote_branch_target(&branch, remote_name, new_target);
+        let new_remote_ref = RemoteRef { target: new_target };
+        mut_repo.set_remote_branch(&branch, remote_name, new_remote_ref);
     }
 }
 

lib/src/repo.rs

@@ -1003,9 +1003,9 @@ impl MutableRepo {
             .with_ref(|v| v.get_remote_branch(name, remote_name).clone())
     }
 
-    pub fn set_remote_branch_target(&mut self, name: &str, remote_name: &str, target: RefTarget) {
+    pub fn set_remote_branch(&mut self, name: &str, remote_name: &str, remote_ref: RemoteRef) {
         self.view_mut()
-            .set_remote_branch_target(name, remote_name, target);
+            .set_remote_branch(name, remote_name, remote_ref);
     }
 
     pub fn remove_remote(&mut self, remote_name: &str) {

lib/src/view.rs

@@ -221,17 +221,38 @@ impl View {
         }
     }
 
+    /// Sets remote-tracking branch to the given target and state. If the target
+    /// is absent, the branch will be removed.
+    pub fn set_remote_branch(&mut self, name: &str, remote_name: &str, remote_ref: RemoteRef) {
+        if remote_ref.is_present() {
+            let remote_view = self
+                .data
+                .remote_views
+                .entry(remote_name.to_owned())
+                .or_default();
+            remote_view.branches.insert(name.to_owned(), remote_ref);
+        } else if let Some(remote_view) = self.data.remote_views.get_mut(remote_name) {
+            remote_view.branches.remove(name);
+        }
+    }
+
     /// Sets remote-tracking branch to point to the given target. If the target
     /// is absent, the branch will be removed.
-    pub fn set_remote_branch_target(&mut self, name: &str, remote_name: &str, target: RefTarget) {
+    ///
+    /// If the branch already exists, its tracking state won't be changed.
+    fn set_remote_branch_target(&mut self, name: &str, remote_name: &str, target: RefTarget) {
         if target.is_present() {
-            let remote_ref = RemoteRef { target }; // TODO: preserve or reset tracking flag?
             let remote_view = self
                 .data
                 .remote_views
                 .entry(remote_name.to_owned())
                 .or_default();
-            remote_view.branches.insert(name.to_owned(), remote_ref);
+            if let Some(remote_ref) = remote_view.branches.get_mut(name) {
+                remote_ref.target = target;
+            } else {
+                let remote_ref = RemoteRef { target };
+                remote_view.branches.insert(name.to_owned(), remote_ref);
+            }
         } else if let Some(remote_view) = self.data.remote_views.get_mut(remote_name) {
             remote_view.branches.remove(name);
         }

lib/tests/test_git.rs

@@ -1709,7 +1709,7 @@ fn test_export_undo_reexport() {
     );
 
     // Undo remote changes only
-    mut_repo.set_remote_branch_target("main", "git", RefTarget::absent());
+    mut_repo.set_remote_branch("main", "git", RemoteRef::absent());
 
     // Reexport should update the Git-tracking branch
     assert!(git::export_refs(mut_repo, &git_repo).unwrap().is_empty());

lib/tests/test_mut_repo.rs

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use jj_lib::backend::CommitId;
 use jj_lib::op_store::{RefTarget, RemoteRef, WorkspaceId};
 use jj_lib::repo::Repo;
 use maplit::hashset;
@@ -441,6 +442,9 @@ fn test_has_changed() {
     let settings = testutils::user_settings();
     let test_repo = TestRepo::init();
     let repo = &test_repo.repo;
+    let normal_remote_ref = |id: &CommitId| RemoteRef {
+        target: RefTarget::normal(id.clone()),
+    };
 
     let mut tx = repo.start_transaction(&settings, "test");
     let mut_repo = tx.mut_repo();
@@ -453,7 +457,7 @@ fn test_has_changed() {
         .set_wc_commit(ws_id.clone(), commit1.id().clone())
         .unwrap();
     mut_repo.set_local_branch_target("main", RefTarget::normal(commit1.id().clone()));
-    mut_repo.set_remote_branch_target("main", "origin", RefTarget::normal(commit1.id().clone()));
+    mut_repo.set_remote_branch("main", "origin", normal_remote_ref(commit1.id()));
     let repo = tx.commit();
     // Test the setup
     assert_eq!(repo.view().heads(), &hashset! {commit1.id().clone()});
@@ -468,13 +472,13 @@ fn test_has_changed() {
         .set_wc_commit(ws_id.clone(), commit1.id().clone())
         .unwrap();
     mut_repo.set_local_branch_target("main", RefTarget::normal(commit1.id().clone()));
-    mut_repo.set_remote_branch_target("main", "origin", RefTarget::normal(commit1.id().clone()));
+    mut_repo.set_remote_branch("main", "origin", normal_remote_ref(commit1.id()));
     assert!(!mut_repo.has_changes());
 
     mut_repo.remove_public_head(commit2.id());
     mut_repo.remove_head(commit2.id());
     mut_repo.set_local_branch_target("stable", RefTarget::absent());
-    mut_repo.set_remote_branch_target("stable", "origin", RefTarget::absent());
+    mut_repo.set_remote_branch("stable", "origin", RemoteRef::absent());
     assert!(!mut_repo.has_changes());
 
     mut_repo.add_head(&commit2);
@@ -504,9 +508,9 @@ fn test_has_changed() {
     mut_repo.set_local_branch_target("main", RefTarget::normal(commit1.id().clone()));
     assert!(!mut_repo.has_changes());
 
-    mut_repo.set_remote_branch_target("main", "origin", RefTarget::normal(commit2.id().clone()));
+    mut_repo.set_remote_branch("main", "origin", normal_remote_ref(commit2.id()));
     assert!(mut_repo.has_changes());
-    mut_repo.set_remote_branch_target("main", "origin", RefTarget::normal(commit1.id().clone()));
+    mut_repo.set_remote_branch("main", "origin", normal_remote_ref(commit1.id()));
     assert!(!mut_repo.has_changes());
 }
 
@@ -593,7 +597,7 @@ fn test_rename_remote() {
     let remote_ref = RemoteRef {
         target: RefTarget::normal(commit.id().clone()),
     };
-    mut_repo.set_remote_branch_target("main", "origin", remote_ref.target.clone());
+    mut_repo.set_remote_branch("main", "origin", remote_ref.clone());
     mut_repo.rename_remote("origin", "upstream");
     assert_eq!(mut_repo.get_remote_branch("main", "upstream"), remote_ref);
     assert_eq!(

lib/tests/test_revset.rs

@@ -26,7 +26,7 @@ use jj_lib::commit::Commit;
 use jj_lib::git;
 use jj_lib::git_backend::GitBackend;
 use jj_lib::index::{HexPrefix, PrefixResolution};
-use jj_lib::op_store::{RefTarget, WorkspaceId};
+use jj_lib::op_store::{RefTarget, RemoteRef, WorkspaceId};
 use jj_lib::repo::Repo;
 use jj_lib::repo_path::RepoPath;
 use jj_lib::revset::{
@@ -382,6 +382,8 @@ fn test_resolve_symbol_branches() {
     let settings = testutils::user_settings();
     let test_repo = TestRepo::init();
     let repo = &test_repo.repo;
+    let remote_ref = |target| RemoteRef { target };
+    let normal_remote_ref = |id: &CommitId| remote_ref(RefTarget::normal(id.clone()));
 
     let mut tx = repo.start_transaction(&settings, "test");
     let mut_repo = tx.mut_repo();
@@ -393,26 +395,22 @@ fn test_resolve_symbol_branches() {
     let commit5 = write_random_commit(mut_repo, &settings);
 
     mut_repo.set_local_branch_target("local", RefTarget::normal(commit1.id().clone()));
-    mut_repo.set_remote_branch_target("remote", "origin", RefTarget::normal(commit2.id().clone()));
+    mut_repo.set_remote_branch("remote", "origin", normal_remote_ref(commit2.id()));
     mut_repo.set_local_branch_target("local-remote", RefTarget::normal(commit3.id().clone()));
-    mut_repo.set_remote_branch_target(
-        "local-remote",
-        "origin",
-        RefTarget::normal(commit4.id().clone()),
-    );
+    mut_repo.set_remote_branch("local-remote", "origin", normal_remote_ref(commit4.id()));
     mut_repo.set_local_branch_target(
         "local-remote@origin", // not a remote branch
         RefTarget::normal(commit5.id().clone()),
     );
-    mut_repo.set_remote_branch_target(
+    mut_repo.set_remote_branch(
         "local-remote",
         "mirror",
-        mut_repo.get_local_branch("local-remote"),
+        remote_ref(mut_repo.get_local_branch("local-remote")),
     );
-    mut_repo.set_remote_branch_target(
+    mut_repo.set_remote_branch(
         "local-remote",
         git::REMOTE_NAME_FOR_LOCAL_GIT_REPO,
-        mut_repo.get_local_branch("local-remote"),
+        remote_ref(mut_repo.get_local_branch("local-remote")),
     );
 
     mut_repo.set_local_branch_target(
@@ -422,13 +420,13 @@ fn test_resolve_symbol_branches() {
             [commit3.id().clone(), commit2.id().clone()],
         ),
     );
-    mut_repo.set_remote_branch_target(
+    mut_repo.set_remote_branch(
         "remote-conflicted",
         "origin",
-        RefTarget::from_legacy_form(
+        remote_ref(RefTarget::from_legacy_form(
             [commit3.id().clone()],
             [commit5.id().clone(), commit4.id().clone()],
-        ),
+        )),
     );
 
     // Local only
@@ -1798,6 +1796,8 @@ fn test_evaluate_expression_remote_branches() {
     let settings = testutils::user_settings();
     let test_repo = TestRepo::init();
     let repo = &test_repo.repo;
+    let remote_ref = |target| RemoteRef { target };
+    let normal_remote_ref = |id: &CommitId| remote_ref(RefTarget::normal(id.clone()));
 
     let mut tx = repo.start_transaction(&settings, "test");
     let mut_repo = tx.mut_repo();
@@ -1810,12 +1810,8 @@ fn test_evaluate_expression_remote_branches() {
     // Can get branches when there are none
     assert_eq!(resolve_commit_ids(mut_repo, "remote_branches()"), vec![]);
     // Can get a few branches
-    mut_repo.set_remote_branch_target("branch1", "origin", RefTarget::normal(commit1.id().clone()));
-    mut_repo.set_remote_branch_target(
-        "branch2",
-        "private",
-        RefTarget::normal(commit2.id().clone()),
-    );
+    mut_repo.set_remote_branch("branch1", "origin", normal_remote_ref(commit1.id()));
+    mut_repo.set_remote_branch("branch2", "private", normal_remote_ref(commit2.id()));
     assert_eq!(
         resolve_commit_ids(mut_repo, "remote_branches()"),
         vec![commit2.id().clone(), commit1.id().clone()]
@@ -1882,7 +1878,7 @@ fn test_evaluate_expression_remote_branches() {
     );
     // Two branches pointing to the same commit does not result in a duplicate in
     // the revset
-    mut_repo.set_remote_branch_target("branch3", "origin", RefTarget::normal(commit2.id().clone()));
+    mut_repo.set_remote_branch("branch3", "origin", normal_remote_ref(commit2.id()));
     assert_eq!(
         resolve_commit_ids(mut_repo, "remote_branches()"),
         vec![commit2.id().clone(), commit1.id().clone()]
@@ -1894,23 +1890,23 @@ fn test_evaluate_expression_remote_branches() {
         vec![commit2.id().clone(), commit1.id().clone()]
     );
     // Can get branches when there are conflicted refs
-    mut_repo.set_remote_branch_target(
+    mut_repo.set_remote_branch(
         "branch1",
         "origin",
-        RefTarget::from_legacy_form(
+        remote_ref(RefTarget::from_legacy_form(
             [commit1.id().clone()],
             [commit2.id().clone(), commit3.id().clone()],
-        ),
+        )),
     );
-    mut_repo.set_remote_branch_target(
+    mut_repo.set_remote_branch(
         "branch2",
         "private",
-        RefTarget::from_legacy_form(
+        remote_ref(RefTarget::from_legacy_form(
             [commit2.id().clone()],
             [commit3.id().clone(), commit4.id().clone()],
-        ),
+        )),
     );
-    mut_repo.set_remote_branch_target("branch3", "origin", RefTarget::absent());
+    mut_repo.set_remote_branch("branch3", "origin", RemoteRef::absent());
     assert_eq!(
         resolve_commit_ids(mut_repo, "remote_branches()"),
         vec![

lib/tests/test_rewrite.rs

@@ -1006,7 +1006,7 @@ fn test_rebase_descendants_basic_branch_update_with_non_local_branch() {
     tx.mut_repo()
         .set_local_branch_target("main", RefTarget::normal(commit_b.id().clone()));
     tx.mut_repo()
-        .set_remote_branch_target("main", "origin", commit_b_remote_ref.target.clone());
+        .set_remote_branch("main", "origin", commit_b_remote_ref.clone());
     tx.mut_repo()
         .set_tag_target("v1", RefTarget::normal(commit_b.id().clone()));
     let repo = tx.commit();

lib/tests/test_view.rs

@@ -238,19 +238,24 @@ fn test_merge_views_branches() {
     let main_branch_origin_tx0 = write_random_commit(mut_repo, &settings);
     let main_branch_origin_tx1 = write_random_commit(mut_repo, &settings);
     let main_branch_alternate_tx0 = write_random_commit(mut_repo, &settings);
+    let main_branch_origin_tx0_remote_ref = RemoteRef {
+        target: RefTarget::normal(main_branch_origin_tx0.id().clone()),
+    };
+    let main_branch_origin_tx1_remote_ref = RemoteRef {
+        target: RefTarget::normal(main_branch_origin_tx1.id().clone()),
+    };
+    let main_branch_alternate_tx0_remote_ref = RemoteRef {
+        target: RefTarget::normal(main_branch_alternate_tx0.id().clone()),
+    };
     mut_repo.set_local_branch_target(
         "main",
         RefTarget::normal(main_branch_local_tx0.id().clone()),
     );
-    mut_repo.set_remote_branch_target(
-        "main",
-        "origin",
-        RefTarget::normal(main_branch_origin_tx0.id().clone()),
-    );
-    mut_repo.set_remote_branch_target(
+    mut_repo.set_remote_branch("main", "origin", main_branch_origin_tx0_remote_ref);
+    mut_repo.set_remote_branch(
         "main",
         "alternate",
-        RefTarget::normal(main_branch_alternate_tx0.id().clone()),
+        main_branch_alternate_tx0_remote_ref.clone(),
     );
     let feature_branch_local_tx0 = write_random_commit(mut_repo, &settings);
     mut_repo.set_local_branch_target(
@@ -265,11 +270,8 @@ fn test_merge_views_branches() {
         "main",
         RefTarget::normal(main_branch_local_tx1.id().clone()),
     );
-    tx1.mut_repo().set_remote_branch_target(
-        "main",
-        "origin",
-        RefTarget::normal(main_branch_origin_tx1.id().clone()),
-    );
+    tx1.mut_repo()
+        .set_remote_branch("main", "origin", main_branch_origin_tx1_remote_ref.clone());
     let feature_branch_tx1 = write_random_commit(tx1.mut_repo(), &settings);
     tx1.mut_repo().set_local_branch_target(
         "feature",
@@ -282,19 +284,10 @@ fn test_merge_views_branches() {
         "main",
         RefTarget::normal(main_branch_local_tx2.id().clone()),
     );
-    tx2.mut_repo().set_remote_branch_target(
-        "main",
-        "origin",
-        RefTarget::normal(main_branch_origin_tx1.id().clone()),
-    );
+    tx2.mut_repo()
+        .set_remote_branch("main", "origin", main_branch_origin_tx1_remote_ref.clone());
 
     let repo = commit_transactions(&settings, vec![tx1, tx2]);
-    let main_branch_origin_tx1_remote_ref = RemoteRef {
-        target: RefTarget::normal(main_branch_origin_tx1.id().clone()),
-    };
-    let main_branch_alternate_tx0_remote_ref = RemoteRef {
-        target: RefTarget::normal(main_branch_alternate_tx0.id().clone()),
-    };
     let expected_main_branch = BranchTarget {
         local_target: &RefTarget::from_legacy_form(
             [main_branch_local_tx0.id().clone()],