Download raw body.
histedit/rebase vs author/committer times
Stefan Sperling <stsp@stsp.name> wrote:
> On Fri, Jul 22, 2022 at 11:21:39AM +0200, Omar Polo wrote:
> > here's the updated diff, but at this point i'm even fine with dropping
> > it.
>
> Thinking about this some more: How about we update the author timestamp
> in case author name+email == committer name+email, and keep the existing
> author timestamp otherwise?
>
> This way, we would stop updating the author timestamp only once someone
> else "takes over" a commit object. This is not foolproof, of course (a
> given person could be using multiple email addresses). But a commit with
> equivalent author and committer name+email, and different timestamps
> for each, would look a bit strange.
yes, it seems a good idea
diff 89771dc50558f0fd902ecabecdc57e448bf7ab92 531381f27882dbe3fc1542c2b9c52a84f76aa3e1
commit - 89771dc50558f0fd902ecabecdc57e448bf7ab92
commit + 531381f27882dbe3fc1542c2b9c52a84f76aa3e1
blob - af59edb11f59ce50cc07c7c94d5fb05a9acc0f54
blob + 009c0d06e2e5d8714a10c5c8a008771349f971b0
--- got/got.c
+++ got/got.c
@@ -8484,9 +8484,11 @@ 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 timestamp;
TAILQ_INIT(&paths);
cl_arg.logmsg_path = NULL;
+ timestamp = time(NULL);
while ((ch = getopt(argc, argv, "A:F:m:NS")) != -1) {
switch (ch) {
@@ -8617,9 +8619,9 @@ 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,
- print_status, NULL, repo);
+ error = got_worktree_commit(&id, worktree, &paths, author, timestamp,
+ committer, timestamp, allow_bad_symlinks,
+ collect_commit_logmsg, &cl_arg, print_status, NULL, repo);
if (error) {
if (error->code != GOT_ERR_COMMIT_MSG_EMPTY &&
cl_arg.logmsg_path != NULL)
blob - b2b02753e786580598a72d4ccfa4aeb006309afe
blob + ebd10359580e8245378ef80412c51e2bd57528fa
--- 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 - 718cf72db051a1e897c1d70657ac5f12fa7b1043
blob + 4621b4fb7694cc20f520a898c89289890f5cce3b
--- 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)
@@ -5681,7 +5682,6 @@ commit_worktree(struct got_object_id **new_commit_id,
struct got_object_id_queue parent_ids;
struct got_object_qid *pid = NULL;
char *logmsg = NULL;
- time_t timestamp;
*new_commit_id = NULL;
@@ -5750,9 +5750,9 @@ commit_worktree(struct got_object_id **new_commit_id,
STAILQ_INSERT_TAIL(&parent_ids, pid, entry);
nparents++;
}
- timestamp = time(NULL);
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, committer_time, logmsg,
+ repo);
if (logmsg != NULL)
free(logmsg);
if (err)
@@ -5868,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)
@@ -5952,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;
@@ -6463,6 +6465,8 @@ rebase_commit(struct got_object_id **new_commit_id,
struct got_reference *head_ref = NULL;
struct got_object_id *head_commit_id = NULL;
char *logmsg = NULL;
+ const char *author;
+ time_t author_time, committer_time;
TAILQ_INIT(&commitable_paths);
*new_commit_id = NULL;
@@ -6533,11 +6537,16 @@ rebase_commit(struct got_object_id **new_commit_id,
goto done;
}
+ author = got_object_commit_get_author(orig_commit);
+ author_time = got_object_commit_get_author_time(orig_commit);
+ committer_time = time(NULL);
+ if (!strcmp(author, committer))
+ author_time = committer_time;
+
/* 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),
- committer, collect_rebase_commit_msg, logmsg, rebase_status, NULL,
- repo);
+ NULL, worktree, author, author_time, committer, committer_time,
+ collect_rebase_commit_msg, logmsg, rebase_status, NULL, repo);
if (err)
goto done;
@@ -7604,10 +7613,12 @@ 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 timestamp;
*new_commit_id = NULL;
TAILQ_INIT(&commitable_paths);
+ timestamp = time(NULL);
err = get_fileindex_path(&fileindex_path, worktree);
if (err)
@@ -7664,8 +7675,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,
- merge_commit_msg_cb, &mcm_arg, status_cb, status_arg, repo);
+ head_commit_id, branch_tip, worktree, author, timestamp,
+ committer, timestamp, merge_commit_msg_cb, &mcm_arg,
+ status_cb, status_arg, repo);
if (err)
goto done;
blob - 26f717faa35b5906f8abb7c9f459f633ebaa09dc
blob + 88c72142ecf6a2486b63f1860cc0adc15989f5d7
--- regress/cmdline/common.sh
+++ regress/cmdline/common.sh
@@ -87,6 +87,13 @@ git_show_author_time()
(cd $repo && git show --no-patch --pretty='format:%at' $object)
}
+git_show_committer_time()
+{
+ local repo="$1"
+ local object="$2"
+ (cd $repo && git show --no-patch --pretty='format:%ct' $object)
+}
+
git_show_tagger_time()
{
local repo="$1"
blob - 1264347441d2f3f6859d520ab1230f6161a9d666
blob + f6acd386fa4215aa63dcca724a2318d5ab9b4b51
--- regress/cmdline/histedit.sh
+++ regress/cmdline/histedit.sh
@@ -2157,6 +2157,108 @@ test_histedit_resets_committer() {
test_done "$testroot" "$ret"
}
+test_histedit_keep_author_time() {
+ local testroot=`test_init keep_author_time`
+ local orig_commit=`git_show_head $testroot/repo`
+ local author='Flan Luck <flan_luck@openbsd.org>'
+
+ 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
+ fi
+ test_done "$testroot" $ret
+}
+
+test_histedit_reset_committer_time() {
+ local testroot=`test_init histedit_reset_committer_time`
+ local orig_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 $ret
+ fi
+
+ echo 'modified alpha on master' > $testroot/wt/alpha
+ (cd $testroot/wt && got commit -m 'edit alpha') >/dev/null
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ test_done "$testroot" $ret
+ return 1
+ fi
+
+ local cid=`git_show_head $testroot/repo`
+ local atime0=`git_show_author_time $testroot/repo master`
+ local ctime0=`git_show_committer_time $testroot/repo master`
+
+ # wait so the timestamp for the histedited commit will be
+ # different.
+ sleep 1
+
+ (cd $testroot/wt && got update -c $orig_commit) >/dev/null
+ echo "pick $cid" > $testroot/histedit-script
+ echo "mesg edited commit mesg" >> $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
+
+ local atime1=`git_show_author_time $testroot/repo master`
+ local ctime1=`git_show_committer_time $testroot/repo master`
+
+ if [ "$atime0" -eq "$atime1" -o "$ctime0" -eq "$ctime1" ]; then
+ echo "time mismatch ($atime0, $atime1, $ctime0, $ctime1)" >&2
+ test_done "$testroot" 1
+ return
+ fi
+
+ test_done "$testroot" 0
+}
+
test_parseargs "$@"
run_test test_histedit_no_op
run_test test_histedit_swap
@@ -2179,3 +2281,5 @@ run_test test_histedit_edit_only
run_test test_histedit_prepend_line
run_test test_histedit_mesg_invalid
run_test test_histedit_resets_committer
+run_test test_histedit_keep_author_time
+run_test test_histedit_reset_committer_time
blob - 2bdfc42c631a9bad8763528b9082b0deacbed9d9
blob + 5facfdf7c1fab68265ac6f16dcf9e83d2583e9ce
--- regress/cmdline/rebase.sh
+++ regress/cmdline/rebase.sh
@@ -1568,6 +1568,141 @@ test_rebase_resets_committer() {
test_done "$testroot" "$ret"
}
+test_rebase_keep_author_time() {
+ local testroot=`test_init rebase_keep_author`
+ local commit0=`git_show_head $testroot/repo`
+ local author='Flan Luck <flan_luck@openbsd.org>'
+
+ 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
+
+ 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 cat newbranch | grep ^author) \
+ > $testroot/author.expected
+
+ (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 cat newbranch | grep ^author) \
+ > $testroot/author
+
+ cmp -s $testroot/author $testroot/author.expected
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/author $testroot/author.expected
+ fi
+ test_done "$testroot" $ret
+}
+
+test_rebase_reset_committer_time() {
+ local testroot=`test_init rebase_reset_committer_time`
+
+ 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
+
+ echo 'modified alpha on branch' > $testroot/wt/alpha
+ (cd $testroot/wt && got commit -m 'edit alpha') >/dev/null
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ test_done "$testroot" $ret
+ return $ret
+ fi
+
+ local commit0=`git_show_head $testroot/repo`
+
+ (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
+
+ # wait so the timestamp for the rebased commit will be
+ # different.
+ sleep 1
+
+ (cd $testroot/wt && got update && got rebase newbranch) >/dev/null
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ test_done "$testroot" $ret
+ return $ret
+ fi
+
+ local commit1=`git_show_head $testroot/repo`
+
+ local atime0=`git_show_author_time $testroot/repo $commit0`
+ local atime1=`git_show_author_time $testroot/repo $commit1`
+ local ctime0=`git_show_committer_time $testroot/repo $commit0`
+ local ctime1=`git_show_committer_time $testroot/repo $commit1`
+
+ if [ "$atime0" -ne "$atime1" -o "$ctime0" -ne "$ctime1" -o \
+ "$atime0" -ne "$ctime0" ]; then
+ echo "time mismatch ($atime0, $atime1, $ctime0, $ctime1)" >&2
+ test_done "$testroot" 1
+ return
+ fi
+
+ test_done "$testroot" 0
+}
+
test_parseargs "$@"
run_test test_rebase_basic
run_test test_rebase_ancestry_check
@@ -1584,3 +1719,5 @@ 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_resets_committer
+run_test test_rebase_keep_author_time
+run_test test_rebase_reset_committer_time
histedit/rebase vs author/committer times