Refactor legacy git init (#20376)

* merge `CheckLFSVersion` into `InitFull` (renamed from `InitWithSyncOnce`)
* remove the `Once` during git init, no data-race now
* for doctor sub-commands, `InitFull` should only be called in initialization stage

Co-authored-by: zeripath <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>

Author: 3 people

cmd/doctor.go

@@ -14,6 +14,7 @@ import (
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/migrations"
 	"code.gitea.io/gitea/modules/doctor"
+	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 
@@ -124,13 +125,18 @@ func runRecreateTable(ctx *cli.Context) error {
 }
 
 func runDoctor(ctx *cli.Context) error {
+	stdCtx, cancel := installSignals()
+	defer cancel()
+
+	// some doctor sub-commands need to use git command
+	if err := git.InitFull(stdCtx); err != nil {
+		return err
+	}
+
 	// Silence the default loggers
 	log.DelNamedLogger("console")
 	log.DelNamedLogger(log.DEFAULT)
 
-	stdCtx, cancel := installSignals()
-	defer cancel()
-
 	// Now setup our own
 	logFile := ctx.String("log-file")
 	if !ctx.IsSet("log-file") {

contrib/pr/checkout.go

@@ -80,7 +80,6 @@ func runPR() {
 	setting.RunUser = curUser.Username
 
 	log.Printf("[PR] Loading fixtures data ...\n")
-	gitea_git.CheckLFSVersion()
 	//models.LoadConfigs()
 	/*
 		setting.Database.Type = "sqlite3"

integrations/git_test.go

@@ -156,11 +156,6 @@ func standardCommitAndPushTest(t *testing.T, dstPath string) (little, big string
 func lfsCommitAndPushTest(t *testing.T, dstPath string) (littleLFS, bigLFS string) {
 	t.Run("LFS", func(t *testing.T) {
 		defer PrintCurrentTest(t)()
-		git.CheckLFSVersion()
-		if !setting.LFS.StartServer {
-			t.Skip()
-			return
-		}
 		prefix := "lfs-data-file-"
 		err := git.NewCommand(git.DefaultContext, "lfs").AddArguments("install").Run(&git.RunOpts{Dir: dstPath})
 		assert.NoError(t, err)
@@ -226,7 +221,6 @@ func rawTest(t *testing.T, ctx *APITestContext, little, big, littleLFS, bigLFS s
 		resp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
 		assert.Equal(t, littleSize, resp.Length)
 
-		git.CheckLFSVersion()
 		if setting.LFS.StartServer {
 			req = NewRequest(t, "GET", path.Join("/", username, reponame, "/raw/branch/master/", littleLFS))
 			resp := session.MakeRequest(t, req, http.StatusOK)
@@ -268,12 +262,9 @@ func mediaTest(t *testing.T, ctx *APITestContext, little, big, littleLFS, bigLFS
 		resp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
 		assert.Equal(t, littleSize, resp.Length)
 
-		git.CheckLFSVersion()
-		if setting.LFS.StartServer {
-			req = NewRequest(t, "GET", path.Join("/", username, reponame, "/media/branch/master/", littleLFS))
-			resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
-			assert.Equal(t, littleSize, resp.Length)
-		}
+		req = NewRequest(t, "GET", path.Join("/", username, reponame, "/media/branch/master/", littleLFS))
+		resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
+		assert.Equal(t, littleSize, resp.Length)
 
 		if !testing.Short() {
 			req = NewRequest(t, "GET", path.Join("/", username, reponame, "/media/branch/master/", big))

integrations/integration_test.go

@@ -175,10 +175,9 @@ func initIntegrationTest() {
 	setting.Repository.DefaultBranch = "master" // many test code still assume that default branch is called "master"
 	_ = util.RemoveAll(repo_module.LocalCopyPath())
 
-	if err := git.InitOnceWithSync(context.Background()); err != nil {
+	if err := git.InitFull(context.Background()); err != nil {
 		log.Fatal("git.InitOnceWithSync: %v", err)
 	}
-	git.CheckLFSVersion()
 
 	setting.InitDBConfig()
 	if err := storage.Init(); err != nil {
@@ -285,7 +284,6 @@ func prepareTestEnv(t testing.TB, skip ...int) func() {
 	assert.NoError(t, unittest.LoadFixtures())
 	assert.NoError(t, util.RemoveAll(setting.RepoRootPath))
 	assert.NoError(t, unittest.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"), setting.RepoRootPath))
-	assert.NoError(t, git.InitOnceWithSync(context.Background())) // the gitconfig has been removed above, so sync the gitconfig again
 	ownerDirs, err := os.ReadDir(setting.RepoRootPath)
 	if err != nil {
 		assert.NoError(t, err, "unable to read the new repo root: %v\n", err)
@@ -586,7 +584,6 @@ func resetFixtures(t *testing.T) {
 	assert.NoError(t, unittest.LoadFixtures())
 	assert.NoError(t, util.RemoveAll(setting.RepoRootPath))
 	assert.NoError(t, unittest.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"), setting.RepoRootPath))
-	assert.NoError(t, git.InitOnceWithSync(context.Background())) // the gitconfig has been removed above, so sync the gitconfig again
 	ownerDirs, err := os.ReadDir(setting.RepoRootPath)
 	if err != nil {
 		assert.NoError(t, err, "unable to read the new repo root: %v\n", err)

integrations/lfs_getobject_test.go

@@ -14,7 +14,6 @@ import (
 
 	git_model "code.gitea.io/gitea/models/git"
 	repo_model "code.gitea.io/gitea/models/repo"
-	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/lfs"
 	"code.gitea.io/gitea/modules/setting"
@@ -83,11 +82,6 @@ func checkResponseTestContentEncoding(t *testing.T, content *[]byte, resp *httpt
 
 func TestGetLFSSmall(t *testing.T) {
 	defer prepareTestEnv(t)()
-	git.CheckLFSVersion()
-	if !setting.LFS.StartServer {
-		t.Skip()
-		return
-	}
 	content := []byte("A very small file\n")
 
 	resp := storeAndGetLfs(t, &content, nil, http.StatusOK)
@@ -96,11 +90,6 @@ func TestGetLFSSmall(t *testing.T) {
 
 func TestGetLFSLarge(t *testing.T) {
 	defer prepareTestEnv(t)()
-	git.CheckLFSVersion()
-	if !setting.LFS.StartServer {
-		t.Skip()
-		return
-	}
 	content := make([]byte, web.GzipMinSize*10)
 	for i := range content {
 		content[i] = byte(i % 256)
@@ -112,11 +101,6 @@ func TestGetLFSLarge(t *testing.T) {
 
 func TestGetLFSGzip(t *testing.T) {
 	defer prepareTestEnv(t)()
-	git.CheckLFSVersion()
-	if !setting.LFS.StartServer {
-		t.Skip()
-		return
-	}
 	b := make([]byte, web.GzipMinSize*10)
 	for i := range b {
 		b[i] = byte(i % 256)
@@ -133,11 +117,6 @@ func TestGetLFSGzip(t *testing.T) {
 
 func TestGetLFSZip(t *testing.T) {
 	defer prepareTestEnv(t)()
-	git.CheckLFSVersion()
-	if !setting.LFS.StartServer {
-		t.Skip()
-		return
-	}
 	b := make([]byte, web.GzipMinSize*10)
 	for i := range b {
 		b[i] = byte(i % 256)
@@ -156,11 +135,6 @@ func TestGetLFSZip(t *testing.T) {
 
 func TestGetLFSRangeNo(t *testing.T) {
 	defer prepareTestEnv(t)()
-	git.CheckLFSVersion()
-	if !setting.LFS.StartServer {
-		t.Skip()
-		return
-	}
 	content := []byte("123456789\n")
 
 	resp := storeAndGetLfs(t, &content, nil, http.StatusOK)
@@ -169,11 +143,6 @@ func TestGetLFSRangeNo(t *testing.T) {
 
 func TestGetLFSRange(t *testing.T) {
 	defer prepareTestEnv(t)()
-	git.CheckLFSVersion()
-	if !setting.LFS.StartServer {
-		t.Skip()
-		return
-	}
 	content := []byte("123456789\n")
 
 	tests := []struct {

integrations/migration-test/migration_test.go

@@ -82,8 +82,7 @@ func initMigrationTest(t *testing.T) func() {
 		}
 	}
 
-	assert.NoError(t, git.InitOnceWithSync(context.Background()))
-	git.CheckLFSVersion()
+	assert.NoError(t, git.InitFull(context.Background()))
 	setting.InitDBConfig()
 	setting.NewLogServices(true)
 	return deferFn

models/migrations/migrations_test.go

@@ -66,11 +66,10 @@ func TestMain(m *testing.M) {
 
 	setting.SetCustomPathAndConf("", "", "")
 	setting.LoadForTest()
-	if err = git.InitOnceWithSync(context.Background()); err != nil {
-		fmt.Printf("Unable to InitOnceWithSync: %v\n", err)
+	if err = git.InitFull(context.Background()); err != nil {
+		fmt.Printf("Unable to InitFull: %v\n", err)
 		os.Exit(1)
 	}
-	git.CheckLFSVersion()
 	setting.InitDBConfig()
 	setting.NewLogServices(true)
 
@@ -207,7 +206,6 @@ func prepareTestEnv(t *testing.T, skip int, syncModels ...interface{}) (*xorm.En
 	deferFn := PrintCurrentTest(t, ourSkip)
 	assert.NoError(t, os.RemoveAll(setting.RepoRootPath))
 	assert.NoError(t, unittest.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"), setting.RepoRootPath))
-	assert.NoError(t, git.InitOnceWithSync(context.Background())) // the gitconfig has been removed above, so sync the gitconfig again
 	ownerDirs, err := os.ReadDir(setting.RepoRootPath)
 	if err != nil {
 		assert.NoError(t, err, "unable to read the new repo root: %v\n", err)

models/unittest/testdb.go

@@ -120,11 +120,9 @@ func MainTest(m *testing.M, testOpts *TestOptions) {
 		fatalTestError("util.CopyDir: %v\n", err)
 	}
 
-	if err = git.InitOnceWithSync(context.Background()); err != nil {
+	if err = git.InitFull(context.Background()); err != nil {
 		fatalTestError("git.Init: %v\n", err)
 	}
-	git.CheckLFSVersion()
-
 	ownerDirs, err := os.ReadDir(setting.RepoRootPath)
 	if err != nil {
 		fatalTestError("unable to read the new repo root: %v\n", err)
@@ -206,8 +204,6 @@ func PrepareTestEnv(t testing.TB) {
 	assert.NoError(t, util.RemoveAll(setting.RepoRootPath))
 	metaPath := filepath.Join(giteaRoot, "integrations", "gitea-repositories-meta")
 	assert.NoError(t, CopyDir(metaPath, setting.RepoRootPath))
-	assert.NoError(t, git.InitOnceWithSync(context.Background())) // the gitconfig has been removed above, so sync the gitconfig again
-
 	ownerDirs, err := os.ReadDir(setting.RepoRootPath)
 	assert.NoError(t, err)
 	for _, ownerDir := range ownerDirs {

modules/doctor/mergebase.go

@@ -30,9 +30,6 @@ func iteratePRs(ctx context.Context, repo *repo_model.Repository, each func(*rep
 }
 
 func checkPRMergeBase(ctx context.Context, logger log.Logger, autofix bool) error {
-	if err := git.InitOnceWithSync(ctx); err != nil {
-		return err
-	}
 	numRepos := 0
 	numPRs := 0
 	numPRsUpdated := 0

modules/doctor/misc.go

@@ -190,10 +190,6 @@ func checkDaemonExport(ctx context.Context, logger log.Logger, autofix bool) err
 }
 
 func checkCommitGraph(ctx context.Context, logger log.Logger, autofix bool) error {
-	if err := git.InitOnceWithSync(ctx); err != nil {
-		return err
-	}
-
 	numRepos := 0
 	numNeedUpdate := 0
 	numWritten := 0