From 09b97f969c566059a648bc02f1909906caa0b925 Mon Sep 17 00:00:00 2001 From: Scott Taylor Date: Fri, 14 Jun 2024 21:10:59 -0500 Subject: [PATCH] rewrite: add `reset-author-timestamp` revset It's common to create empty working-copy commits while using jj, and currently the author timestamp for a commit is only set when it is first created. If you create an empty commit, then don't work on a repo for a few days, and then start working on a new feature without abandoning the working-copy commit, the author timestamp will remain as the time the commit was created rather than being updated to the time that work began or finished. This commit adds a new revset `reset-author-timestamp` which specifies which commits should have their author timestamp updated whenever changes are made to them. This can be configured to fit many different workflows. By default, empty commits with no description have their author timestamps reset, meaning that the author timestamp will become finalized whenever a commit is given a description or becomes non-empty. Rebasing commits never updates the author timestamp, and commits by a different author are also never updated. --- CHANGELOG.md | 3 ++ cli/src/cli_util.rs | 58 ++++++++++++++++++++++++++++++++++++- cli/src/config-schema.json | 5 ++++ cli/src/config/revsets.toml | 2 ++ docs/config.md | 20 +++++++++++++ lib/src/commit_builder.rs | 10 +++++++ lib/src/id_prefix.rs | 4 ++- 7 files changed, 100 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26ce0775aa..033998f8ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * New command `jj branch move` let you update branches by name pattern or source revision. +* `revsets.reset-author-timestamp` can be used to configure which author + timestamps should be reset when rewriting commits. + ### Fixed bugs ## [0.18.0] - 2024-06-05 diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 8434337d4f..462b13ff44 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1157,8 +1157,47 @@ impl WorkspaceCommandHelper { self.check_repo_rewritable(self.repo().as_ref(), commits) } + fn reset_author_timestamp_revset(&self) -> Result, CommandError> { + let reset_author_timestamp = self + .settings + .config() + .get_string("revsets.reset-author-timestamp") + .unwrap_or_default(); + revset::parse(&reset_author_timestamp, &self.revset_parse_context()) + .map_err(|e| config_error_with_message("Invalid `revsets.reset-author-timestamp`", e)) + } + + fn evaluate_reset_author_timestamp( + mut expression: RevsetExpressionEvaluator, + commit_id: &CommitId, + ) -> Result { + expression.intersect_with(&RevsetExpression::commit(commit_id.clone())); + Ok(expression + .evaluate_to_commit_ids() + .map_err(|e| config_error_with_message("Invalid `revsets.reset-author-timestamp`", e))? + .next() + .is_some()) + } + + pub fn check_reset_author_timestamp( + &self, + repo: &dyn Repo, + commit_id: &CommitId, + ) -> Result { + let expression = RevsetExpressionEvaluator::new( + repo, + self.revset_extensions.clone(), + self.id_prefix_context()?, + self.reset_author_timestamp_revset()?, + ); + Self::evaluate_reset_author_timestamp(expression, commit_id) + } + #[instrument(skip_all)] fn snapshot_working_copy(&mut self, ui: &mut Ui) -> Result<(), CommandError> { + let reset_author_timestamp_revset = self.reset_author_timestamp_revset()?; + let id_prefix_context = self.id_prefix_context()?.clone(); + let workspace_id = self.workspace_id().to_owned(); let get_wc_commit = |repo: &ReadonlyRepo| -> Result, _> { repo.view() @@ -1229,12 +1268,22 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin })?; drop(progress); if new_tree_id != *wc_commit.tree_id() { + let reset_author_timestamp = Self::evaluate_reset_author_timestamp( + RevsetExpressionEvaluator::new( + self.user_repo.repo.as_ref(), + self.revset_extensions.clone(), + &id_prefix_context, + reset_author_timestamp_revset, + ), + wc_commit.id(), + )?; let mut tx = start_repo_transaction(&self.user_repo.repo, &self.settings, &self.string_args); tx.set_is_snapshot(true); let mut_repo = tx.mut_repo(); let commit = mut_repo .rewrite_commit(&self.settings, &wc_commit) + .reset_author_timestamp(reset_author_timestamp) .set_tree_id(new_tree_id) .write()?; mut_repo.set_wc_commit(workspace_id, commit.id().clone())?; @@ -1628,8 +1677,15 @@ impl WorkspaceCommandTransaction<'_> { } pub fn rewrite_commit(&mut self, commit: &Commit) -> Result { + let reset_author_timestamp = self + .helper + .check_reset_author_timestamp(self.tx.mut_repo(), commit.id())?; let settings = &self.helper.settings; - Ok(self.tx.mut_repo().rewrite_commit(settings, commit)) + Ok(self + .tx + .mut_repo() + .rewrite_commit(settings, commit) + .reset_author_timestamp(reset_author_timestamp)) } pub fn format_commit_summary(&self, commit: &Commit) -> String { diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index 5fb08e129f..12d7b8169e 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -370,6 +370,11 @@ "type": "string", "description": "Revisions to give shorter change and commit IDs to", "default": "" + }, + "reset-author-timestamp": { + "type": "string", + "description": "Revisions which should have their author timestamp reset when they are rewritten", + "default": "description(exact:\"\") & empty()" } }, "additionalProperties": { diff --git a/cli/src/config/revsets.toml b/cli/src/config/revsets.toml index d84a745b3e..2205374587 100644 --- a/cli/src/config/revsets.toml +++ b/cli/src/config/revsets.toml @@ -4,6 +4,8 @@ [revsets] fix = "reachable(@, mutable())" log = "@ | ancestors(immutable_heads().., 2) | trunk()" +reset-author-timestamp = "none()" # To make tests pass in this commit +# reset-author-timestamp = 'description(exact:"") & empty()' [revset-aliases] 'trunk()' = ''' diff --git a/docs/config.md b/docs/config.md index d75a5de949..43f104aba6 100644 --- a/docs/config.md +++ b/docs/config.md @@ -214,6 +214,26 @@ revset-aliases."immutable_heads()" = "main@origin | (main@origin.. & ~mine())" Ancestors of the configured set are also immutable. The root commit is always immutable even if the set is empty. +### Author timestamps + +Since `jj` often creates working-copy commits automatically, it is usually +undesirable to have the author timestamp be set at the time a commit is created. +To resolve this problem, by default empty commits with no description will have +their author timestamp reset whenever they are rewritten. This can be configured +using `revsets.reset-author-timestamp`, which has a default value of +`description(exact:"") & empty()`. For example, to reset the author timestamp +when rewriting any commit: + +```toml +# Reset the author timestamp when rewriting any commit +revsets.reset-author-timestamp = "all()" +``` + +Rebasing commits never updates the author timestamp, and commits by a different +author are also never updated. A more Git-like behavior might be to finalize a +commit's author timestamp when a description is set (e.g. with `jj commit`), +which could be achieved using `description(exact:"")`. + ## Log ### Default revisions diff --git a/lib/src/commit_builder.rs b/lib/src/commit_builder.rs index cef5b9789b..13f579e008 100644 --- a/lib/src/commit_builder.rs +++ b/lib/src/commit_builder.rs @@ -94,6 +94,16 @@ impl CommitBuilder<'_> { } } + pub fn reset_author_timestamp(mut self, should_reset: bool) -> Self { + if should_reset + && self.commit.author.name == self.commit.committer.name + && self.commit.author.email == self.commit.committer.email + { + self.commit.author.timestamp = self.commit.committer.timestamp.clone(); + } + self + } + pub fn parents(&self) -> &[CommitId] { &self.commit.parents } diff --git a/lib/src/id_prefix.rs b/lib/src/id_prefix.rs index f3a47e706d..8f6acea413 100644 --- a/lib/src/id_prefix.rs +++ b/lib/src/id_prefix.rs @@ -32,11 +32,13 @@ use crate::revset::{ struct PrefixDisambiguationError; +#[derive(Clone)] struct DisambiguationData { expression: Rc, indexes: OnceCell, } +#[derive(Clone)] struct Indexes { commit_change_ids: Vec<(CommitId, ChangeId)>, commit_index: IdIndex, @@ -99,7 +101,7 @@ impl IdIndexSourceEntry for &'_ (CommitId, ChangeId) { } } -#[derive(Default)] +#[derive(Clone, Default)] pub struct IdPrefixContext { disambiguation: Option, extensions: Arc,