From e160970b794374e1849960f2bdd8d0601a3ca012 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Mon, 2 Oct 2023 01:32:25 +0900
Subject: [PATCH] git: migrate import_refs() to diffing git_refs and known
 remote refs

---
 cli/tests/test_git_import_export.rs | 22 +++-----------------
 lib/src/git.rs                      | 32 +++++++++++++++++++++--------
 2 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/cli/tests/test_git_import_export.rs b/cli/tests/test_git_import_export.rs
index c09bd820c..99956f8dc 100644
--- a/cli/tests/test_git_import_export.rs
+++ b/cli/tests/test_git_import_export.rs
@@ -143,8 +143,9 @@ fn test_git_import_undo() {
     a: qpvuntsm 230dd059 (empty) (no description set)
     "###);
 
-    // If we don't restore the git_refs, undoing the import removes the local branch
-    // but makes a following import a no-op.
+    // Even if we don't restore the git_refs, "git import" can re-import the
+    // local branch "a".
+    // TODO: This will be the default "op restore" mode.
     let stdout = test_env.jj_cmd_success(
         &repo_path,
         &[
@@ -158,23 +159,6 @@ fn test_git_import_undo() {
     insta::assert_snapshot!(stdout, @r###"
     "###);
     insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @"");
-    // Try "git import" again, which should *not* re-import the branch "a" and be a
-    // no-op.
-    insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["git", "import"]), @r###"
-    Nothing changed.
-    "###);
-    insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @"");
-
-    // We can restore *only* the git refs to make an import re-import the branch
-    let stdout = test_env.jj_cmd_success(
-        &repo_path,
-        &["op", "restore", &base_operation_id, "--what=git-tracking"],
-    );
-    insta::assert_snapshot!(stdout, @r###"
-    "###);
-    // The git-tracking branch disappears.
-    insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @"");
-    // Try "git import" again, which should again re-import the branch "a".
     insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["git", "import"]), @"");
     insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
     a: qpvuntsm 230dd059 (empty) (no description set)
diff --git a/lib/src/git.rs b/lib/src/git.rs
index ab780915f..33b689ea0 100644
--- a/lib/src/git.rs
+++ b/lib/src/git.rs
@@ -340,15 +340,29 @@ fn diff_refs_to_import(
             git_ref_filter(&ref_name).then_some((full_name.as_ref(), target))
         })
         .collect();
-    let mut known_remote_refs: HashMap<RefName, &RefTarget> = view
-        .git_refs()
-        .iter()
-        .filter_map(|(full_name, target)| {
-            // TODO: or clean up invalid ref in case it was stored due to historical bug?
-            let ref_name = parse_git_ref(full_name).expect("stored git ref should be parsable");
-            git_ref_filter(&ref_name).then_some((ref_name, target))
-        })
-        .collect();
+    let mut known_remote_refs: HashMap<RefName, &RefTarget> = itertools::chain(
+        view.remote_branches().map(|((branch, remote), target)| {
+            // TODO: want to abstract local ref as "git" tracking remote, but
+            // we'll probably need to refactor the git_ref_filter API first.
+            let ref_name = if remote == REMOTE_NAME_FOR_LOCAL_GIT_REPO {
+                RefName::LocalBranch(branch.to_owned())
+            } else {
+                RefName::RemoteBranch {
+                    branch: branch.to_owned(),
+                    remote: remote.to_owned(),
+                }
+            };
+            (ref_name, target)
+        }),
+        // TODO: compare to tags stored in the "git" remote view. Since tags should never
+        // be moved locally in jj, we can consider local tags as merge base.
+        view.tags().iter().map(|(name, target)| {
+            let ref_name = RefName::Tag(name.to_owned());
+            (ref_name, target)
+        }),
+    )
+    .filter(|(ref_name, _)| git_ref_filter(ref_name))
+    .collect();
     let mut changed_git_refs = Vec::new();
     let mut changed_remote_refs = BTreeMap::new();
     for git_ref in git_repo.references()? {