Download raw body.
histedit/rebase vs author/committer times
regarding the commit author vs committer turns out that we are already doing a sensible thing (i.e. keeping the original author when rebasing commits.) So, diff below just fixes the time accounting; the idea is: - use the current time when creating a new commit - don't call time(NULL) twice - when rebasing a commit use the old author_time and a new committer time from some tests this seems to put us in par with git, and I think that it's a sensible behaviour. (the diff goes on and adds some further checks in histedit and rebase to ensure we keep the authorship information and -to some extent- the times too) git (somehow) keeps the authorship also when "squashing" commits. (i.e. you fold a commit from Alice with one did by you using 'git rebase -i' and the resulting commit will still be from alice.) not sure if this is my lazyness speaking, but i don't think `got histedit should follow the same behaviour in this case. (going out of topic) it would be nice to have a mean to change the commit metadata (author and committer) during histedit however. i was thinking of adding `author' and `committer' that, like `mesg', would allow to change those info in-line; i.e.: pick abcdefg commit 1 author Flan Hacker <flan@openbsd.org> committer Omar Polo <op@omarpolo.com> mesg ... they aren't 4 letters long like pick, edit etc but "a" and "c" are free. (i thought of "meta" which would be 4 letters long but "m" is taken, or "info author xyz" / "info committer xyz" that's still 4 letter and "i" is free.) anyway, there's no rush, we can come back to it in the future. back on topic, ok/comments for the diff? :) diff refs/heads/main refs/heads/authorship commit - 97f28afb9820967508ab57f5ed6a5c75f49464b4 commit + e1d9e9decf0e09c7920560b46884c5e44350b0bf blob - 34f6db81edda3318d140a359f85fce30ea5fc4a7 blob + a31378bbbec0b87ac7b7404efe0a5d4516f67e2f --- got/got.c +++ got/got.c @@ -8486,7 +8486,10 @@ cmd_commit(int argc, char *argv[]) int allow_bad_symlinks = 0, non_interactive = 0, merge_in_progress = 0; struct got_pathlist_head paths; int *pack_fds = NULL; + time_t now; + now = time(NULL); + TAILQ_INIT(&paths); cl_arg.logmsg_path = NULL; @@ -8619,8 +8622,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, collect_commit_logmsg, &cl_arg, + error = got_worktree_commit(&id, worktree, &paths, author, now, + committer, now, allow_bad_symlinks, collect_commit_logmsg, &cl_arg, print_status, NULL, repo); if (error) { if (error->code != GOT_ERR_COMMIT_MSG_EMPTY && blob - 610e03aa8788afbfcdcdc782c96c1c1965707674 blob + 0874d5b5c83a449548ce984bc8d64dbcee49bf9e --- include/got_worktree.h +++ include/got_worktree.h @@ -241,7 +241,7 @@ typedef const struct got_error *(*got_worktree_commit_ */ const struct got_error *got_worktree_commit(struct got_object_id **, struct got_worktree *, struct got_pathlist_head *, const char *, - const char *, int, got_worktree_commit_msg_cb, void *, + time_t, const char *, time_t, int, got_worktree_commit_msg_cb, void *, got_worktree_status_cb, void *, struct got_repository *); /* Get the path of a commitable worktree item. */ blob - 585486aa9bd3b5620681a9e4ccad89befbbaf36b blob + 54ce7f86a3ee99f093efc3dbf88307dcb2f073b7 --- lib/worktree.c +++ lib/worktree.c @@ -5664,7 +5664,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, + const char *author, time_t author_time, + const char *committer, time_t committer_time, got_worktree_commit_msg_cb commit_msg_cb, void *commit_arg, got_worktree_status_cb status_cb, void *status_arg, struct got_repository *repo) @@ -5750,7 +5751,8 @@ commit_worktree(struct got_object_id **new_commit_id, nparents++; } err = got_object_commit_create(new_commit_id, new_tree_id, &parent_ids, - nparents, author, time(NULL), committer, time(NULL), logmsg, repo); + nparents, author, author_time, committer, committer_time, logmsg, + repo); if (logmsg != NULL) free(logmsg); if (err) @@ -5866,7 +5868,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, + const char *author, time_t author_time, const char *committer, + time_t committer_time, int allow_bad_symlinks, got_worktree_commit_msg_cb commit_msg_cb, void *commit_arg, got_worktree_status_cb status_cb, void *status_arg, struct got_repository *repo) @@ -5950,8 +5953,9 @@ got_worktree_commit(struct got_object_id **new_commit_ } err = commit_worktree(new_commit_id, &commitable_paths, - head_commit_id, NULL, worktree, author, committer, - commit_msg_cb, commit_arg, status_cb, status_arg, repo); + head_commit_id, NULL, worktree, author, author_time, committer, + committer_time, commit_msg_cb, commit_arg, status_cb, status_arg, + repo); if (err) goto done; @@ -6533,7 +6537,8 @@ 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_committer(orig_commit), + got_object_commit_get_author_time(orig_commit), + got_object_commit_get_committer(orig_commit), time(NULL), collect_rebase_commit_msg, logmsg, rebase_status, NULL, repo); if (err) goto done; @@ -7599,6 +7604,7 @@ got_worktree_merge_commit(struct got_object_id **new_c int have_staged_files = 0; struct merge_commit_msg_arg mcm_arg; char *fileindex_path = NULL; + time_t now; *new_commit_id = NULL; @@ -7656,10 +7662,12 @@ got_worktree_merge_commit(struct got_object_id **new_c } + now = time(NULL); + 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, + head_commit_id, branch_tip, worktree, author, now, committer, now, merge_commit_msg_cb, &mcm_arg, status_cb, status_arg, repo); if (err) goto done; blob - 89e063ffccc47d6560386bc393c8e4351e66eb36 blob + ed23d81e72b628a7197f0a4050a7a10aee7195bf --- regress/cmdline/histedit.sh +++ regress/cmdline/histedit.sh @@ -2092,6 +2092,72 @@ test_histedit_mesg_invalid() { test_done "$testroot" $ret } +test_histedit_fold_keep_author() { + local testroot=`test_init fold_keep_author` + local orig_commit=`git_show_head $testroot/repo` + local author='Foo <foo@example.com>' + + got checkout $testroot/repo $testroot/wt >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return 1 + fi + + echo "modified alpha on master" > $testroot/wt/alpha + (cd $testroot/wt && got commit -A "$author" -m 'edit alpha') >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return 1 + fi + + (cd $testroot/wt && got cat master | grep ^author) \ + > $testroot/author.expected + + local new_commit=`git_show_head $testroot/repo` + + (cd $testroot/wt && got update -c $orig_commit) >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return 1 + fi + + echo "pick $new_commit" > $testroot/histedit-script + echo "mesg edited alpha" >> $testroot/histedit-script + (cd $testroot/wt && got histedit -F $testroot/histedit-script) \ + >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return 1 + fi + + (cd $testroot/wt && got cat master | grep ^author) \ + > $testroot/author + cmp -s $testroot/author $testroot/author.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/author $testroot/author.expected + test_done "$testroot" $ret + return 1 + fi + + (cd $testroot/wt && got cat master | grep ^committer) | \ + sed 's/>.*/>/' > $testroot/committer + echo "committer $GOT_AUTHOR" > $testroot/committer.expected + cmp -s $testroot/committer $testroot/committer.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/committer $testroot/committer.expected + test_done "$testroot" $ret + return 1 + fi + + test_done "$testroot" $ret +} + test_parseargs "$@" run_test test_histedit_no_op run_test test_histedit_swap @@ -2113,3 +2179,4 @@ run_test test_histedit_fold_only_empty_logmsg run_test test_histedit_edit_only run_test test_histedit_prepend_line run_test test_histedit_mesg_invalid +run_test test_histedit_fold_keep_author blob - 350e19ca9e343605f4497444e665c380a5212b8a blob + cd52fa5597e77d8c9d5f5190880db91925daced2 --- regress/cmdline/rebase.sh +++ regress/cmdline/rebase.sh @@ -1480,6 +1480,75 @@ test_rebase_rm_add_rm_file() { test_done "$testroot" "$ret" } +test_rebase_keep_author() { + local testroot=`test_init rebase_keep_author` + local commit0=`git_show_head $testroot/repo` + + got checkout $testroot/repo $testroot/wt >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return $ret + fi + + (cd $testroot/wt && got branch newbranch) >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return $ret + fi + + local author='Foo <foo@example.com>' + + echo 'modified alpha on branch' > $testroot/wt/alpha + (cd $testroot/wt && got commit -A "$author" -m 'edit alpha') >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return $ret + fi + + (cd $testroot/wt && got update -b master) >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return $ret + fi + + echo "modified zeta on master" > $testroot/wt/epsilon/zeta + (cd $testroot/wt && got commit -m 'edit zeta') >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return $ret + fi + + (cd $testroot/wt && got update) >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return $ret + fi + + (cd $testroot/wt && got rebase newbranch) >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return $ret + fi + + (cd $testroot/wt && got log -l 1 | egrep '^(from|via):') \ + > $testroot/meta + echo "from: $author" > $testroot/meta.expected + echo "via: $GOT_AUTHOR" >> $testroot/meta.expected + cmp -s $testroot/meta.expected $testroot/meta + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/meta.expected $testroot/meta + fi + test_done "$testroot" $ret +} + test_parseargs "$@" run_test test_rebase_basic run_test test_rebase_ancestry_check @@ -1495,3 +1564,4 @@ run_test test_rebase_out_of_date run_test test_rebase_trims_empty_dir run_test test_rebase_delete_missing_file run_test test_rebase_rm_add_rm_file +run_test test_rebase_keep_author
histedit/rebase vs author/committer times