index: fix reindexing to scan all referenced commits such as hidden remote refs

Since hidden commits can be looked up by remote_branches() revset for example,
reindexing should traverse ancestors from all named refs in addition to the
visible heads.

lib/src/default_index/store.rs

@@ -147,7 +147,8 @@ impl DefaultIndexStore {
         let operations_dir = self.dir.join("operations");
         let commit_id_length = store.commit_id_length();
         let change_id_length = store.change_id_length();
-        let mut visited_heads: HashSet<CommitId> = view.heads().clone();
+        let mut visited_heads: HashSet<CommitId> =
+            view.all_referenced_commit_ids().cloned().collect();
         let mut historical_heads: Vec<(CommitId, OperationId)> = visited_heads
             .iter()
             .map(|commit_id| (commit_id.clone(), operation.id().clone()))
@@ -167,7 +168,7 @@ impl DefaultIndexStore {
                 parent_op_id = Some(op.id().clone());
             }
             // TODO: no need to walk ancestors of the parent_op_id operation
-            for commit_id in op.view()?.heads() {
+            for commit_id in op.view()?.all_referenced_commit_ids() {
                 if visited_heads.insert(commit_id.clone()) {
                     historical_heads.push((commit_id.clone(), op.id().clone()));
                 }

lib/src/view.rs

@@ -311,6 +311,50 @@ impl View {
         self.data.git_head = target;
     }
 
+    /// Iterates all commit ids referenced by this view.
+    ///
+    /// This can include hidden commits referenced by remote branches, previous
+    /// positions of conflicted branches, etc. The ancestors and predecessors of
+    /// the returned commits should be considered reachable from the view. Use
+    /// this to build commit index from scratch.
+    ///
+    /// The iteration order is unspecified, and may include duplicated entries.
+    pub fn all_referenced_commit_ids(&self) -> impl Iterator<Item = &CommitId> {
+        // Include both added/removed ids since ancestry information of old
+        // references will be needed while merging views.
+        fn ref_target_ids(target: &RefTarget) -> impl Iterator<Item = &CommitId> {
+            target.as_merge().iter().flatten()
+        }
+
+        // Some of the fields (e.g. wc_commit_ids) would be redundant, but let's
+        // not be smart here. Callers will build a larger set of commits anyway.
+        let op_store::View {
+            head_ids,
+            public_head_ids,
+            local_branches,
+            tags,
+            remote_views,
+            git_refs,
+            git_head,
+            wc_commit_ids,
+        } = &self.data;
+        itertools::chain!(
+            head_ids,
+            public_head_ids,
+            local_branches.values().flat_map(ref_target_ids),
+            tags.values().flat_map(ref_target_ids),
+            remote_views.values().flat_map(|remote_view| {
+                let op_store::RemoteView { branches } = remote_view;
+                branches
+                    .values()
+                    .flat_map(|remote_ref| ref_target_ids(&remote_ref.target))
+            }),
+            git_refs.values().flat_map(ref_target_ids),
+            ref_target_ids(git_head),
+            wc_commit_ids.values()
+        )
+    }
+
     pub fn set_view(&mut self, data: op_store::View) {
         self.data = data;
     }

lib/tests/test_index.rs

@@ -25,6 +25,7 @@ use jj_lib::default_index::{
 };
 use jj_lib::index::Index as _;
 use jj_lib::object_id::{HexPrefix, ObjectId as _, PrefixResolution};
+use jj_lib::op_store::{RefTarget, RemoteRef};
 use jj_lib::repo::{MutableRepo, ReadonlyRepo, Repo};
 use jj_lib::settings::UserSettings;
 use testutils::test_backend::TestBackend;
@@ -313,6 +314,54 @@ fn test_index_commits_previous_operations() {
     assert_eq!(generation_number(index, commit_c.id()), 3);
 }
 
+#[test]
+fn test_index_commits_hidden_but_referenced() {
+    // Test that hidden-but-referenced commits are indexed.
+    let settings = testutils::user_settings();
+    let test_repo = TestRepo::init();
+    let repo = &test_repo.repo;
+
+    // Remote branches are usually visible at a certain point in operation
+    // history, but that's not guaranteed if old operations have been discarded.
+    // This can also happen if imported remote branches get immediately
+    // abandoned because the other branch has moved.
+    let mut tx = repo.start_transaction(&settings);
+    let commit_a = write_random_commit(tx.mut_repo(), &settings);
+    let commit_b = write_random_commit(tx.mut_repo(), &settings);
+    let commit_c = write_random_commit(tx.mut_repo(), &settings);
+    tx.mut_repo().remove_head(commit_a.id());
+    tx.mut_repo().remove_head(commit_b.id());
+    tx.mut_repo().remove_head(commit_c.id());
+    tx.mut_repo().set_remote_branch(
+        "branch",
+        "origin",
+        RemoteRef {
+            target: RefTarget::from_legacy_form(
+                [commit_a.id().clone()],
+                [commit_b.id().clone(), commit_c.id().clone()],
+            ),
+            state: jj_lib::op_store::RemoteRefState::New,
+        },
+    );
+    let repo = tx.commit("test");
+
+    // All commits should be indexed
+    assert!(repo.index().has_id(commit_a.id()));
+    assert!(repo.index().has_id(commit_b.id()));
+    assert!(repo.index().has_id(commit_c.id()));
+
+    // Delete index from disk
+    let default_index_store: &DefaultIndexStore =
+        repo.index_store().as_any().downcast_ref().unwrap();
+    default_index_store.reinit().unwrap();
+
+    let repo = load_repo_at_head(&settings, repo.repo_path());
+    // All commits should be reindexed
+    assert!(repo.index().has_id(commit_a.id()));
+    assert!(repo.index().has_id(commit_b.id()));
+    assert!(repo.index().has_id(commit_c.id()));
+}
+
 #[test]
 fn test_index_commits_incremental() {
     let settings = testutils::user_settings();