commit_builder: reset author timestamp on discardable commits

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 changes the behavior so that discardable commits (empty commits with no description) by the current user have their author timestamps reset when they are rewritten, meaning that the author timestamp will become finalized whenever a commit is given a description or becomes non-empty.
jj-vcs · Jun 29, 2024 · 1eebbe5 · 1eebbe5
1 parent e21e5e6
commit 1eebbe5
Show file tree

Hide file tree

Showing 36 changed files with 1,330 additions and 1,236 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -72,6 +72,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * New command `jj git remote set-url` that sets the url of a git remote.
 
+* Author timestamp is now reset when rewriting discardable commits (empty
+  commits with no description) if authored by the current user.
+  [#2000](https://github.com/martinvonz/jj/issues/2000)
+
 ### Fixed bugs
 
 * `jj git push` now ignores immutable commits when checking whether a

diff --git a/cli/tests/test_branch_command.rs b/cli/tests/test_branch_command.rs
@@ -165,7 +165,7 @@ fn test_branch_move() {
     test_env.jj_cmd_ok(&repo_path, &["branch", "delete", "foo"]);
     insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
     foo (deleted)
-      @origin: qpvuntsm 29a62310 (empty) commit
+      @origin: qpvuntsm 1eb845f3 (empty) commit
     "###);
 
     // Deleted tracking branch name should still be allocated
@@ -179,8 +179,8 @@ fn test_branch_move() {
     let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "set", "foo"]);
     insta::assert_snapshot!(stderr, @"");
     insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
-    foo: mzvwutvl d5f17aba (empty) (no description set)
-      @origin (behind by 1 commits): qpvuntsm 29a62310 (empty) commit
+    foo: mzvwutvl 66d48752 (empty) (no description set)
+      @origin (behind by 1 commits): qpvuntsm 1eb845f3 (empty) commit
     "###);
 
     // Untracked remote branch shouldn't block creation of local branch
@@ -189,8 +189,8 @@ fn test_branch_move() {
     let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "create", "foo"]);
     insta::assert_snapshot!(stderr, @"");
     insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
-    foo: mzvwutvl d5f17aba (empty) (no description set)
-    foo@origin: qpvuntsm 29a62310 (empty) commit
+    foo: mzvwutvl 66d48752 (empty) (no description set)
+    foo@origin: qpvuntsm 1eb845f3 (empty) commit
     "###);
 }
 
@@ -496,7 +496,7 @@ fn test_branch_delete_glob() {
     test_env.jj_cmd_ok(&repo_path, &["git", "push", "--all"]);
 
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @  bar-2 foo-1 foo-3 foo-4 6fbf398c2d59
+    @  bar-2 foo-1 foo-3 foo-4 312a98d6f27b
     ◉   000000000000
     "###);
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "delete", "glob:foo-[1-3]"]);
@@ -511,7 +511,7 @@ fn test_branch_delete_glob() {
     Deleted 2 branches.
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @  bar-2 foo-1@origin foo-3@origin foo-4 6fbf398c2d59
+    @  bar-2 foo-1@origin foo-3@origin foo-4 312a98d6f27b
     ◉   000000000000
     "###);
 
@@ -531,20 +531,20 @@ fn test_branch_delete_glob() {
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @"");
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @  bar-2 foo-1@origin foo-3@origin foo-4@origin 6fbf398c2d59
+    @  bar-2 foo-1@origin foo-3@origin foo-4@origin 312a98d6f27b
     ◉   000000000000
     "###);
 
     // The deleted branches are still there
     insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
-    bar-2: qpvuntsm 6fbf398c (empty) commit
-      @origin: qpvuntsm 6fbf398c (empty) commit
+    bar-2: qpvuntsm 312a98d6 (empty) commit
+      @origin: qpvuntsm 312a98d6 (empty) commit
     foo-1 (deleted)
-      @origin: qpvuntsm 6fbf398c (empty) commit
+      @origin: qpvuntsm 312a98d6 (empty) commit
     foo-3 (deleted)
-      @origin: qpvuntsm 6fbf398c (empty) commit
+      @origin: qpvuntsm 312a98d6 (empty) commit
     foo-4 (deleted)
-      @origin: qpvuntsm 6fbf398c (empty) commit
+      @origin: qpvuntsm 312a98d6 (empty) commit
     "###);
 
     // Malformed glob
@@ -1014,11 +1014,11 @@ fn test_branch_track_conflict() {
     );
     let (_, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "track", "main@origin"]);
     insta::assert_snapshot!(stderr, @r###"
-main (conflicted):
-  + qpvuntsm b4a6b8c5 (empty) b
-  + qpvuntsm hidden 4bfd80cd (empty) a
-  @origin (behind by 1 commits): qpvuntsm hidden 4bfd80cd (empty) a
-"###);
+    main (conflicted):
+      + qpvuntsm e802c4f8 (empty) b
+      + qpvuntsm hidden 427890ea (empty) a
+      @origin (behind by 1 commits): qpvuntsm hidden 427890ea (empty) a
+    "###);
 }
 
 #[test]

