Skip to content

Commit

Permalink
cli: change default log revset to not include all tagged heads
Browse files Browse the repository at this point in the history
The default immutable_heads() includes tags(), which makes sense, but computing
heads(tags()) can be expensive because the tags() set is usually sparse. For
example, "jj bench revset 'heads(tags())'" took 157ms in my linux stable
mirror. We can of course optimize the heads evaluation by using bit set or
segmented index, but the query includes many historical heads if the repository
has per-release branches, which are uninteresting anyway. So, this patch
replaces heads(immutable_heads()) with trunk().

The reason we include heads(immutable_heads()) is to mitigate the following
problem. Suppose trunk() is the branch to be based off, I think using trunk()
here is pretty good.

```
A   B
*---*----* trunk() ⊆ immutable_heads()
     \
      * C
```
#2247 (comment)
  • Loading branch information
yuja committed Feb 20, 2024
1 parent 106483a commit 5b7ecb8
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 9 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Dropped support for the "legacy" graph-drawing style. Use "ascii" for a very
similar result.

* The default log output no longer lists all tagged heads. Set `revsets.log =
"@ | ancestors(immutable_heads().., 2) | heads(immutable_heads())"` to restore
the old behavior.

* Dropped support for the deprecated `:` revset operator. Use `::` instead.

### New features
Expand Down
3 changes: 1 addition & 2 deletions cli/src/commands/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ use crate::ui::Ui;
#[derive(clap::Args, Clone, Debug)]
pub(crate) struct LogArgs {
/// Which revisions to show. Defaults to the `revsets.log` setting, or
/// `@ | ancestors(immutable_heads().., 2) | heads(immutable_heads())` if
/// it is not set.
/// `@ | ancestors(immutable_heads().., 2) | trunk()` if it is not set.
#[arg(long, short)]
revisions: Vec<RevisionArg>,
/// Show commits modifying the given paths
Expand Down
2 changes: 1 addition & 1 deletion cli/src/config-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@
"log": {
"type": "string",
"description": "Default set of revisions to show when no explicit revset is given for jj log and similar commands",
"default": "@ | ancestors(immutable_heads().., 2) | heads(immutable_heads())"
"default": "@ | ancestors(immutable_heads().., 2) | trunk()"
},
"short-prefixes": {
"type": "string",
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -1002,7 +1002,7 @@ Show commit history
###### **Options:**
* `-r`, `--revisions <REVISIONS>` — Which revisions to show. Defaults to the `revsets.log` setting, or `@ | ancestors(immutable_heads().., 2) | heads(immutable_heads())` if it is not set
* `-r`, `--revisions <REVISIONS>` — Which revisions to show. Defaults to the `revsets.log` setting, or `@ | ancestors(immutable_heads().., 2) | trunk()` if it is not set
* `--reversed` — Show revisions in the opposite order (older revisions first)
Possible values: `true`, `false`
Expand Down
5 changes: 3 additions & 2 deletions cli/tests/test_immutable_commits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ fn test_rewrite_immutable_generic() {
insta::assert_snapshot!(stderr, @r###"
Config error: Invalid `revsets.short-prefixes`: --> 1:31
|
1 | @ | ancestors(immutable_heads().., 2) | heads(immutable_heads())
1 | @ | ancestors(immutable_heads().., 2) | trunk()
| ^
|
= Invalid arguments to revset function "immutable_heads": Expected 1 arguments
Expand Down Expand Up @@ -93,8 +93,9 @@ fn test_rewrite_immutable_commands() {
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "main"]);
test_env.jj_cmd_ok(&repo_path, &["new", "description(b)"]);
test_env.add_config(r#"revset-aliases."immutable_heads()" = "main""#);
test_env.add_config(r#"revset-aliases."trunk()" = "main""#);

// Log shows mutable commits and immutable heads by default
// Log shows mutable commits, their parents, and trunk() by default
let stdout = test_env.jj_cmd_success(&repo_path, &["log"]);
insta::assert_snapshot!(stdout, @r###"
@ yqosqzyt [email protected] 2001-02-03 04:05:13.000 +07:00 3f89addf
Expand Down
4 changes: 1 addition & 3 deletions lib/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,7 @@ impl UserSettings {
// For compatibility with old config files (<0.8.0)
self.config
.get_string("ui.default-revset")
.unwrap_or_else(|_| {
"@ | ancestors(immutable_heads().., 2) | heads(immutable_heads())".to_string()
})
.unwrap_or_else(|_| "@ | ancestors(immutable_heads().., 2) | trunk()".to_string())
})
}

Expand Down

0 comments on commit 5b7ecb8

Please sign in to comment.