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

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: reset committer name during rebase and histedit
To:
Stefan Sperling <stsp@stsp.name>
Cc:
gameoftrees@openbsd.org
Date:
Sat, 23 Jul 2022 15:57:57 +0200

Download raw body.

Thread
Stefan Sperling <stsp@stsp.name> 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?

the test_rebase_no_author_info fails if there is a ~/.gitconfig file.
overriding HOME is not feasible, and providing an empty one in the repo
is not ideal either because get_author() will fall back to the global
config if the local one doesn't have a author defined (and if
$GOT_AUTHOR is not defined.)

So, as discussed on irc add an undocumented env var,
GOT_IGNORE_GITCONFIG for the tests.  only that this breaks the commit
tests.  To work around that and to hopefully simplify future tests, diff
belows moves some variable definition from the global scope into
run_test: this way we can override them in functions without affecting
the following tests in the same file.

diff /home/op/w/got
commit - 2c8899b952f67f90ab3a26b4331491832fe93c8a
path + /home/op/w/got
blob - 8546c7cf32d663cd5a666d40d2f858885cac2483
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -578,6 +578,7 @@ get_author(char **author, struct got_repository *repo,
 	const struct got_error *err = NULL;
 	const char *got_author = NULL, *name, *email;
 	const struct got_gotconfig *worktree_conf = NULL, *repo_conf = NULL;
+	int ignore_gitconfig = 0;
 
 	*author = NULL;
 
@@ -585,6 +586,8 @@ get_author(char **author, struct got_repository *repo,
 		worktree_conf = got_worktree_get_gotconfig(worktree);
 	repo_conf = got_repo_get_gotconfig(repo);
 
+	ignore_gitconfig = getenv("GOT_IGNORE_GITCONFIG") != NULL;
+
 	/*
 	 * Priority of potential author information sources, from most
 	 * significant to least significant:
@@ -600,16 +603,19 @@ get_author(char **author, struct got_repository *repo,
 	if (got_author == NULL)
 		got_author = got_gotconfig_get_author(repo_conf);
 	if (got_author == NULL) {
-		name = got_repo_get_gitconfig_author_name(repo);
-		email = got_repo_get_gitconfig_author_email(repo);
-		if (name && email) {
-			if (asprintf(author, "%s <%s>", name, email) == -1)
-				return got_error_from_errno("asprintf");
-			return NULL;
+		if (!ignore_gitconfig) {
+			name = got_repo_get_gitconfig_author_name(repo);
+			email = got_repo_get_gitconfig_author_email(repo);
+			if (name && email) {
+				if (asprintf(author, "%s <%s>", name, email)
+				    == -1)
+					return got_error_from_errno("asprintf");
+				return NULL;
+			}
 		}
 
 		got_author = getenv("GOT_AUTHOR");
-		if (got_author == NULL) {
+		if (got_author == NULL && !ignore_gitconfig) {
 			name = got_repo_get_global_gitconfig_author_name(repo);
 			email = got_repo_get_global_gitconfig_author_email(
 			    repo);
@@ -619,11 +625,14 @@ get_author(char **author, struct got_repository *repo,
 					return got_error_from_errno("asprintf");
 				return NULL;
 			}
-			/* TODO: Look up user in password database? */
-			return got_error(GOT_ERR_COMMIT_NO_AUTHOR);
 		}
 	}
 
+	if (got_author == NULL) {
+		/* TODO: Look up user in password database? */
+		return got_error(GOT_ERR_COMMIT_NO_AUTHOR);
+	}
+
 	*author = strdup(got_author);
 	if (*author == NULL)
 		return got_error_from_errno("strdup");
blob - bd32f96c5071b834cc2b1478703442467b2cbfca
file + regress/cmdline/commit.sh
--- regress/cmdline/commit.sh
+++ regress/cmdline/commit.sh
@@ -897,6 +897,8 @@ test_commit_gitconfig_author() {
 		return 1
 	fi
 
+	unset GOT_IGNORE_GITCONFIG
+
 	(cd $testroot/repo && git config user.name 'Flan Luck')
 	(cd $testroot/repo && git config user.email 'flan_luck@openbsd.org')
 
blob - 26f717faa35b5906f8abb7c9f459f633ebaa09dc
file + regress/cmdline/common.sh
--- regress/cmdline/common.sh
+++ regress/cmdline/common.sh
@@ -14,16 +14,6 @@
 # ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 # OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-export GIT_AUTHOR_NAME="Flan Hacker"
-export GIT_AUTHOR_EMAIL="flan_hacker@openbsd.org"
-export GIT_COMMITTER_NAME="$GIT_AUTHOR_NAME"
-export GIT_COMMITTER_EMAIL="$GIT_AUTHOR_EMAIL"
-export GOT_AUTHOR="$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
-export GOT_AUTHOR_8="flan_hac"
-export GOT_AUTHOR_11="flan_hacker"
-export GOT_LOG_DEFAULT_LIMIT=0
-export GOT_TEST_ROOT="/tmp"
-
 export MALLOC_OPTIONS=S
 
 git_init()
@@ -243,6 +233,17 @@ test_parseargs()
 
 run_test()
 {
+	export GIT_AUTHOR_NAME="Flan Hacker"
+	export GIT_AUTHOR_EMAIL="flan_hacker@openbsd.org"
+	export GIT_COMMITTER_NAME="$GIT_AUTHOR_NAME"
+	export GIT_COMMITTER_EMAIL="$GIT_AUTHOR_EMAIL"
+	export GOT_AUTHOR="$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
+	export GOT_AUTHOR_8="flan_hac"
+	export GOT_AUTHOR_11="flan_hacker"
+	export GOT_LOG_DEFAULT_LIMIT=0
+	export GOT_TEST_ROOT="/tmp"
+	export GOT_IGNORE_GITCONFIG=1
+
 	testfunc="$1"
 	if [ -z "$GOT_TEST_QUIET" ]; then
 		echo -n "$testfunc "