Refactor env var related code (#33075)

And add more comments

5 files changed, 32 insertions, 52 deletions

diff --git a/cmd/main_test.go b/cmd/main_test.go
index c182b44019..3ec584d323 100644
--- a/cmd/main_test.go
+++ b/cmd/main_test.go
@@ -6,7 +6,6 @@ package cmd
 import (
 	"fmt"
 	"io"
-	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -113,37 +112,17 @@ func TestCliCmd(t *testing.T) {
 		_, _ = fmt.Fprint(ctx.App.Writer, makePathOutput(setting.AppWorkPath, setting.CustomPath, setting.CustomConf))
 		return nil
 	})
-	var envBackup []string
-	for _, s := range os.Environ() {
-		if strings.HasPrefix(s, "GITEA_") && strings.Contains(s, "=") {
-			envBackup = append(envBackup, s)
-		}
-	}
-	clearGiteaEnv := func() {
-		for _, s := range os.Environ() {
-			if strings.HasPrefix(s, "GITEA_") {
-				_ = os.Unsetenv(s)
-			}
-		}
-	}
-	defer func() {
-		clearGiteaEnv()
-		for _, s := range envBackup {
-			k, v, _ := strings.Cut(s, "=")
-			_ = os.Setenv(k, v)
-		}
-	}()
-
 	for _, c := range cases {
-		clearGiteaEnv()
-		for k, v := range c.env {
-			_ = os.Setenv(k, v)
-		}
-		args := strings.Split(c.cmd, " ") // for test only, "split" is good enough
-		r, err := runTestApp(app, args...)
-		assert.NoError(t, err, c.cmd)
-		assert.NotEmpty(t, c.exp, c.cmd)
-		assert.Contains(t, r.Stdout, c.exp, c.cmd)
+		t.Run(c.cmd, func(t *testing.T) {
+			for k, v := range c.env {
+				t.Setenv(k, v)
+			}
+			args := strings.Split(c.cmd, " ") // for test only, "split" is good enough
+			r, err := runTestApp(app, args...)
+			assert.NoError(t, err, c.cmd)
+			assert.NotEmpty(t, c.exp, c.cmd)
+			assert.Contains(t, r.Stdout, c.exp, c.cmd)
+		})
 	}
 }
 
diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go
index 43d084845c..7bf9829b6f 100644
--- a/models/unittest/testdb.go
+++ b/models/unittest/testdb.go
@@ -60,6 +60,12 @@ func InitSettings() {
 
 	setting.PasswordHashAlgo, _ = hash.SetDefaultPasswordHashAlgorithm("dummy")
 	setting.InitGiteaEnvVars()
+
+	// Avoid loading the git's system config.
+	// On macOS, system config sets the osxkeychain credential helper, which will cause tests to freeze with a dialog.
+	// But we do not set it in production at the moment, because it might be a "breaking" change,
+	// more details are in "modules/git.commonBaseEnvs".
+	_ = os.Setenv("GIT_CONFIG_NOSYSTEM", "true")
 }
 
 // TestOptions represents test options
diff --git a/modules/setting/config_env.go b/modules/setting/config_env.go
index f05e8a4d7b..e72249d82c 100644
--- a/modules/setting/config_env.go
+++ b/modules/setting/config_env.go
@@ -167,7 +167,21 @@ func EnvironmentToConfig(cfg ConfigProvider, envs []string) (changed bool) {
 	return changed
 }
 
-// InitGiteaEnvVars initilises the environment for gitea
+// InitGiteaEnvVars initializes the environment variables for gitea
 func InitGiteaEnvVars() {
-	_ = os.Unsetenv("XDG_CONFIG_HOME") // unset if set as HOME is managed by gitea
+	// Ideally Gitea should only accept the environment variables which it clearly knows instead of unsetting the ones it doesn't want,
+	// but the ideal behavior would be a breaking change, and it seems not bringing enough benefits to end users,
+	// so at the moment we could still keep "unsetting the unnecessary environments"
+
+	// HOME is managed by Gitea, Gitea's git should use "HOME/.gitconfig".
+	// But git would try "XDG_CONFIG_HOME/git/config" first if "HOME/.gitconfig" does not exist,
+	// then our git.InitFull would still write to "XDG_CONFIG_HOME/git/config" if XDG_CONFIG_HOME is set.
+	_ = os.Unsetenv("XDG_CONFIG_HOME")
+
+	_ = os.Unsetenv("GIT_AUTHOR_NAME")
+	_ = os.Unsetenv("GIT_AUTHOR_EMAIL")
+	_ = os.Unsetenv("GIT_AUTHOR_DATE")
+	_ = os.Unsetenv("GIT_COMMITTER_NAME")
+	_ = os.Unsetenv("GIT_COMMITTER_EMAIL")
+	_ = os.Unsetenv("GIT_COMMITTER_DATE")
 }
diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go
index 78e42777bb..ece136be50 100644
--- a/tests/e2e/e2e_test.go
+++ b/tests/e2e/e2e_test.go
@@ -40,13 +40,6 @@ func TestMain(m *testing.M) {
 	tests.InitTest(false)
 	testE2eWebRoutes = routers.NormalRoutes()
 
-	os.Unsetenv("GIT_AUTHOR_NAME")
-	os.Unsetenv("GIT_AUTHOR_EMAIL")
-	os.Unsetenv("GIT_AUTHOR_DATE")
-	os.Unsetenv("GIT_COMMITTER_NAME")
-	os.Unsetenv("GIT_COMMITTER_EMAIL")
-	os.Unsetenv("GIT_COMMITTER_DATE")
-
 	err := unittest.InitFixtures(
 		unittest.FixturesOptions{
 			Dir: filepath.Join(filepath.Dir(setting.AppPath), "models/fixtures/"),
diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go
index 9b3b2f2b92..46b93b0a10 100644
--- a/tests/integration/integration_test.go
+++ b/tests/integration/integration_test.go
@@ -88,18 +88,6 @@ func TestMain(m *testing.M) {
 	tests.InitTest(true)
 	testWebRoutes = routers.NormalRoutes()
 
-	os.Unsetenv("GIT_AUTHOR_NAME")
-	os.Unsetenv("GIT_AUTHOR_EMAIL")
-	os.Unsetenv("GIT_AUTHOR_DATE")
-	os.Unsetenv("GIT_COMMITTER_NAME")
-	os.Unsetenv("GIT_COMMITTER_EMAIL")
-	os.Unsetenv("GIT_COMMITTER_DATE")
-
-	// Avoid loading the default system config. On MacOS, this config
-	// sets the osxkeychain credential helper, which will cause tests
-	// to freeze with a dialog.
-	os.Setenv("GIT_CONFIG_NOSYSTEM", "true")
-
 	err := unittest.InitFixtures(
 		unittest.FixturesOptions{
 			Dir: filepath.Join(filepath.Dir(setting.AppPath), "models/fixtures/"),