"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
reset committer name during rebase and histedit
To:
gameoftrees@openbsd.org
Date:
Thu, 21 Jul 2022 23:30:44 +0200

Download raw body.

Thread 
This makes the rebase and histedit commands reset committer name in
newly created commits, based on author info found in the usual way.

Before this change, the committer field was simply copied from the
meta-data of each rebased/edited commit. It makes sense to reset this
field when commits get passed on via someone else (this was suggested
to me by jrick during r2k22).

Also, this behaviour change is required to support server-side checks
which ensure that a committer's name matches authentication credentials
used to send commits.
Which is something our own future server should probably do.

ok?

diff refs/heads/main refs/heads/reset-committer
commit - f26cc5f2126442207cd94f69d093f159445e8e20
commit + 2b6152e714dff8c212da28a1a2daffb00435263c
blob - 9f183e571635bf216c5457b05b5b7a35c77fa034
blob + af59edb11f59ce50cc07c7c94d5fb05a9acc0f54
--- got/got.c
+++ got/got.c
@@ -9578,7 +9578,7 @@ rebase_complete(struct got_worktree *worktree, struct 
 static const struct got_error *
 rebase_commit(struct got_pathlist_head *merged_paths,
     struct got_worktree *worktree, struct got_fileindex *fileindex,
-    struct got_reference *tmp_branch,
+    struct got_reference *tmp_branch, const char *committer,
     struct got_object_id *commit_id, struct got_repository *repo)
 {
 	const struct got_error *error;
@@ -9590,7 +9590,8 @@ rebase_commit(struct got_pathlist_head *merged_paths,
 		return error;
 
 	error = got_worktree_rebase_commit(&new_commit_id, merged_paths,
-	    worktree, fileindex, tmp_branch, commit, commit_id, repo);
+	    worktree, fileindex, tmp_branch, committer, commit, commit_id,
+	    repo);
 	if (error) {
 		if (error->code != GOT_ERR_COMMIT_NO_CHANGES)
 			goto done;
@@ -10016,7 +10017,7 @@ cmd_rebase(int argc, char *argv[])
 	struct got_worktree *worktree = NULL;
 	struct got_repository *repo = NULL;
 	struct got_fileindex *fileindex = NULL;
-	char *cwd = NULL;
+	char *cwd = NULL, *committer = NULL;
 	struct got_reference *branch = NULL;
 	struct got_reference *new_base_branch = NULL, *tmp_branch = NULL;
 	struct got_object_id *commit_id = NULL, *parent_id = NULL;
@@ -10122,6 +10123,10 @@ cmd_rebase(int argc, char *argv[])
 	if (error != NULL)
 		goto done;
 
+	error = get_author(&committer, repo, worktree);
+	if (error)
+		goto done;
+
 	error = apply_unveil(got_repo_get_path(repo), 0,
 	    worktree ? got_worktree_get_root_path(worktree) : NULL);
 	if (error)
@@ -10189,7 +10194,7 @@ cmd_rebase(int argc, char *argv[])
 			goto done;
 
 		error = rebase_commit(NULL, worktree, fileindex, tmp_branch,
-		    resume_commit_id, repo);
+		    committer, resume_commit_id, repo);
 		if (error)
 			goto done;
 
@@ -10352,7 +10357,7 @@ cmd_rebase(int argc, char *argv[])
 		}
 
 		error = rebase_commit(&merged_paths, worktree, fileindex,
-		    tmp_branch, commit_id, repo);
+		    tmp_branch, committer, commit_id, repo);
 		got_worktree_rebase_pathlist_free(&merged_paths);
 		if (error)
 			goto done;
@@ -10387,6 +10392,7 @@ cmd_rebase(int argc, char *argv[])
 		    new_base_branch, tmp_branch, repo, create_backup);
 done:
 	free(cwd);
+	free(committer);
 	got_object_id_queue_free(&commits);
 	free(branch_head_commit_id);
 	free(resume_commit_id);
