Skip to content

Commit

Permalink
revset: do not lookup unimported tags or remote branches by unqualifi…
Browse files Browse the repository at this point in the history
…ed name

Since e7e4952 "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.
  • Loading branch information
yuja committed Sep 6, 2023
1 parent d228b63 commit 787fd34
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 19 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions lib/src/revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
26 changes: 10 additions & 16 deletions lib/tests/test_revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 { .. })
);
}

Expand Down Expand Up @@ -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###"
Expand All @@ -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(),
Expand All @@ -744,19 +738,19 @@ 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
mut_repo.set_git_ref_target(
"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.
Expand Down

0 comments on commit 787fd34

Please sign in to comment.