diff --git a/cli/tests/test_builtin_aliases.rs b/cli/tests/test_builtin_aliases.rs
@@ -52,7 +52,7 @@ fn test_builtin_alias_trunk_matches_main() {
 
     let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-r", "trunk()"]);
     insta::assert_snapshot!(stdout, @r###"
-    ◉  lzmmnrxq [email protected] 2001-02-03 08:05:08 main 45a3aa29
+    ◉  xtvrqkyv [email protected] 2001-02-03 08:05:08 main d13ecdbd
     │  (empty) description 1
     ~
     "###);
@@ -64,7 +64,7 @@ fn test_builtin_alias_trunk_matches_master() {
 
     let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-r", "trunk()"]);
     insta::assert_snapshot!(stdout, @r###"
-    ◉  lzmmnrxq [email protected] 2001-02-03 08:05:08 master 45a3aa29
+    ◉  xtvrqkyv [email protected] 2001-02-03 08:05:08 master d13ecdbd
     │  (empty) description 1
     ~
     "###);
@@ -76,7 +76,7 @@ fn test_builtin_alias_trunk_matches_trunk() {
 
     let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-r", "trunk()"]);
     insta::assert_snapshot!(stdout, @r###"
-    ◉  lzmmnrxq [email protected] 2001-02-03 08:05:08 trunk 45a3aa29
+    ◉  xtvrqkyv [email protected] 2001-02-03 08:05:08 trunk d13ecdbd
     │  (empty) description 1
     ~
     "###);
@@ -91,7 +91,7 @@ fn test_builtin_alias_trunk_matches_exactly_one_commit() {
 
     let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-r", "trunk()"]);
     insta::assert_snapshot!(stdout, @r###"
-    ◉  lzmmnrxq [email protected] 2001-02-03 08:05:08 main 45a3aa29
+    ◉  xtvrqkyv [email protected] 2001-02-03 08:05:08 main d13ecdbd
     │  (empty) description 1
     ~
     "###);
@@ -107,7 +107,7 @@ fn test_builtin_alias_trunk_override_alias() {
 
     let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-r", "trunk()"]);
     insta::assert_snapshot!(stdout, @r###"
-    ◉  lzmmnrxq [email protected] 2001-02-03 08:05:08 override-trunk 45a3aa29
+    ◉  xtvrqkyv [email protected] 2001-02-03 08:05:08 override-trunk d13ecdbd
     │  (empty) description 1
     ~
     "###);

diff --git a/cli/tests/test_checkout.rs b/cli/tests/test_checkout.rs
@@ -31,23 +31,23 @@ fn test_checkout() {
     insta::assert_snapshot!(stderr, @r###"
     Warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent
     Warning: `jj checkout` will be removed in a future version, and this will be a hard error
-    Working copy now at: zsuskuln 05ce7118 (empty) (no description set)
-    Parent commit      : rlvkpnrz 5c52832c (empty) second
+    Working copy now at: zsuskuln c97da310 (empty) (no description set)
+    Parent commit      : rlvkpnrz 9ed53a4a (empty) second
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @  05ce7118568d3007efc9163b055f9cb4a6becfde
-    ◉  5c52832c3483e0ace06d047a806024984f28f1d7 second
-    ◉  69542c1984c1f9d91f7c6c9c9e6941782c944bd9 first
+    @  c97da310c66008034013412d321397242e1e43ef
+    ◉  9ed53a4a1becd028f9a2fe0d5275973acea7e8da second
+    ◉  fa15625b4a986997697639dfc2844138900c79f2 first
     ◉  0000000000000000000000000000000000000000
     "###);
 
     // Can provide a description
     test_env.jj_cmd_ok(&repo_path, &["checkout", "@--", "-m", "my message"]);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @  1191baaf276e3d0b96b1747e885b3a517be80d6f my message
-    │ ◉  5c52832c3483e0ace06d047a806024984f28f1d7 second
+    @  6f9c4a002224fde4ebc48ce6ec03d5ffcfa64ad2 my message
+    │ ◉  9ed53a4a1becd028f9a2fe0d5275973acea7e8da second
     ├─╯
-    ◉  69542c1984c1f9d91f7c6c9c9e6941782c944bd9 first
+    ◉  fa15625b4a986997697639dfc2844138900c79f2 first
     ◉  0000000000000000000000000000000000000000
     "###);
 }
@@ -70,11 +70,11 @@ fn test_checkout_not_single_rev() {
     Warning: `jj checkout` will be removed in a future version, and this will be a hard error
     Error: Revset "root()..@" resolved to more than one revision
     Hint: The revset "root()..@" resolved to these revisions:
-      royxmykx 2f859371 (empty) (no description set)
-      mzvwutvl 5c1afd8b (empty) fifth
-      zsuskuln 009f88bf (empty) fourth
-      kkmpptxz 3fa8931e (empty) third
-      rlvkpnrz 5c52832c (empty) second
+      royxmykx 554d2245 (empty) (no description set)
+      mzvwutvl a497e2bf (empty) fifth
+      zsuskuln 9d7e5e99 (empty) fourth
+      kkmpptxz 30056b0c (empty) third
+      rlvkpnrz 9ed53a4a (empty) second
       ...
     "###);
 
@@ -84,11 +84,11 @@ fn test_checkout_not_single_rev() {
     Warning: `jj checkout` will be removed in a future version, and this will be a hard error
     Error: Revset "root()..@-" resolved to more than one revision
     Hint: The revset "root()..@-" resolved to these revisions:
-      mzvwutvl 5c1afd8b (empty) fifth
-      zsuskuln 009f88bf (empty) fourth
-      kkmpptxz 3fa8931e (empty) third
-      rlvkpnrz 5c52832c (empty) second
-      qpvuntsm 69542c19 (empty) first
+      mzvwutvl a497e2bf (empty) fifth
+      zsuskuln 9d7e5e99 (empty) fourth
+      kkmpptxz 30056b0c (empty) third
+      rlvkpnrz 9ed53a4a (empty) second
+      qpvuntsm fa15625b (empty) first
     "###);
 
     let stderr = test_env.jj_cmd_failure(&repo_path, &["checkout", "@-|@--"]);
@@ -97,8 +97,8 @@ fn test_checkout_not_single_rev() {
     Warning: `jj checkout` will be removed in a future version, and this will be a hard error
     Error: Revset "@-|@--" resolved to more than one revision
     Hint: The revset "@-|@--" resolved to these revisions:
-      mzvwutvl 5c1afd8b (empty) fifth
-      zsuskuln 009f88bf (empty) fourth
+      mzvwutvl a497e2bf (empty) fifth
+      zsuskuln 9d7e5e99 (empty) fourth
     "###);
 
     let stderr = test_env.jj_cmd_failure(&repo_path, &["checkout", "none()"]);

diff --git a/cli/tests/test_commit_command.rs b/cli/tests/test_commit_command.rs
@@ -25,8 +25,8 @@ fn test_commit_with_description_from_cli() {
     // Description applies to the current working-copy (not the new one)
     test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=first"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  b88fb4e51bdd
-    ◉  69542c1984c1 first
+    @  e8ea92a8b6b3
+    ◉  fa15625b4a98 first
     ◉  000000000000
     "###);
 }
@@ -44,8 +44,8 @@ fn test_commit_with_editor() {
     std::fs::write(&edit_script, ["dump editor0", "write\nmodified"].join("\0")).unwrap();
     test_env.jj_cmd_ok(&workspace_path, &["commit"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  3df78bc2b9b5
-    ◉  30a8c2b3d6eb modified
+    @  a57b2c95fb75
+    ◉  159271101e05 modified
     ◉  000000000000
     "###);
     insta::assert_snapshot!(
@@ -136,11 +136,11 @@ fn test_commit_with_default_description() {
     std::fs::write(edit_script, ["dump editor"].join("\0")).unwrap();
     test_env.jj_cmd_ok(&workspace_path, &["commit"]);
 
-    insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r#"
-    @  8dc0591d00f7
-    ◉  7e780ba80aeb TESTED=TODO
+    insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
+    @  c65242099289
+    ◉  573b6df51aea TESTED=TODO
     ◉  000000000000
-    "#);
+    "###);
     assert_eq!(
         std::fs::read_to_string(test_env.env_root().join("editor")).unwrap(),
         r#"
@@ -203,8 +203,8 @@ fn test_commit_paths_warning() {
     let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=first", "file3"]);
     insta::assert_snapshot!(stderr, @r###"
     Warning: The given paths do not match any file: file3
-    Working copy now at: rlvkpnrz 67872820 (no description set)
-    Parent commit      : qpvuntsm 69542c19 (empty) first
+    Working copy now at: rlvkpnrz d1872100 (no description set)
+    Parent commit      : qpvuntsm fa15625b (empty) first
     "###);
     insta::assert_snapshot!(stdout, @"");