On Fri, Jul 22, 2022 at 10:09:58PM +0200, Stefan Sperling wrote:
> 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.

Ok

>  
> 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
> 

-- 

Tracey Emery