From: "Sven M. Hallberg" Subject: Re: [patch] preserve and show author dates To: Stefan Sperling Cc: gameoftrees@openbsd.org Date: Mon, 19 Aug 2024 11:32:25 +0200 Here's the updated diff split into four parts: - got rebase and supporting changes - cvg analogues - tog: pressing '@' shows author dates - tog: add "orig" header in Diff view I backed out my changes to got log entirely, including the light refactor of splitting out the printing of a date header into its own function. I left the "orig" header in tog's Diff view because AFAICT it only appears when viewing single commits. That leaves a way to view these timestamps beside 'got cat' and (at date granularity) by pressing '@' in tog. The latter also still makes sense, I think. ----------------------------------------------- commit 6b473a45e0593273a08e2164e51fa23052d63254 from: Sven M. Hallberg date: Mon Aug 19 08:39:13 2024 UTC got rebase: preserve author timestamps Adds test_rebase_preserves_author_data to regress/cmdline/rebase.sh. diff 8cc4eb801418181a7eddf2ad28d85b4e60661ae7 6b473a45e0593273a08e2164e51fa23052d63254 commit - 8cc4eb801418181a7eddf2ad28d85b4e60661ae7 commit + 6b473a45e0593273a08e2164e51fa23052d63254 blob - 1817e3e5ed30160e539643b9331c2c8515ecdd4a blob + e8d84954238ba28d6c5572b92f97922298324aa3 --- got/got.c +++ got/got.c @@ -9552,8 +9552,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); @@ -9603,8 +9606,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 && @@ -13842,7 +13845,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 - 91bfba2775bd8f47a9c55f66b6815495fbc53d7b blob + 6d1b6efe73eb2cf4d46c63cd4a5ffcef25b0a623 --- include/got_worktree.h +++ include/got_worktree.h @@ -280,12 +280,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 *); @@ -503,9 +505,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_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); /* blob - 6f1ab86e6bb284e7bd1520334604392068bf8022 blob + e9998f86abe6db9c0a4ba95258cba5db502fab63 --- lib/worktree.c +++ lib/worktree.c @@ -6375,8 +6375,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) { @@ -6464,8 +6464,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) @@ -6581,8 +6583,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) @@ -6695,7 +6697,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) @@ -7343,6 +7345,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); @@ -8655,11 +8658,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; @@ -8712,8 +8714,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 + 2939c92222e12b92b220a53b982a6f2b974d4c60 --- regress/cmdline/rebase.sh +++ regress/cmdline/rebase.sh @@ -1002,6 +1002,68 @@ test_rebase_preserves_logmsg() { test_done "$testroot" "$ret" } +test_rebase_preserves_author_data() { + local testroot=`test_init rebase_preserves_author_data` + + git -C $testroot/repo checkout -q -b newbranch + echo "modified delta on branch" > $testroot/repo/gamma/delta + TZ=EST git_commit $testroot/repo -m "modified delta on newbranch" + + sleep 1 # get a new timestamp + echo "modified alpha on branch" > $testroot/repo/alpha + TZ=CET git_commit $testroot/repo -m "modified alpha on newbranch" + + local orig_commit1=`git_show_parent_commit $testroot/repo` + local orig_commit2=`git_show_head $testroot/repo` + + (cd $testroot/repo && got cat $orig_commit1 $orig_commit2 | \ + grep '^author ' > $testroot/author_data.expected) + + git -C $testroot/repo checkout -q master + echo "modified zeta on master" > $testroot/repo/epsilon/zeta + git_commit $testroot/repo -m "committing to zeta on master" + local master_commit=`git_show_head $testroot/repo` + + got checkout $testroot/repo $testroot/wt > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" "$ret" + return 1 + fi + + sleep 1 # get a new timestamp + (cd $testroot/wt && got rebase newbranch > /dev/null \ + 2> $testroot/stderr) + + git -C $testroot/repo checkout -q newbranch + local new_commit1=`git_show_parent_commit $testroot/repo` + local new_commit2=`git_show_head $testroot/repo` + + echo -n > $testroot/stderr.expected + cmp -s $testroot/stderr.expected $testroot/stderr + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stderr.expected $testroot/stderr + test_done "$testroot" "$ret" + return 1 + fi + + (cd $testroot/wt && got cat $new_commit1 $new_commit2 | \ + grep '^author ' > $testroot/author_data) + # note: got deliberately clobbers the timezone to UTC, so expect that + ed -s $testroot/author_data.expected <<-EOF + ,s/ [+-][0-9]\{4\}\$/ +0000/ + w + EOF + cmp -s $testroot/author_data.expected $testroot/author_data + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/author_data.expected $testroot/author_data + fi + + test_done "$testroot" "$ret" +} + test_rebase_no_commits_to_rebase() { local testroot=`test_init rebase_no_commits_to_rebase` @@ -2286,6 +2348,7 @@ run_test test_rebase_no_op_change run_test test_rebase_in_progress run_test test_rebase_path_prefix run_test test_rebase_preserves_logmsg +run_test test_rebase_preserves_author_data run_test test_rebase_no_commits_to_rebase run_test test_rebase_forward run_test test_rebase_forward_path_prefix ----------------------------------------------- commit 3b1c57a08e0ae678850bad996672d98768e09f84 from: Sven M. Hallberg date: Mon Aug 19 08:39:14 2024 UTC cvg: add author_time argument to got_worktree_cvg_commit() No functional change, but keeps the code in sync with got. diff 6b473a45e0593273a08e2164e51fa23052d63254 3b1c57a08e0ae678850bad996672d98768e09f84 commit - 6b473a45e0593273a08e2164e51fa23052d63254 commit + 3b1c57a08e0ae678850bad996672d98768e09f84 blob - 2aaaecc2e9d45cd6eb0bba01deb76049d231c968 blob + 7ed8ab9c1b29110d883c6e3d36b8344b3f1d6c69 --- cvg/cvg.c +++ cvg/cvg.c @@ -7907,8 +7907,11 @@ cmd_commit(int argc, char *argv[]) if (error) goto done; - if (author == NULL) + if (author == NULL) { + /* got_worktree_cvg_commit() treats committer as optional */ author = committer; + committer = NULL; /* => author timestamp is ignored */ + } remote_name = GOT_SEND_DEFAULT_REMOTE_NAME; worktree_conf = got_worktree_get_gotconfig(worktree); @@ -8021,9 +8024,10 @@ cmd_commit(int argc, char *argv[]) cl_arg.repo_path = got_repo_get_path(repo); cl_arg.dial_proto = proto; error = got_worktree_cvg_commit(&id, worktree, &paths, author, - committer, allow_bad_symlinks, show_diff, commit_conflicts, - collect_commit_logmsg, &cl_arg, print_status, NULL, proto, host, - port, server_path, verbosity, remote, check_cancelled, repo); + time(NULL), committer, allow_bad_symlinks, show_diff, + commit_conflicts, collect_commit_logmsg, &cl_arg, print_status, + NULL, proto, host, port, server_path, verbosity, remote, + check_cancelled, repo); if (error) { if (error->code != GOT_ERR_COMMIT_MSG_EMPTY && cl_arg.logmsg_path != NULL) blob - 5607eff4903ac486cf8766910d0e6ea7a47f928a blob + 0ea849834dcb2c8bdb84b998209dc58648b302d7 --- include/got_worktree_cvg.h +++ include/got_worktree_cvg.h @@ -23,12 +23,14 @@ * 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_cvg_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 *, const char *, const char *, const char *, const char *, int, const struct got_remote_repo *, got_cancel_cb, blob - da97321077d9a4c57c2b10105cd568a1bc9c944e blob + ba550d94227fc2d310d5e48355462f0c1ee487b3 --- lib/worktree_cvg.c +++ lib/worktree_cvg.c @@ -2229,8 +2229,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) { @@ -2315,8 +2315,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) @@ -2904,8 +2906,8 @@ done: const struct got_error * got_worktree_cvg_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, const char *proto, const char *host, const char *port, @@ -3046,7 +3048,7 @@ got_worktree_cvg_commit(struct got_object_id **new_com } 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) ----------------------------------------------- commit 7b0989c733694e4d79049803668c74b4840b6e20 from: Sven M. Hallberg date: Mon Aug 19 08:39:14 2024 UTC tog log: show author dates if showing author names (@) diff 3b1c57a08e0ae678850bad996672d98768e09f84 7b0989c733694e4d79049803668c74b4840b6e20 commit - 3b1c57a08e0ae678850bad996672d98768e09f84 commit + 7b0989c733694e4d79049803668c74b4840b6e20 blob - 091072c0d6ba8b4c672d3c4534c282af4f86830b blob + db270280ca4d6040a9b0446506b8b3dbb684c927 --- tog/tog.c +++ tog/tog.c @@ -575,7 +575,7 @@ struct tog_help_view_state { " commit"), \ KEY_("m", "Mark or unmark the selected entry for diffing with the " \ "next 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"), \ @@ -2483,7 +2483,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; @@ -2498,8 +2498,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(×tamp, &tm) == NULL) return got_error_from_errno("gmtime_r"); if (strftime(datebuf, sizeof(datebuf), "%F ", &tm) == 0) return got_error(GOT_ERR_NO_SPACE); ----------------------------------------------- commit 5f2b57d4a3b2a0b0a66e032ca158b370e32d3067 (authortime2, origin/authortime2) from: Sven M. Hallberg date: Mon Aug 19 08:39:14 2024 UTC tog: show author ("orig") date in Diff view if different from committer date diff 7b0989c733694e4d79049803668c74b4840b6e20 5f2b57d4a3b2a0b0a66e032ca158b370e32d3067 commit - 7b0989c733694e4d79049803668c74b4840b6e20 commit + 5f2b57d4a3b2a0b0a66e032ca158b370e32d3067 blob - db270280ca4d6040a9b0446506b8b3dbb684c927 blob + 4ea8a78d3b0816de91a5213adb84b8c113384b69 --- tog/tog.c +++ tog/tog.c @@ -5103,7 +5103,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; @@ -5254,16 +5254,41 @@ write_diffstat(FILE *outfile, struct got_diff_line **l } 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; off_t outoff = 0; @@ -5298,19 +5323,20 @@ 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 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); + 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 (strcmp(author, committer) != 0) { n = fprintf(outfile, "via: %s\n", committer); if (n < 0) { @@ -5323,20 +5349,18 @@ 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); + err = write_date(lines, nlines, "date:", &committer_time, outfile, + &outoff); + if (err) + goto done; + if (author_time != committer_time) { + err = write_date(lines, nlines, "orig:", &author_time, outfile, + &outoff); if (err) goto done; } + + /* parent commits */ if (got_object_commit_get_nparents(commit) > 1) { const struct got_object_id_queue *parent_ids; struct got_object_qid *qid;