Incorrect revsets involving hidden commits and negation

[DavidDTA]

Description

Hidden revisions are not always included in a revset when they are explicitly mentioned by commit id.

Steps to Reproduce the Problem

Here are some commands that demonstrate the issue

# The hidden commit is logged as normal
jj log --no-graph --template builtin_log_oneline --revisions 'e59f122f'
zksmnskl hidden dta 2 hours ago e59f122f (empty) <redacted description>
# The hidden commit is not logged when unioned a revset containing no revisions
jj log --no-graph --template builtin_log_oneline --revisions '(none() ~ empty()) | e59f122f'
# The hidden commit is still not logged when the negation does not match it
jj log --no-graph --template builtin_log_oneline --revisions '(none() ~ description(exact:"does not match actual description")) | e59f122f'
# Not all revsets cause this bug in conjunction with negation
jj log --no-graph --template builtin_log_oneline --revisions '(none() ~ description(exact:"does not match actual description")) | e59f122f'
jj log --no-graph --template builtin_log_oneline --revisions '(none() ~ all()) | e59f122f'
zksmnskl hidden dta 2 hours ago e59f122f (empty) <redacted description>

Expected Behavior

Hidden revisions are always included in a revset when they are unioned with another revset

Actual Behavior

This is not always the case

Specifications

• Platform: Mac OSX
• Version: 0.26.0

[martinvonz]

There is some known weirdness related to hidden commits, especially when it comes to optimizations. I'm not sure if what you're seeing is a result of an optimization or not. You can pass the queries to jj debug revset to try to understand it better.

[DavidDTA]

Without being an expert on the syntax returned by that or on the optimizations, it looks like the problem might be in the "Backend" phase, where the entire union is wrapped in a FilterWithin that probably only has the visible heads as candidates.

[yuja]

Yes, ~empty() | hash is transformed to filter(all(), |c| !c.is_empty() | c == hash) (because empty() doesn't produce commits.)

To fix this, the all() set will have to be expanded to include hash. (Maybe union could be special-cased in this example, but I don't think it will work generally.)

[DavidDTA]

As a workaround, I've found that x..y & y is equivalent to y ~ x and appears to behave correctly