Hi there,

I've noticed that got rebase, and histedit by extension, overwrite the
author timestamp with the current (committer) time.

Since this was itching me, I scratched it. The patch below is my initial
work to gauge if I'm on the right track here. It avoids clobbering the
author timestamp in lib/worktree.c:commit_worktree() by threading it
through as a separate argument.

In addition, it adds support for showing a (differing) author timestamp
to got log and tog.

You can also see this split into some individual commits at:

 https://gotweb.khjk.org/?action=summary&headref=authortime&path=got.git

A word on my commit_worktree() changes: I opted to keep the creation of
the committer timestamp where it was, right before the call to
got_object_commit_create(). Then I detect whether a different author
timestamp should be applied by the presence of the optional _committer_
argument. This works fine, but seems unintuitive. The direct alternative
would have been to change the interface of the function to make author
the optional argument.

Feedback welcome. Also, thanks a lot for got. :)

-pesco

PS: I have not checked thoroughly whether there are other places where
author time should be preserved (cherrypick?). I have also not touched
cvg or added any tests at this point.


diff refs/heads/main refs/heads/authortime
commit - 4b4fd6aa67065da8141a7a08bda84b95bc6ae060
commit + bfbb258b77efcda05d51b22c11c23c2b94c3cf05
blob - eedba272792e095dfe5a62504a47e88c579c69c8
blob + 717a7c74d145e56db2a81a960f1739187059af09
--- got/got.c
+++ got/got.c
@@ -4117,7 +4117,7 @@ done:
 }
 
 static char *
-get_datestr(time_t *time, char *datebuf)
+get_datestr(const time_t *time, char *datebuf)
 {
 	struct tm mytm, *tm;
 	char *p, *s;
@@ -4405,6 +4405,18 @@ printfile(FILE *f)
 	return NULL;
 }
 
