From a868b2d9a5d6dbedfe850821f587ccb240f27593 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Wed, 6 Sep 2023 10:24:11 +0900
Subject: [PATCH] revset: do not lookup unimported tags or remote branches by
 unqualified name

Since e7e49527efdf "git: ensure that remote branches never diverge", the last
known "refs/remotes" ref should be synced with the corresponding remote branch.
So we can always trust the branch@remote expression. We don't need "refs/tags"
lookup either since tags should have been imported by git::import_refs().

FWIW, I'm thinking of reorganizing view.git_refs() map as per-remote views.
It would be nice if we can get rid of revsets and template keywords exposing
low-level Git ref primitives.
---
 CHANGELOG.md             |  3 +++
 lib/src/revset.rs        |  4 +---
 lib/tests/test_revset.rs | 26 ++++++++++----------------
 3 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4195c22244..4006c65405 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -62,6 +62,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   rename` to rename the existing remote.
   [#1690](https://github.com/martinvonz/jj/issues/1690)
 
+* Revset expression like `origin/main` will no longer resolve to a
+  remote-tracking branch. Use `main@origin` instead.
+
 ### New features
 
 * Default template for `jj log` now does not show irrelevant information
diff --git a/lib/src/revset.rs b/lib/src/revset.rs
index 04e267cb6e..b3c60f92eb 100644
--- a/lib/src/revset.rs
+++ b/lib/src/revset.rs
@@ -1899,9 +1899,7 @@ fn filter_map_values_by_key_pattern<'a: 'b, 'b, V>(
 
 fn resolve_git_ref(repo: &dyn Repo, symbol: &str) -> Option<Vec<CommitId>> {
     let view = repo.view();
-    // TODO: We should remove `refs/remotes` from this list once we have a better
-    // way to address local git repo's remote-tracking branches.
-    for git_ref_prefix in &["", "refs/", "refs/tags/", "refs/remotes/"] {
+    for git_ref_prefix in &["", "refs/"] {
         let target = view.get_git_ref(&(git_ref_prefix.to_string() + symbol));
         if target.is_present() {
             return Some(target.added_ids().cloned().collect());
diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs
index b525ced416..e6696f1ae7 100644
--- a/lib/tests/test_revset.rs
+++ b/lib/tests/test_revset.rs
@@ -615,10 +615,9 @@ fn test_resolve_symbol_tags() {
         vec![commit1.id().clone()],
     );
 
-    // TODO: remove refs/tags lookup
-    assert_eq!(
-        resolve_symbol(mut_repo, "unimported").unwrap(),
-        vec![commit3.id().clone()],
+    assert_matches!(
+        resolve_symbol(mut_repo, "unimported"),
+        Err(RevsetResolutionError::NoSuchRevision { .. })
     );
 }
 
@@ -718,6 +717,7 @@ fn test_resolve_symbol_git_refs() {
 
     // Qualified with only heads/
     mut_repo.set_git_ref_target("refs/heads/branch", RefTarget::normal(commit5.id().clone()));
+    mut_repo.set_git_ref_target("refs/tags/branch", RefTarget::normal(commit4.id().clone()));
     // branch alone is not recognized
     insta::assert_debug_snapshot!(
         resolve_symbol(mut_repo, "branch").unwrap_err(), @r###"
@@ -730,12 +730,6 @@ fn test_resolve_symbol_git_refs() {
         ],
     }
     "###);
-    mut_repo.set_git_ref_target("refs/tags/branch", RefTarget::normal(commit4.id().clone()));
-    // The *tag* branch is recognized
-    assert_eq!(
-        resolve_symbol(mut_repo, "branch").unwrap(),
-        vec![commit4.id().clone()]
-    );
     // heads/branch does get resolved to the git ref refs/heads/branch
     assert_eq!(
         resolve_symbol(mut_repo, "heads/branch").unwrap(),
@@ -744,9 +738,9 @@ fn test_resolve_symbol_git_refs() {
 
     // Unqualified tag name
     mut_repo.set_git_ref_target("refs/tags/tag", RefTarget::normal(commit4.id().clone()));
-    assert_eq!(
-        resolve_symbol(mut_repo, "tag").unwrap(),
-        vec![commit4.id().clone()]
+    assert_matches!(
+        resolve_symbol(mut_repo, "tag"),
+        Err(RevsetResolutionError::NoSuchRevision { .. })
     );
 
     // Unqualified remote-tracking branch name
@@ -754,9 +748,9 @@ fn test_resolve_symbol_git_refs() {
         "refs/remotes/origin/remote-branch",
         RefTarget::normal(commit2.id().clone()),
     );
-    assert_eq!(
-        resolve_symbol(mut_repo, "origin/remote-branch").unwrap(),
-        vec![commit2.id().clone()]
+    assert_matches!(
+        resolve_symbol(mut_repo, "origin/remote-branch"),
+        Err(RevsetResolutionError::NoSuchRevision { .. })
     );
 
     // "@" (quoted) can be resolved, and root is a normal symbol.