Skip to content

Commit

Permalink
rewrite: add reset-author-timestamp revset
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
scott2000 committed Jun 19, 2024
1 parent 19c47b8 commit 09b97f9
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 2 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
58 changes: 57 additions & 1 deletion cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1157,8 +1157,47 @@ impl WorkspaceCommandHelper {
self.check_repo_rewritable(self.repo().as_ref(), commits)
}

fn reset_author_timestamp_revset(&self) -> Result<Rc<RevsetExpression>, 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<bool, CommandError> {
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<bool, CommandError> {
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<Option<_>, _> {
repo.view()
Expand Down Expand Up @@ -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())?;
Expand Down Expand Up @@ -1628,8 +1677,15 @@ impl WorkspaceCommandTransaction<'_> {
}

pub fn rewrite_commit(&mut self, commit: &Commit) -> Result<CommitBuilder, CommandError> {
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 {
Expand Down
5 changes: 5 additions & 0 deletions cli/src/config-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,11 @@
"type": "string",
"description": "Revisions to give shorter change and commit IDs to",
"default": "<revsets.log>"
},
"reset-author-timestamp": {
"type": "string",
"description": "Revisions which should have their author timestamp reset when they are rewritten",
"default": "description(exact:\"\") & empty()"
}
},
"additionalProperties": {
Expand Down
2 changes: 2 additions & 0 deletions cli/src/config/revsets.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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()' = '''
Expand Down
20 changes: 20 additions & 0 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions lib/src/commit_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 3 additions & 1 deletion lib/src/id_prefix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@ use crate::revset::{

struct PrefixDisambiguationError;

#[derive(Clone)]
struct DisambiguationData {
expression: Rc<RevsetExpression>,
indexes: OnceCell<Indexes>,
}

#[derive(Clone)]
struct Indexes {
commit_change_ids: Vec<(CommitId, ChangeId)>,
commit_index: IdIndex<CommitId, u32, 4>,
Expand Down Expand Up @@ -99,7 +101,7 @@ impl IdIndexSourceEntry<ChangeId> for &'_ (CommitId, ChangeId) {
}
}

#[derive(Default)]
#[derive(Clone, Default)]
pub struct IdPrefixContext {
disambiguation: Option<DisambiguationData>,
extensions: Arc<RevsetExtensions>,
Expand Down

0 comments on commit 09b97f9

Please sign in to comment.