@@ -11157,7 +11163,7 @@ static const struct got_error *
 histedit_commit(struct got_pathlist_head *merged_paths,
     struct got_worktree *worktree, struct got_fileindex *fileindex,
     struct got_reference *tmp_branch, struct got_histedit_list_entry *hle,
-    struct got_repository *repo)
+    const char *committer, struct got_repository *repo)
 {
 	const struct got_error *err;
 	struct got_commit_object *commit;
@@ -11175,7 +11181,7 @@ histedit_commit(struct got_pathlist_head *merged_paths
 		return err;
 
 	err = got_worktree_histedit_commit(&new_commit_id, merged_paths,
-	    worktree, fileindex, tmp_branch, commit, hle->commit_id,
+	    worktree, fileindex, tmp_branch, committer, commit, hle->commit_id,
 	    hle->logmsg, repo);
 	if (err) {
 		if (err->code != GOT_ERR_COMMIT_NO_CHANGES)
@@ -11250,7 +11256,7 @@ cmd_histedit(int argc, char *argv[])
 	struct got_worktree *worktree = NULL;
 	struct got_fileindex *fileindex = NULL;
 	struct got_repository *repo = NULL;
-	char *cwd = NULL;
+	char *cwd = NULL, *committer = NULL;
 	struct got_reference *branch = NULL;
 	struct got_reference *tmp_branch = NULL;
 	struct got_object_id *resume_commit_id = NULL;
@@ -11494,6 +11500,10 @@ cmd_histedit(int argc, char *argv[])
 		goto done;
 	}
 
+	error = get_author(&committer, repo, worktree);
+	if (error)
+		goto done;
+
 	if (continue_edit) {
 		char *path;
 
@@ -11668,7 +11678,8 @@ cmd_histedit(int argc, char *argv[])
 				}
 				if (have_changes) {
 					error = histedit_commit(NULL, worktree,
-					    fileindex, tmp_branch, hle, repo);
+					    fileindex, tmp_branch, hle,
+					    committer, repo);
 					if (error)
 						goto done;
 				} else {
@@ -11744,7 +11755,7 @@ cmd_histedit(int argc, char *argv[])
 		}
 
 		error = histedit_commit(&merged_paths, worktree, fileindex,
-		    tmp_branch, hle, repo);
+		    tmp_branch, hle, committer, repo);
 		got_worktree_rebase_pathlist_free(&merged_paths);
 		if (error)
 			goto done;
@@ -11779,6 +11790,7 @@ cmd_histedit(int argc, char *argv[])
 		    branch, repo);
 done:
 	free(cwd);
+	free(committer);
 	got_object_id_queue_free(&commits);
 	histedit_free_list(&histedit_cmds);
 	free(head_commit_id);
blob - 610e03aa8788afbfcdcdc782c96c1c1965707674
blob + b2b02753e786580598a72d4ccfa4aeb006309afe
--- include/got_worktree.h
+++ include/got_worktree.h
@@ -301,7 +301,7 @@ const struct got_error *got_worktree_rebase_merge_file
  */
 const struct got_error *got_worktree_rebase_commit(struct got_object_id **,
     struct got_pathlist_head *, struct got_worktree *, struct got_fileindex *,
-    struct got_reference *, struct got_commit_object *,
+    struct got_reference *, const char *, struct got_commit_object *,
     struct got_object_id *, struct got_repository *);
 
 /* Free a list of merged paths from got_worktree_merge_files. */
