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

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: reset committer name during rebase and histedit
To:
Mikhail <mp39590@gmail.com>
Cc:
Omar Polo <op@omarpolo.com>, gameoftrees@openbsd.org
Date:
Fri, 22 Jul 2022 22:09:58 +0200

Download raw body.

Thread
On Fri, Jul 22, 2022 at 07:49:07PM +0300, Mikhail wrote:
> It looks like this patch breaks my usual workflow:
> 
> misha:/home/misha/work/got:1358$ got fetch && got up -b origin/main && got rebase main
> Connecting to "origin" git.gameoftrees.org
> Already up-to-date
> Already up-to-date
> got: GOT_AUTHOR environment variable is not set
> 
> It fails to rebase.
> 
> If I do got bo 598eac4331d322ab9e91ee6864c54845e3a6e86c and recompile
> everything starts to work fine.
> 
> Is it supposed to be like that?

No, this was an oversight.

It should hopefully work again as before with the diff below, and keep
the new behaviour intact if commits are being rebased and author info
can be found in the config. Does this repair the behaviour for you?

-----------------------------------------------
commit 3b021824845e1028bc462e443e0703bffc279d19 (rebase-noauthor)
from: Stefan Sperling <stsp@stsp.name>
date: Fri Jul 22 20:07:44 2022 UTC
 
 do not require local author info during 'got rebase'
 
 My commit to reset committer info during rebase was too strict
 in some use cases. Even when simply forwarding a branch the rebase
 operation could now fail if GOT_AUTHOR is not set.
 
 To fix this, fall back on existing commiter information if no author
 is configured. And try to obtain author info from Git config in case
 GOT_AUTHOR is not set.
 
 Problems reported by Mikhail.
 
diff 5e98fb339b4588f8e07c12b9f9ace9dbdcc47592 3b021824845e1028bc462e443e0703bffc279d19
commit - 5e98fb339b4588f8e07c12b9f9ace9dbdcc47592
commit + 3b021824845e1028bc462e443e0703bffc279d19
blob - af59edb11f59ce50cc07c7c94d5fb05a9acc0f54
blob + 802e8d33b5cc2470f76a85b5a09648dfdcaceed3
--- got/got.c
+++ got/got.c
@@ -10017,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, *committer = NULL;
+	char *cwd = NULL, *committer = NULL, *gitconfig_path = 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;
@@ -10117,14 +10117,17 @@ cmd_rebase(int argc, char *argv[])
 		}
 	}
 
+	error = get_gitconfig_path(&gitconfig_path);
+	if (error)
+		goto done;
 	error = got_repo_open(&repo,
-	    worktree ? got_worktree_get_repo_path(worktree) : cwd, NULL,
-	    pack_fds);
+	    worktree ? got_worktree_get_repo_path(worktree) : cwd,
+	    gitconfig_path, pack_fds);
 	if (error != NULL)
 		goto done;
 
 	error = get_author(&committer, repo, worktree);
-	if (error)
+	if (error && error->code != GOT_ERR_COMMIT_NO_AUTHOR)
 		goto done;
 
 	error = apply_unveil(got_repo_get_path(repo), 0,
@@ -10393,6 +10396,7 @@ cmd_rebase(int argc, char *argv[])
 done:
 	free(cwd);
 	free(committer);
+	free(gitconfig_path);
 	got_object_id_queue_free(&commits);
 	free(branch_head_commit_id);
 	free(resume_commit_id);
blob - 718cf72db051a1e897c1d70657ac5f12fa7b1043
blob + 587972ac2ca3434668208f4e401ac33c622657a1
--- lib/worktree.c
+++ lib/worktree.c
@@ -6536,8 +6536,9 @@ 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),
-	    committer, collect_rebase_commit_msg, logmsg, rebase_status, NULL,
-	    repo);
+	    committer ? committer :
+	    got_object_commit_get_committer(orig_commit),
+	    collect_rebase_commit_msg, logmsg, rebase_status, NULL, repo);
 	if (err)
 		goto done;
 
blob - 2bdfc42c631a9bad8763528b9082b0deacbed9d9
blob + 17e0a03100631d85b1f0e371cf6380a5c8c25fe3
--- regress/cmdline/rebase.sh
+++ regress/cmdline/rebase.sh
@@ -1568,6 +1568,93 @@ test_rebase_resets_committer() {
 	test_done "$testroot" "$ret"
 }
 
+test_rebase_no_author_info() {
+	local testroot=`test_init rebase_no_author_info`
+	local commit0=`git_show_head $testroot/repo`
+	local commit0_author_time=`git_show_author_time $testroot/repo`
+	local committer="$GOT_AUTHOR"
+
+	(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
+
+	unset GOT_AUTHOR
+	(cd $testroot/wt && 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: $committer" > $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
+
+	# Author info of rebased commit should match the original
+	(cd $testroot/repo && got log -l1 -c $new_commit2 | \
+		egrep '^(from|via):' > $testroot/stdout)
+	echo "from: $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
@@ -1584,3 +1671,4 @@ 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_no_author_info