Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
Git HEAD to the parent of the fresh working-copy commit.
[#9936](https://github.com/jj-vcs/jj/issues/9936)

* Fixed crash in `jj log` involving hidden revisions and the
`log-graph-prioritize` revset.
[#9975](https://github.com/jj-vcs/jj/issues/9975)

## [0.44.0] - 2026-08-05

### Release highlights
Expand Down
17 changes: 13 additions & 4 deletions cli/src/commands/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,22 @@ pub(crate) async fn cmd_log(
return Ok(());
}

let prio_revset = settings.get_string("revsets.log-graph-prioritize")?;
let mut prio_revset = workspace_command.parse_revset(ui, &RevisionArg::from(prio_revset))?;
prio_revset.intersect_with(revset_expression.expression());

let repo = workspace_command.repo();
let matcher = fileset_expression.to_matcher();

let prio_revset = {
let text = settings.get_string("revsets.log-graph-prioritize")?;
let mut prio_revset = workspace_command.parse_revset(ui, &RevisionArg::from(text))?;
// Resolve the query within a separate scope so its visibility and
// referenced commits aren't affected by the prioritize expression:
// `<prio> & at_operation(@, <query>)`
let query_expr = revset_expression
.expression()
.within_visibility(repo.as_ref());
prio_revset.intersect_with(&query_expr);
prio_revset
};

let store = repo.store();
let diff_renderer = workspace_command.diff_renderer_for_log(&args.diff_format, args.patch)?;
let graph_style = GraphStyle::from_settings(settings)?;
Expand Down
52 changes: 52 additions & 0 deletions cli/tests/test_log_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,58 @@ fn test_log_prefix_highlight_counts_hidden_commits() {
[EOF]
");
}
#[test]
fn test_log_graph_prioritize_hidden_commits() {
let test_env = TestEnvironment::default();
test_env.run_jj_in(".", ["git", "init", "repo"]).success();
let work_dir = test_env.work_dir("repo");

work_dir.run_jj(["describe", "-m1"]).success();
work_dir.run_jj(["describe", "-m2"]).success();

// Sanity check: the revision "1" should be displayed first
let output = work_dir.run_jj([
"log",
"--config=revsets.log-graph-prioritize=at_operation(@-, @)",
"-r::(@|at_operation(@-, @))",
]);
insta::assert_snapshot!(output, @"
○ qpvuntsm/1 test.user@example.com 2001-02-03 08:05:08 884fe9b9 (hidden)
│ (empty) 1
│ @ qpvuntsm test.user@example.com 2001-02-03 08:05:09 4e123bae
├─╯ (empty) 2
◆ zzzzzzzz root() 00000000
[EOF]
");

// The revision "1" shouldn't be prioritized because it isn't included in
// the query.
let output = work_dir.run_jj([
"log",
"--config=revsets.log-graph-prioritize=at_operation(@-, @ | subject(1))",
"-rall()",
]);
insta::assert_snapshot!(output, @"
@ qpvuntsm test.user@example.com 2001-02-03 08:05:09 4e123bae
│ (empty) 2
◆ zzzzzzzz root() 00000000
[EOF]
");

// The revision "2" shouldn't be prioritized because it isn't included in
// the query.
let output = work_dir.run_jj([
"log",
"--config=revsets.log-graph-prioritize=@ | subject(2)",
"-rat_operation(@-, all())",
]);
insta::assert_snapshot!(output, @"
○ qpvuntsm/1 test.user@example.com 2001-02-03 08:05:08 884fe9b9 (hidden)
│ (empty) 1
◆ zzzzzzzz root() 00000000
[EOF]
");
}

#[test]
fn test_log_author_format() {
Expand Down
65 changes: 49 additions & 16 deletions lib/src/revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,8 @@ pub enum RevsetExpression<St: ExpressionState> {
commits: Vec<CommitId>,
},
/// Resolves visibility within the specified repo state.
///
/// Commits referenced by the `candidates` expression are also scoped.
WithinVisibility {
candidates: Arc<Self>,
/// Copy of `repo.view().heads()` at the operation.
Expand Down Expand Up @@ -614,6 +616,18 @@ impl<St: ExpressionState> RevsetExpression<St> {
})
}

/// Resolves visibility within the given repo.
///
/// This is equivalent to `at_operation(repo.op_id(), self)`, except that
/// `CommitRef`s are resolved separately. Commits referenced by the `self`
/// expression are also scoped.
pub fn within_visibility(self: &Arc<Self>, repo: &dyn Repo) -> Arc<Self> {
Arc::new(Self::WithinVisibility {
candidates: self.clone(),
visible_heads: repo.view().heads().iter().cloned().collect(),
})
}

/// Suppresses name resolution error within `self`.
pub fn present(self: &Arc<Self>) -> Arc<Self> {
Arc::new(Self::Present(self.clone()))
Expand Down Expand Up @@ -1931,6 +1945,15 @@ where
/// User symbols and `at_operation()` scopes should have been resolved.
fn resolve_referenced_commits<St: ExpressionState>(
expression: &Arc<RevsetExpression<St>>,
) -> TransformedExpression<St> {
// Omit empty node to keep test/debug output concise
let omit_empty = true;
resolve_referenced_commits_sub(expression, omit_empty)
}

fn resolve_referenced_commits_sub<St: ExpressionState>(
expression: &Arc<RevsetExpression<St>>,
omit_empty: bool,
) -> TransformedExpression<St> {
// Trust precomputed value if any
if matches!(
Expand Down Expand Up @@ -1958,12 +1981,15 @@ fn resolve_referenced_commits<St: ExpressionState>(
} => {
// ::visible_heads shouldn't be filtered out by outer
inner_commits.extend_from_slice(visible_heads);
let transformed = resolve_referenced_commits(candidates);
// Empty WithinReference node shouldn't be omitted because
// at_operation() creates a new resolution scope.
let transformed = resolve_referenced_commits_sub(candidates, false);
// Referenced commits shouldn't be filtered out by outer
if let RevsetExpression::WithinReference { commits, .. } =
transformed.as_deref().unwrap_or(candidates)
{
inner_commits.extend_from_slice(commits);
match transformed.as_deref().unwrap_or(candidates) {
RevsetExpression::WithinReference { commits, .. } => {
inner_commits.extend_from_slice(commits);
}
_ => unreachable!("WithinReference should never be omitted"),
}
ControlFlow::Break(transformed.map(|candidates| {
Arc::new(RevsetExpression::WithinVisibility {
Expand All @@ -1985,8 +2011,7 @@ fn resolve_referenced_commits<St: ExpressionState>(
// Commits could be deduplicated here, but they'll be concatenated with
// the visible heads later, which may have duplicates.
outer_commits.extend(inner_commits);
if outer_commits.is_empty() {
// Omit empty node to keep test/debug output concise
if omit_empty && outer_commits.is_empty() {
return transformed;
}
Some(Arc::new(RevsetExpression::WithinReference {
Expand Down Expand Up @@ -3079,12 +3104,9 @@ impl ExpressionStateFolder<UserExpressionState, ResolvedExpressionState>
let repo = reload_repo_at_operation(self.repo(), operation)?;
self.repo_stack.push(repo);
let candidates = self.fold_expression(candidates)?;
let visible_heads = self.repo().view().heads().iter().cloned().collect();
let expression = candidates.within_visibility(self.repo());
self.repo_stack.pop();
Ok(Arc::new(RevsetExpression::WithinVisibility {
candidates,
visible_heads,
}))
Ok(expression)
}
}

Expand Down Expand Up @@ -4593,7 +4615,9 @@ mod tests {
)
"#);

// Inner scope has no references, so WithinReference should be omitted.
// Inner scope has no references, but WithinReference isn't omitted
// because commits referenced by sibling expressions shouldn't be
// visible to the inner expression.
insta::assert_debug_snapshot!(
resolve_referenced_commits(
&visibility2
Expand All @@ -4605,7 +4629,10 @@ mod tests {
candidates: Union(
Intersection(
WithinVisibility {
candidates: Filter(HasConflict),
candidates: WithinReference {
candidates: Filter(HasConflict),
commits: [],
},
visible_heads: [
CommitId("200000"),
],
Expand Down Expand Up @@ -4655,7 +4682,10 @@ mod tests {
],
},
WithinVisibility {
candidates: Filter(HasConflict),
candidates: WithinReference {
candidates: Filter(HasConflict),
commits: [],
},
visible_heads: [
CommitId("200000"),
],
Expand Down Expand Up @@ -4899,7 +4929,10 @@ mod tests {
})), @r#"
WithinReference {
candidates: WithinVisibility {
candidates: CommitRef(Bookmarks(Pattern(Substring("")))),
candidates: WithinReference {
candidates: CommitRef(Bookmarks(Pattern(Substring("")))),
commits: [],
},
visible_heads: [
CommitId("012345"),
],
Expand Down
31 changes: 31 additions & 0 deletions lib/tests/test_revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4239,6 +4239,37 @@ fn test_evaluate_expression_at_operation() -> TestResult {
]
);

// Visibility and referenced commits resolution between sub expressions:
// each at_operation() node should create its own scope.
Comment thread
josephlou5 marked this conversation as resolved.
assert_eq!(
resolve_commit_ids(
repo2.as_ref(),
"at_operation(@, all()) & at_operation(@-, all())"
),
vec![commit2_op1.id().clone(), root_commit.id().clone()]
);
assert_eq!(
resolve_commit_ids(
repo2.as_ref(),
"at_operation(@, commit1_ref) & at_operation(@-, commit1_ref)"
),
vec![]
);
assert_eq!(
resolve_commit_ids(
repo2.as_ref(),
"at_operation(@, commit1_ref) & at_operation(@-, all())"
),
vec![]
);
assert_eq!(
resolve_commit_ids(
repo2.as_ref(),
"at_operation(@, all()) & at_operation(@-, commit1_ref)"
),
vec![]
);

// Operation is resolved relative to the outer ReadonlyRepo.
assert_eq!(
resolve_commit_ids(repo2.as_ref(), "at_operation(@-, at_operation(@-, all()))"),
Expand Down
Loading