@@ -378,7 +378,7 @@ const struct got_error *got_worktree_histedit_merge_fi
  */
 const struct got_error *got_worktree_histedit_commit(struct got_object_id **,
     struct got_pathlist_head *, struct got_worktree *, struct got_fileindex *,
-    struct got_reference *, struct got_commit_object *,
+    struct got_reference *, const char *, struct got_commit_object *,
     struct got_object_id *, const char *, struct got_repository *);
 
 /*
blob - cc19332e99847eb9da2ce2e427552b4b000d5413
blob + 1aa74370d5f4ab8ad0df2976589a98d01b6e1657
--- lib/worktree.c
+++ lib/worktree.c
@@ -6450,8 +6450,9 @@ static const struct got_error *
 rebase_commit(struct got_object_id **new_commit_id,
     struct got_pathlist_head *merged_paths, struct got_reference *commit_ref,
     struct got_worktree *worktree, struct got_fileindex *fileindex,
-    struct got_reference *tmp_branch, struct got_commit_object *orig_commit,
-    const char *new_logmsg, struct got_repository *repo)
+    struct got_reference *tmp_branch, const char *committer,
+    struct got_commit_object *orig_commit, const char *new_logmsg,
+    struct got_repository *repo)
 {
 	const struct got_error *err, *sync_err;
 	struct got_pathlist_head commitable_paths;
@@ -6533,8 +6534,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),
-	    collect_rebase_commit_msg, logmsg, rebase_status, NULL, repo);
+	    committer, collect_rebase_commit_msg, logmsg, rebase_status, NULL,
+	    repo);
 	if (err)
 		goto done;
 
@@ -6567,7 +6568,7 @@ const struct got_error *
 got_worktree_rebase_commit(struct got_object_id **new_commit_id,
     struct got_pathlist_head *merged_paths, struct got_worktree *worktree,
     struct got_fileindex *fileindex, struct got_reference *tmp_branch,
-    struct got_commit_object *orig_commit,
+    const char *committer, struct got_commit_object *orig_commit,
     struct got_object_id *orig_commit_id, struct got_repository *repo)
 {
 	const struct got_error *err;
@@ -6591,7 +6592,8 @@ got_worktree_rebase_commit(struct got_object_id **new_
 	}
 
 	err = rebase_commit(new_commit_id, merged_paths, commit_ref,
-	    worktree, fileindex, tmp_branch, orig_commit, NULL, repo);
+	    worktree, fileindex, tmp_branch, committer, orig_commit,
+	    NULL, repo);
 done:
 	if (commit_ref)
 		got_ref_close(commit_ref);
@@ -6604,7 +6606,7 @@ const struct got_error *
 got_worktree_histedit_commit(struct got_object_id **new_commit_id,
     struct got_pathlist_head *merged_paths, struct got_worktree *worktree,
     struct got_fileindex *fileindex, struct got_reference *tmp_branch,
-    struct got_commit_object *orig_commit,
+    const char *committer, struct got_commit_object *orig_commit,
     struct got_object_id *orig_commit_id, const char *new_logmsg,
     struct got_repository *repo)
 {
@@ -6621,7 +6623,8 @@ got_worktree_histedit_commit(struct got_object_id **ne
 		goto done;
 
 	err = rebase_commit(new_commit_id, merged_paths, commit_ref,
-	    worktree, fileindex, tmp_branch, orig_commit, new_logmsg, repo);
+	    worktree, fileindex, tmp_branch, committer, orig_commit,
+	    new_logmsg, repo);
 done:
 	if (commit_ref)
 		got_ref_close(commit_ref);
blob - 89e063ffccc47d6560386bc393c8e4351e66eb36
blob + 1264347441d2f3f6859d520ab1230f6161a9d666
--- regress/cmdline/histedit.sh
+++ regress/cmdline/histedit.sh
@@ -2092,6 +2092,71 @@ test_histedit_mesg_invalid() {
 	test_done "$testroot" $ret
 }
 
+test_histedit_resets_committer() {
+	local testroot=`test_init histedit_resets_committer`
+	local orig_commit=`git_show_head $testroot/repo`
+	local committer="Flan Luck <flan_luck@openbsd.org>"
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+
+	echo "modified alpha" > $testroot/wt/alpha
+
+	(cd $testroot/wt/ && got commit -m 'modified alpha on master' \
+		alpha > /dev/null)
+	ret=$?
+	if [ "$?" != 0 ]; then
+		echo "got commit failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	local top_commit=`git_show_head $testroot/repo`
+	echo "pick $top_commit" > "$testroot/histedit-script"
+
+	(cd $testroot/wt/ && got update -c $orig_commit > /dev/null)
+	ret=$?
+	if [ "$?" != 0 ]; then
+		echo "got update failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	(cd $testroot/wt && env GOT_AUTHOR="$committer" \
+		got histedit -F "$testroot/histedit-script" > /dev/null)
+	ret=$?
+	if [ "$?" != 0 ]; then
+		echo "got histedit failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+	local edited_commit=`git_show_head $testroot/repo`
+
+	# Original commit only had one author
+	(cd $testroot/repo && got log -l1 -c $top_commit | \
+		egrep '^(from|via):' > $testroot/stdout)
+	echo "from: $GOT_AUTHOR" > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# Edited commit should have new committer name added
+	(cd $testroot/repo && got log -l1 -c $edited_commit | \
+		egrep '^(from|via):' > $testroot/stdout)
+	echo "from: $GOT_AUTHOR" > $testroot/stdout.expected
+	echo "via: $committer" >> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
+	test_done "$testroot" "$ret"
+}
+
 test_parseargs "$@"
 run_test test_histedit_no_op
 run_test test_histedit_swap
@@ -2113,3 +2178,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_resets_committer
blob - 350e19ca9e343605f4497444e665c380a5212b8a
blob + 2bdfc42c631a9bad8763528b9082b0deacbed9d9
--- regress/cmdline/rebase.sh
+++ regress/cmdline/rebase.sh
@@ -1480,6 +1480,94 @@ test_rebase_rm_add_rm_file() {
 	test_done "$testroot" "$ret"
 }
 
+test_rebase_resets_committer() {
+	local testroot=`test_init rebase_resets_committer`
+	local commit0=`git_show_head $testroot/repo`
+	local commit0_author_time=`git_show_author_time $testroot/repo`
+	local committer="Flan Luck <flan_luck@openbsd.org>"
+
+	(cd $testroot/repo && git checkout -q -b newbranch)
+	echo "modified delta on branch" > $testroot/repo/gamma/delta
+	git_commit $testroot/repo -m "committing to delta on newbranch"
+
+	echo "modified alpha on branch" > $testroot/repo/alpha
+	git_commit $testroot/repo -m "committing more changes on newbranch"
+
+	local orig_commit1=`git_show_parent_commit $testroot/repo`
+	local orig_commit2=`git_show_head $testroot/repo`
+	local orig_author_time2=`git_show_author_time $testroot/repo`
+
+	(cd $testroot/repo && git 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
+
+	(cd $testroot/wt && env GOT_AUTHOR="$committer" \
+		got rebase newbranch > $testroot/stdout)
+
+	(cd $testroot/repo && git checkout -q newbranch)
+	local new_commit1=`git_show_parent_commit $testroot/repo`
+	local new_commit2=`git_show_head $testroot/repo`
+	local new_author_time2=`git_show_author_time $testroot/repo`
+
+	local short_orig_commit1=`trim_obj_id 28 $orig_commit1`
+	local short_orig_commit2=`trim_obj_id 28 $orig_commit2`
+	local short_new_commit1=`trim_obj_id 28 $new_commit1`
+	local short_new_commit2=`trim_obj_id 28 $new_commit2`
+
+	echo "G  gamma/delta" >> $testroot/stdout.expected
+	echo -n "$short_orig_commit1 -> $short_new_commit1" \
+		>> $testroot/stdout.expected
+	echo ": committing to delta on newbranch" >> $testroot/stdout.expected
+	echo "G  alpha" >> $testroot/stdout.expected
+	echo -n "$short_orig_commit2 -> $short_new_commit2" \
+		>> $testroot/stdout.expected
+	echo ": committing more changes on newbranch" \
+		>> $testroot/stdout.expected
+	echo "Switching work tree to refs/heads/newbranch" \
+		>> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# Original commit only had one author
+	(cd $testroot/repo && got log -l1 -c $orig_commit2 | \
+		egrep '^(from|via):' > $testroot/stdout)
+	echo "from: $GOT_AUTHOR" > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# Rebased commit should have new committer name added
+	(cd $testroot/repo && got log -l1 -c $new_commit2 | \
+		egrep '^(from|via):' > $testroot/stdout)
+	echo "from: $GOT_AUTHOR" > $testroot/stdout.expected
+	echo "via: $committer" >> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
+	test_done "$testroot" "$ret"
+}
+
 test_parseargs "$@"
 run_test test_rebase_basic
 run_test test_rebase_ancestry_check
@@ -1495,3 +1583,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_resets_committer