Skip to content

Commit

Permalink
revset: add optional tracked arg to remote_branches()
Browse files Browse the repository at this point in the history
Adds support for `remote_branches(tracked=false)` and
`remote_branches(tracked=true)`. I think this would be especially useful
for configuring `immutable_heads()` because rewriting untracked remote
branches usually wouldn't be desirable (since it wouldn't update the
remote branch). It also makes it easy to hide branches that you don't
care about from the log, since you could hide untracked branches and
then only track branches that you care about.
  • Loading branch information
scott2000 committed Jul 11, 2024
1 parent 503771c commit 0de177b
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 21 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
address unconditionally. Only ASCII case folding is currently implemented,
but this will likely change in the future.

* `remote_branches()` revset now supports optional `tracked` argument (e.g.
`remote_branches(tracked=false)` to filter to untracked remote branches).

### Fixed bugs

## [0.19.0] - 2024-07-03
Expand Down
13 changes: 9 additions & 4 deletions cli/src/commands/branch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ mod untrack;

use itertools::Itertools as _;
use jj_lib::backend::CommitId;
use jj_lib::op_store::{RefTarget, RemoteRef};
use jj_lib::op_store::{RefTarget, RemoteRef, RemoteRefState};
use jj_lib::repo::Repo;
use jj_lib::str_util::StringPattern;
use jj_lib::view::View;
Expand Down Expand Up @@ -129,7 +129,7 @@ fn find_remote_branches<'a>(
let mut unmatched_patterns = vec![];
for pattern in name_patterns {
let mut matches = view
.remote_branches_matching(&pattern.branch, &pattern.remote)
.remote_branches_matching(&pattern.branch, &pattern.remote, None)
.map(|((branch, remote), remote_ref)| {
let name = RemoteBranchName {
branch: branch.to_owned(),
Expand Down Expand Up @@ -162,8 +162,13 @@ fn find_remote_branches<'a>(
/// Whether or not the `branch` has any tracked remotes (i.e. is a tracking
/// local branch.)
fn has_tracked_remote_branches(view: &View, branch: &str) -> bool {
view.remote_branches_matching(&StringPattern::exact(branch), &StringPattern::everything())
.any(|(_, remote_ref)| remote_ref.is_tracking())
view.remote_branches_matching(
&StringPattern::exact(branch),
&StringPattern::everything(),
Some(RemoteRefState::Tracking),
)
.next()
.is_some()
}

fn is_fast_forward(repo: &dyn Repo, old_target: &RefTarget, new_target_id: &CommitId) -> bool {
Expand Down
1 change: 1 addition & 0 deletions cli/src/commands/git/push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,7 @@ fn find_branches_targeted_by_revisions<'a>(
let current_branches_expression = RevsetExpression::remote_branches(
StringPattern::everything(),
StringPattern::exact(remote_name),
None,
)
.range(&RevsetExpression::commit(wc_commit_id))
.intersection(&RevsetExpression::branches(StringPattern::everything()));
Expand Down
9 changes: 7 additions & 2 deletions docs/revsets.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ revsets (expressions) as arguments.
branches `push-123` and `repushed` but not the branch `main`. If a branch is
in a conflicted state, all its possible targets are included.

* `remote_branches([branch_pattern[, [remote=]remote_pattern]])`: All remote
branch targets across all remotes. If just the `branch_pattern` is
* `remote_branches([branch_pattern], [[remote=]remote_pattern], [[tracked=]tracked])`:
All remote branch targets across all remotes. If just the `branch_pattern` is
specified, the branches whose names match the given [string
pattern](#string-patterns) across all remotes are selected. If both
`branch_pattern` and `remote_pattern` are specified, the selection is
Expand All @@ -214,6 +214,11 @@ revsets (expressions) as arguments.
`main@origin` or `main@upstream`. If a branch is in a conflicted state, all
its possible targets are included.

Tracked remote branches can be selected with `tracked=true`, while untracked
remote branches can be selected with `tracked=false`. For example,
`remote_branches(remote=origin, tracked=false)` would select any untracked
branches from remotes matching `origin`.

While Git-tracking branches can be selected by `<name>@git`, these branches
aren't included in `remote_branches()`.

Expand Down
56 changes: 49 additions & 7 deletions lib/src/revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use crate::graph::GraphEdge;
use crate::hex_util::to_forward_hex;
use crate::id_prefix::IdPrefixContext;
use crate::object_id::{HexPrefix, PrefixResolution};
use crate::op_store::WorkspaceId;
use crate::op_store::{RemoteRefState, WorkspaceId};
use crate::repo::Repo;
use crate::repo_path::RepoPathUiConverter;
pub use crate::revset_parser::{
Expand Down Expand Up @@ -107,6 +107,7 @@ pub enum RevsetCommitRef {
RemoteBranches {
branch_pattern: StringPattern,
remote_pattern: StringPattern,
remote_ref_state: Option<RemoteRefState>,
},
Tags,
GitRefs,
Expand Down Expand Up @@ -239,11 +240,13 @@ impl RevsetExpression {
pub fn remote_branches(
branch_pattern: StringPattern,
remote_pattern: StringPattern,
remote_ref_state: Option<RemoteRefState>,
) -> Rc<RevsetExpression> {
Rc::new(RevsetExpression::CommitRef(
RevsetCommitRef::RemoteBranches {
branch_pattern,
remote_pattern,
remote_ref_state,
},
))
}
Expand Down Expand Up @@ -626,8 +629,8 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
Ok(RevsetExpression::branches(pattern))
});
map.insert("remote_branches", |function, _context| {
let ([], [branch_opt_arg, remote_opt_arg]) =
function.expect_named_arguments(&["", "remote"])?;
let ([], [branch_opt_arg, remote_opt_arg, tracked_opt_arg]) =
function.expect_named_arguments(&["", "remote", "tracked"])?;
let branch_pattern = if let Some(branch_arg) = branch_opt_arg {
expect_string_pattern(branch_arg)?
} else {
Expand All @@ -638,9 +641,19 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
} else {
StringPattern::everything()
};
let remote_ref_state = if let Some(tracked_arg) = tracked_opt_arg {
if revset_parser::expect_bool(tracked_arg)? {
Some(RemoteRefState::Tracking)
} else {
Some(RemoteRefState::New)
}
} else {
None
};
Ok(RevsetExpression::remote_branches(
branch_pattern,
remote_pattern,
remote_ref_state,
))
});
map.insert("tags", |function, _context| {
Expand Down Expand Up @@ -1603,11 +1616,12 @@ fn resolve_commit_ref(
RevsetCommitRef::RemoteBranches {
branch_pattern,
remote_pattern,
remote_ref_state,
} => {
// TODO: should we allow to select @git branches explicitly?
let commit_ids = repo
.view()
.remote_branches_matching(branch_pattern, remote_pattern)
.remote_branches_matching(branch_pattern, remote_pattern, *remote_ref_state)
.filter(|&((_, remote_name), _)| {
#[cfg(feature = "git")]
{
Expand Down Expand Up @@ -2307,14 +2321,16 @@ mod tests {
RemoteBranches {
branch_pattern: Substring(""),
remote_pattern: Substring(""),
remote_ref_state: None,
},
)
"###);
insta::assert_debug_snapshot!(parse("remote_branches()").unwrap(), @r###"
insta::assert_debug_snapshot!(parse("remote_branches(foo, bar, true)").unwrap(), @r###"
CommitRef(
RemoteBranches {
branch_pattern: Substring(""),
remote_pattern: Substring(""),
branch_pattern: Substring("foo"),
remote_pattern: Substring("bar"),
remote_ref_state: Some(Tracking),
},
)
"###);
Expand Down Expand Up @@ -2560,6 +2576,17 @@ mod tests {
RemoteBranches {
branch_pattern: Substring(""),
remote_pattern: Substring("foo"),
remote_ref_state: None,
},
)
"###);
insta::assert_debug_snapshot!(
parse("remote_branches(tracked=false, remote=foo)").unwrap(), @r###"
CommitRef(
RemoteBranches {
branch_pattern: Substring(""),
remote_pattern: Substring("foo"),
remote_ref_state: Some(New),
},
)
"###);
Expand All @@ -2569,9 +2596,24 @@ mod tests {
RemoteBranches {
branch_pattern: Substring("foo"),
remote_pattern: Substring("bar"),
remote_ref_state: None,
},
)
"###);
insta::assert_debug_snapshot!(
parse("remote_branches(foo, remote=bar, tracked=true)").unwrap(), @r###"
CommitRef(
RemoteBranches {
branch_pattern: Substring("foo"),
remote_pattern: Substring("bar"),
remote_ref_state: Some(Tracking),
},
)
"###);
insta::assert_debug_snapshot!(
parse(r#"remote_branches(tracked=foo)"#).unwrap_err().kind(),
@r###"Expression("Expected expression of type bool")"###,
);
insta::assert_debug_snapshot!(
parse(r#"remote_branches(remote=foo, bar)"#).unwrap_err().kind(),
@r###"
Expand Down
11 changes: 11 additions & 0 deletions lib/src/revset_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,17 @@ pub fn expect_literal<T: FromStr>(
})
}

pub fn expect_bool(node: &ExpressionNode) -> Result<bool, RevsetParseError> {
expect_literal_with(node, |node| match &node.kind {
ExpressionKind::Identifier("false") => Ok(false),
ExpressionKind::Identifier("true") => Ok(true),
_ => Err(RevsetParseError::expression(
"Expected expression of type bool",
node.span,
)),
})
}

/// Applies the give function to the innermost `node` by unwrapping alias
/// expansion nodes.
fn expect_literal_with<T>(
Expand Down
18 changes: 13 additions & 5 deletions lib/src/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ use std::collections::{BTreeMap, HashMap, HashSet};
use itertools::Itertools;

use crate::backend::CommitId;
use crate::op_store::{BranchTarget, RefTarget, RefTargetOptionExt as _, RemoteRef, WorkspaceId};
use crate::op_store::{
BranchTarget, RefTarget, RefTargetOptionExt as _, RemoteRef, RemoteRefState, WorkspaceId,
};
use crate::refs::LocalAndRemoteRef;
use crate::str_util::StringPattern;
use crate::{op_store, refs};
Expand Down Expand Up @@ -168,17 +170,23 @@ impl View {
&'a self,
branch_pattern: &'b StringPattern,
remote_pattern: &'b StringPattern,
remote_ref_state: Option<RemoteRefState>,
) -> impl Iterator<Item = ((&'a str, &'a str), &'a RemoteRef)> + 'b {
// Use kmerge instead of flat_map for consistency with all_remote_branches().
remote_pattern
.filter_btree_map(&self.data.remote_views)
.map(|(remote_name, remote_view)| {
branch_pattern.filter_btree_map(&remote_view.branches).map(
|(branch_name, remote_ref)| {
branch_pattern
.filter_btree_map(&remote_view.branches)
.filter(move |(_, remote_ref)| {
remote_ref_state
.map(|state| remote_ref.state == state)
.unwrap_or(true)
})
.map(|(branch_name, remote_ref)| {
let full_name = (branch_name.as_ref(), remote_name.as_ref());
(full_name, remote_ref)
},
)
})
})
.kmerge_by(|(full_name1, _), (full_name2, _)| full_name1 < full_name2)
}
Expand Down
40 changes: 37 additions & 3 deletions lib/tests/test_revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2060,7 +2060,7 @@ fn test_evaluate_expression_remote_branches() {
let repo = &test_repo.repo;
let remote_ref = |target| RemoteRef {
target,
state: RemoteRefState::Tracking, // doesn't matter
state: RemoteRefState::Tracking,
};
let normal_remote_ref = |id: &CommitId| remote_ref(RefTarget::normal(id.clone()));

Expand All @@ -2075,15 +2075,24 @@ 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("branch1", "origin", normal_remote_ref(commit1.id()));
// Branch 1 is untracked on remote origin
mut_repo.set_remote_branch(
"branch1",
"origin",
RemoteRef {
target: RefTarget::normal(commit1.id().clone()),
state: RemoteRefState::New,
},
);
// Branch 2 is tracked on remote private
mut_repo.set_remote_branch("branch2", "private", normal_remote_ref(commit2.id()));
// Git-tracking branches aren't included
mut_repo.set_remote_branch(
"branch",
git::REMOTE_NAME_FOR_LOCAL_GIT_REPO,
normal_remote_ref(commit_git_remote.id()),
);
// Can get a few branches
assert_eq!(
resolve_commit_ids(mut_repo, "remote_branches()"),
vec![commit2.id().clone(), commit1.id().clone()]
Expand Down Expand Up @@ -2127,6 +2136,23 @@ fn test_evaluate_expression_remote_branches() {
resolve_commit_ids(mut_repo, r#"remote_branches(exact:branch1, exact:origin)"#),
vec![commit1.id().clone()]
);
// Can filter branches by tracked and untracked
assert_eq!(
resolve_commit_ids(mut_repo, "remote_branches(tracked=false)"),
vec![commit1.id().clone()]
);
assert_eq!(
resolve_commit_ids(mut_repo, "remote_branches(tracked=true)"),
vec![commit2.id().clone()]
);
assert_eq!(
resolve_commit_ids(mut_repo, "remote_branches(branch1, origin, false)"),
vec![commit1.id().clone()]
);
assert_eq!(
resolve_commit_ids(mut_repo, "remote_branches(branch2, private, true)"),
vec![commit2.id().clone()]
);
// Can silently resolve to an empty set if there's no matches
assert_eq!(
resolve_commit_ids(mut_repo, "remote_branches(branch3)"),
Expand All @@ -2148,6 +2174,14 @@ fn test_evaluate_expression_remote_branches() {
resolve_commit_ids(mut_repo, r#"remote_branches(exact:branch1, exact:orig)"#),
vec![]
);
assert_eq!(
resolve_commit_ids(mut_repo, "remote_branches(branch1, tracked=true)"),
vec![]
);
assert_eq!(
resolve_commit_ids(mut_repo, "remote_branches(branch2, tracked=false)"),
vec![]
);
// Two branches pointing to the same commit does not result in a duplicate in
// the revset
mut_repo.set_remote_branch("branch3", "origin", normal_remote_ref(commit2.id()));
Expand Down

0 comments on commit 0de177b

Please sign in to comment.