+/* print a date line unless an error occurs (or fail silently) */
+static void
+print_date(const char *prefix, const time_t *t)
+{
+	char datebuf[26];
+	char *datestr;
+
+	datestr = get_datestr(t, datebuf);
+	if (datestr)
+		printf("%s %s UTC\n", prefix, datestr);
+}
+
 static const struct got_error *
 print_commit(struct got_commit_object *commit, struct got_object_id *id,
     struct got_repository *repo, const char *path,
@@ -4415,9 +4427,8 @@ print_commit(struct got_commit_object *commit, struct 
 {
 	const struct got_error *err = NULL;
 	FILE *f = NULL;
-	char *id_str, *datestr, *logmsg0, *logmsg, *line;
-	char datebuf[26];
-	time_t committer_time;
+	char *id_str, *logmsg0, *logmsg, *line;
+	time_t author_time, committer_time;
 	const char *author, *committer;
 	char *refs_str = NULL;
 
@@ -4447,15 +4458,20 @@ print_commit(struct got_commit_object *commit, struct 
 	id_str = NULL;
 	free(refs_str);
 	refs_str = NULL;
-	printf("from: %s\n", got_object_commit_get_author(commit));
+
+	/* author and committer data */
 	author = got_object_commit_get_author(commit);
+	author_time = got_object_commit_get_author_time(commit);
 	committer = got_object_commit_get_committer(commit);
+	committer_time = got_object_commit_get_committer_time(commit);
+	printf("from: %s\n", author);
+	if (author_time != committer_time)
+		print_date("orig:", &author_time);
 	if (strcmp(author, committer) != 0)
 		printf("via: %s\n", committer);
-	committer_time = got_object_commit_get_committer_time(commit);
-	datestr = get_datestr(&committer_time, datebuf);
-	if (datestr)
-		printf("date: %s UTC\n", datestr);
+	print_date("date:", &committer_time);
+
+	/* parent commits */
 	if (got_object_commit_get_nparents(commit) > 1) {
 		const struct got_object_id_queue *parent_ids;
 		struct got_object_qid *qid;
@@ -9554,8 +9570,11 @@ cmd_commit(int argc, char *argv[])
 	if (error)
 		goto done;
 
-	if (author == NULL)
+	if (author == NULL) {
+		/* got_worktree_commit() treats committer as the optional one */
 		author = committer;
+		committer = NULL;	/* => author timestamp is ignored */
+	}
 
 	if (logmsg == NULL || strlen(logmsg) == 0) {
 		error = get_editor(&editor);
@@ -9605,8 +9624,8 @@ cmd_commit(int argc, char *argv[])
 		cl_arg.branch_name += 11;
 	}
 	cl_arg.repo_path = got_repo_get_path(repo);
-	error = got_worktree_commit(&id, worktree, &paths, author, committer,
-	    allow_bad_symlinks, show_diff, commit_conflicts,
+	error = got_worktree_commit(&id, worktree, &paths, author, time(NULL),
+	    committer, allow_bad_symlinks, show_diff, commit_conflicts,
 	    collect_commit_logmsg, &cl_arg, print_status, NULL, repo);
 	if (error) {
 		if (error->code != GOT_ERR_COMMIT_MSG_EMPTY &&
@@ -13844,7 +13863,7 @@ cmd_merge(int argc, char *argv[])
 		goto done;
 	} else {
 		error = got_worktree_merge_commit(&merge_commit_id, worktree,
-		    fileindex, author, NULL, 1, branch_tip, branch_name,
+		    fileindex, author, 0, NULL, 1, branch_tip, branch_name,
 		    allow_conflict, repo, continue_merge ? print_status : NULL,
 		    NULL);
 		if (error)
blob - 4ad750063163bd75fcdbccf958d5b7b7a72278fb
blob + 01bbc9dc9ef3289c00fa880bba8f79aa56f756af
--- include/got_worktree.h
+++ include/got_worktree.h
@@ -275,12 +275,14 @@ typedef const struct got_error *(*got_worktree_commit_
  * current base commit.
  * An author and a non-empty log message must be specified.
  * The name of the committer is optional (may be NULL).
+ * If a committer is given, a separate author timestamp can be specified
+ * which is ignored otherwise.
  * If a path to be committed contains a symlink which points outside
  * of the path space under version control, raise an error unless
  * committing of such paths is being forced by the caller.
  */
 const struct got_error *got_worktree_commit(struct got_object_id **,
-    struct got_worktree *, struct got_pathlist_head *, const char *,
+    struct got_worktree *, struct got_pathlist_head *, const char *, time_t,
     const char *, int, int, int, got_worktree_commit_msg_cb, void *,
     got_worktree_status_cb, void *, struct got_repository *);
 
@@ -498,9 +500,9 @@ got_worktree_merge_branch(struct got_worktree *worktre
 const struct got_error *
 got_worktree_merge_commit(struct got_object_id **new_commit_id,
     struct got_worktree *worktree, struct got_fileindex *fileindex,
-    const char *author, const char *committer, int allow_bad_symlinks,
-    struct got_object_id *branch_tip, const char *branch_name,
-    int allow_conflict, struct got_repository *repo,
+    const char *author, time_t author_ts, const char *committer,
+    int allow_bad_symlinks, struct got_object_id *branch_tip,
+    const char *branch_name, int allow_conflict, struct got_repository *repo,
     got_worktree_status_cb status_cb, void *status_arg);
 
 /*
blob - 94cb5018eb8bd148ea52e237479e027fabca1b14
blob + 5b9a648289b2a600c3cfeccfd20bff90c83e6b10
--- lib/worktree.c
+++ lib/worktree.c
@@ -6371,8 +6371,8 @@ commit_worktree(struct got_object_id **new_commit_id,
     struct got_object_id *head_commit_id,
     struct got_object_id *parent_id2,
     struct got_worktree *worktree,
-    const char *author, const char *committer, char *diff_path,
-    got_worktree_commit_msg_cb commit_msg_cb, void *commit_arg,
+    const char *author, time_t author_time, const char *committer,
+    char *diff_path, got_worktree_commit_msg_cb commit_msg_cb, void *commit_arg,
     got_worktree_status_cb status_cb, void *status_arg,
     struct got_repository *repo)
 {
@@ -6460,8 +6460,10 @@ commit_worktree(struct got_object_id **new_commit_id,
 		nparents++;
 	}
 	timestamp = time(NULL);
+	if (committer == NULL)
+		author_time = timestamp;
 	err = got_object_commit_create(new_commit_id, new_tree_id, &parent_ids,
-	    nparents, author, timestamp, committer, timestamp, logmsg, repo);
+	    nparents, author, author_time, committer, timestamp, logmsg, repo);
 	if (logmsg != NULL)
 		free(logmsg);
 	if (err)
@@ -6577,8 +6579,8 @@ check_non_staged_files(struct got_fileindex *fileindex
 const struct got_error *
 got_worktree_commit(struct got_object_id **new_commit_id,
     struct got_worktree *worktree, struct got_pathlist_head *paths,
-    const char *author, const char *committer, int allow_bad_symlinks,
-    int show_diff, int commit_conflicts,
+    const char *author, time_t author_time, const char *committer,
+    int allow_bad_symlinks, int show_diff, int commit_conflicts,
     got_worktree_commit_msg_cb commit_msg_cb, void *commit_arg,
     got_worktree_status_cb status_cb, void *status_arg,
     struct got_repository *repo)
@@ -6691,7 +6693,7 @@ got_worktree_commit(struct got_object_id **new_commit_
 	}
 
 	err = commit_worktree(new_commit_id, &commitable_paths,
-	    head_commit_id, NULL, worktree, author, committer,
+	    head_commit_id, NULL, worktree, author, author_time, committer,
 	    (diff_path && cc_arg.diff_header_shown) ? diff_path : NULL,
 	    commit_msg_cb, commit_arg, status_cb, status_arg, repo);
 	if (err)
@@ -7339,6 +7341,7 @@ rebase_commit(struct got_object_id **new_commit_id,
 	/* NB: commit_worktree will call free(logmsg) */
 	err = commit_worktree(new_commit_id, &commitable_paths, head_commit_id,
 	    NULL, worktree, got_object_commit_get_author(orig_commit),
+	    got_object_commit_get_author_time(orig_commit),
 	    committer ? committer :
 	    got_object_commit_get_committer(orig_commit), NULL,
 	    collect_rebase_commit_msg, logmsg, rebase_status, NULL, repo);
@@ -8651,11 +8654,10 @@ done:
 const struct got_error *
 got_worktree_merge_commit(struct got_object_id **new_commit_id,
     struct got_worktree *worktree, struct got_fileindex *fileindex,
-    const char *author, const char *committer, int allow_bad_symlinks,
-    struct got_object_id *branch_tip, const char *branch_name,
-    int allow_conflict, struct got_repository *repo,
+    const char *author, time_t author_time, const char *committer,
+    int allow_bad_symlinks, struct got_object_id *branch_tip,
+    const char *branch_name, int allow_conflict, struct got_repository *repo,
     got_worktree_status_cb status_cb, void *status_arg)
-
 {
 	const struct got_error *err = NULL, *sync_err;
 	struct got_pathlist_head commitable_paths;
@@ -8708,8 +8710,9 @@ got_worktree_merge_commit(struct got_object_id **new_c
 	mcm_arg.worktree = worktree;
 	mcm_arg.branch_name = branch_name;
 	err = commit_worktree(new_commit_id, &commitable_paths,
-	    head_commit_id, branch_tip, worktree, author, committer, NULL,
-	    merge_commit_msg_cb, &mcm_arg, status_cb, status_arg, repo);
+	    head_commit_id, branch_tip, worktree, author, author_time,
+	    committer, NULL, merge_commit_msg_cb, &mcm_arg, status_cb,
+	    status_arg, repo);
 	if (err)
 		goto done;
 
blob - c824aaf0100a26fa4d112f62f5136272c5c82bbc
blob + aa8c37ce5f9af6c3b3fdf320c95dd8316334d667
--- regress/cmdline/rebase.sh
+++ regress/cmdline/rebase.sh
@@ -952,8 +952,8 @@ test_rebase_preserves_logmsg() {
 	echo "modified alpha on branch" > $testroot/repo/alpha
 	git_commit $testroot/repo -m "modified alpha on newbranch"
 
-	(cd $testroot/repo && got log -c newbranch -l2 | grep -v ^date: \
-		> $testroot/log.expected)
+	(cd $testroot/repo && got log -c newbranch -l2 | \
+		egrep -v '^(date|orig):' > $testroot/log.expected)
 
 	local orig_commit1=`git_show_parent_commit $testroot/repo`
 	local orig_commit2=`git_show_head $testroot/repo`
@@ -986,8 +986,8 @@ test_rebase_preserves_logmsg() {
 		return 1
 	fi
 
-	(cd $testroot/wt && got log -c newbranch -l2 | grep -v ^date: \
-		> $testroot/log)
+	(cd $testroot/wt && got log -c newbranch -l2 | \
+		egrep -v '^(date|orig):' > $testroot/log)
 	ed -s $testroot/log.expected <<-EOF
 	,s/$orig_commit1/$new_commit1/
 	,s/$orig_commit2/$new_commit2/
blob - 674de2c51543e1af563af32b17a4d609805167ec
blob + 5289fe88e2afd9cf2bdc043d4722c21fa4843071
--- tog/tog.c
+++ tog/tog.c
@@ -571,7 +571,7 @@ struct tog_help_view_state {
 	KEY_("R", "Open ref view of all repository references"), \
 	KEY_("T", "Display tree view of the repository from the selected" \
 	    " commit"), \
-	KEY_("@", "Toggle between displaying author and committer name"), \
+	KEY_("@", "Toggle between displaying author and committer data"), \
 	KEY_("&", "Open prompt to enter term to limit commits displayed"), \
 	KEY_("C-g Backspace", "Cancel current search or log operation"), \
 	KEY_("C-l", "Reload the log view with new commits in the repository"), \
@@ -2441,7 +2441,7 @@ draw_commit(struct tog_view *view, struct commit_queue
 	int col, limit, scrollx, logmsg_x;
 	const int avail = view->ncols, marker_column = author_display_cols + 1;
 	struct tm tm;
-	time_t committer_time;
+	time_t timestamp;
 	struct tog_color *tc;
 	struct got_reflist_head *refs;
 
@@ -2456,8 +2456,11 @@ draw_commit(struct tog_view *view, struct commit_queue
 			return got_error_set_errno(rc, "pthread_cond_wait");
 	}
 
-	committer_time = got_object_commit_get_committer_time(commit);
-	if (gmtime_r(&committer_time, &tm) == NULL)
+	if (s->use_committer)
+		timestamp = got_object_commit_get_committer_time(commit);
+	else
+		timestamp = got_object_commit_get_author_time(commit);
+	if (gmtime_r(&timestamp, &tm) == NULL)
 		return got_error_from_errno("gmtime_r");
 	if (strftime(datebuf, sizeof(datebuf), "%F ", &tm) == 0)
 		return got_error(GOT_ERR_NO_SPACE);
@@ -5033,7 +5036,7 @@ draw_file(struct tog_view *view, const char *header)
 }
 
 static char *
-get_datestr(time_t *time, char *datebuf)
+get_datestr(const time_t *time, char *datebuf)
 {
 	struct tm mytm, *tm;
 	char *p, *s;
@@ -5123,16 +5126,41 @@ cat_diff(FILE *dst, FILE *src, struct got_diff_line **
 }
 
 static const struct got_error *
+write_date(struct got_diff_line **lines, size_t *nlines,
+    const char *prefix, const time_t *t, FILE *outfile, off_t *outoff)
+{
+	const struct got_error *err;
+	char datebuf[26], *datestr;
+	int n;
+
+	datestr = get_datestr(t, datebuf);
+	if (datestr == NULL)
+		return NULL;	/* silently ignored */
+
+	n = fprintf(outfile, "%s %s UTC\n", prefix, datestr);
+	if (n < 0) {
+		err = got_error_from_errno("fprintf");
+		return err;
+	}
+	*outoff += n;
+	err = add_line_metadata(lines, nlines, *outoff,
+	    GOT_DIFF_LINE_DATE);
+	if (err)
+		return err;
+
+	return NULL;
+}
+
+static const struct got_error *
 write_commit_info(struct got_diff_line **lines, size_t *nlines,
     struct got_object_id *commit_id, struct got_reflist_head *refs,
     struct got_repository *repo, int ignore_ws, int force_text_diff,
     struct got_diffstat_cb_arg *dsa, FILE *outfile)
 {
 	const struct got_error *err = NULL;
-	char datebuf[26], *datestr;
 	struct got_commit_object *commit;
 	char *id_str = NULL, *logmsg = NULL, *s = NULL, *line;
-	time_t committer_time;
+	time_t author_time, committer_time;
 	const char *author, *committer;
 	char *refs_str = NULL;
 	struct got_pathlist_entry *pe;
@@ -5168,19 +5196,25 @@ write_commit_info(struct got_diff_line **lines, size_t
 	if (err)
 		goto done;
 
-	n = fprintf(outfile, "from: %s\n",
-	    got_object_commit_get_author(commit));
-	if (n < 0) {
-		err = got_error_from_errno("fprintf");
-		goto done;
-	}
-	outoff += n;
-	err = add_line_metadata(lines, nlines, outoff, GOT_DIFF_LINE_AUTHOR);
-	if (err)
-		goto done;
-
 	author = got_object_commit_get_author(commit);
+	author_time = got_object_commit_get_author_time(commit);
 	committer = got_object_commit_get_committer(commit);
+	committer_time = got_object_commit_get_committer_time(commit);
+	n = fprintf(outfile, "from: %s\n", author);
+	if (n < 0) {
+		err = got_error_from_errno("fprintf");
+		goto done;
+	}
+	outoff += n;
+	err = add_line_metadata(lines, nlines, outoff, GOT_DIFF_LINE_AUTHOR);
+	if (err)
+		goto done;
+	if (author_time != committer_time) {
+		err = write_date(lines, nlines, "orig:", &author_time, outfile,
+		    &outoff);
+		if (err)
+			goto done;
+	}
 	if (strcmp(author, committer) != 0) {
 		n = fprintf(outfile, "via: %s\n", committer);
 		if (n < 0) {
@@ -5193,20 +5227,10 @@ write_commit_info(struct got_diff_line **lines, size_t
 		if (err)
 			goto done;
 	}
-	committer_time = got_object_commit_get_committer_time(commit);
-	datestr = get_datestr(&committer_time, datebuf);
-	if (datestr) {
-		n = fprintf(outfile, "date: %s UTC\n", datestr);
-		if (n < 0) {
-			err = got_error_from_errno("fprintf");
-			goto done;
-		}
-		outoff += n;
-		err = add_line_metadata(lines, nlines, outoff,
-		    GOT_DIFF_LINE_DATE);
-		if (err)
-			goto done;
-	}
+	err = write_date(lines, nlines, "date:", &committer_time, outfile,
+	    &outoff);
+	if (err)
+		goto done;
 	if (got_object_commit_get_nparents(commit) > 1) {
 		const struct got_object_id_queue *parent_ids;
 		struct got_object_qid *qid;