From 8cd401ebde1f524bf9b9261251c85eb08e31acfc Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 15 Jul 2022 10:01:41 +0800 Subject: [PATCH 1/6] merge CheckLFSVersion into InitOnceWithSync --- contrib/pr/checkout.go | 1 - integrations/git_test.go | 15 ++------- integrations/integration_test.go | 1 - integrations/lfs_getobject_test.go | 31 ------------------- integrations/migration-test/migration_test.go | 1 - models/migrations/migrations_test.go | 1 - models/unittest/testdb.go | 2 -- modules/git/git.go | 10 +++++- modules/git/lfs.go | 31 ------------------- routers/init.go | 2 -- 10 files changed, 12 insertions(+), 83 deletions(-) delete mode 100644 modules/git/lfs.go diff --git a/contrib/pr/checkout.go b/contrib/pr/checkout.go index f6d29f3c5b574..65762a91e3b8f 100644 --- a/contrib/pr/checkout.go +++ b/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" diff --git a/integrations/git_test.go b/integrations/git_test.go index d6bd673822ba9..f9e1eafb65833 100644 --- a/integrations/git_test.go +++ b/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)) diff --git a/integrations/integration_test.go b/integrations/integration_test.go index 230f780175c9c..5cd2392755b36 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -178,7 +178,6 @@ func initIntegrationTest() { if err := git.InitOnceWithSync(context.Background()); err != nil { log.Fatal("git.InitOnceWithSync: %v", err) } - git.CheckLFSVersion() setting.InitDBConfig() if err := storage.Init(); err != nil { diff --git a/integrations/lfs_getobject_test.go b/integrations/lfs_getobject_test.go index 4b6bb140d3adc..14a8ac253e2c4 100644 --- a/integrations/lfs_getobject_test.go +++ b/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 { diff --git a/integrations/migration-test/migration_test.go b/integrations/migration-test/migration_test.go index 20a5c903a92cc..64ee0f453ed3e 100644 --- a/integrations/migration-test/migration_test.go +++ b/integrations/migration-test/migration_test.go @@ -83,7 +83,6 @@ func initMigrationTest(t *testing.T) func() { } assert.NoError(t, git.InitOnceWithSync(context.Background())) - git.CheckLFSVersion() setting.InitDBConfig() setting.NewLogServices(true) return deferFn diff --git a/models/migrations/migrations_test.go b/models/migrations/migrations_test.go index 46782f24a1003..7c2782092657f 100644 --- a/models/migrations/migrations_test.go +++ b/models/migrations/migrations_test.go @@ -70,7 +70,6 @@ func TestMain(m *testing.M) { fmt.Printf("Unable to InitOnceWithSync: %v\n", err) os.Exit(1) } - git.CheckLFSVersion() setting.InitDBConfig() setting.NewLogServices(true) diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index 7f9af553747cb..215d74ce5dd30 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -123,8 +123,6 @@ func MainTest(m *testing.M, testOpts *TestOptions) { if err = git.InitOnceWithSync(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) diff --git a/modules/git/git.go b/modules/git/git.go index 3bc08ff93b5fa..7110270b90573 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -167,7 +167,7 @@ func InitSimple(ctx context.Context) error { var initOnce sync.Once // InitOnceWithSync initializes git module with version check and change global variables, sync gitconfig. -// This method will update the global variables ONLY ONCE (just like git.CheckLFSVersion -- which is not ideal too), +// This method will update the global variables ONLY ONCE (just like the old git.CheckLFSVersion -- which was not ideal too), // otherwise there will be data-race problem at the moment. func InitOnceWithSync(ctx context.Context) (err error) { if err = checkInit(); err != nil { @@ -200,6 +200,14 @@ func InitOnceWithSync(ctx context.Context) (err error) { } SupportProcReceive = CheckGitVersionAtLeast("2.29") == nil + + if setting.LFS.StartServer { + if CheckGitVersionAtLeast("2.1.2") != nil { + err = errors.New("LFS server support requires Git >= 2.1.2") + } else { + globalCommandArgs = append(globalCommandArgs, "-c", "filter.lfs.required=", "-c", "filter.lfs.smudge=", "-c", "filter.lfs.clean=") + } + } }) if err != nil { return err diff --git a/modules/git/lfs.go b/modules/git/lfs.go deleted file mode 100644 index c5d8354b6dc8c..0000000000000 --- a/modules/git/lfs.go +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2021 The Gitea Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. - -package git - -import ( - "sync" - - logger "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/setting" -) - -var once sync.Once - -// CheckLFSVersion will check lfs version, if not satisfied, then disable it. -func CheckLFSVersion() { - if setting.LFS.StartServer { - // Disable LFS client hooks if installed for the current OS user - // Needs at least git v2.1.2 - if CheckGitVersionAtLeast("2.1.2") != nil { - setting.LFS.StartServer = false - logger.Error("LFS server support needs at least Git v2.1.2") - } else { - once.Do(func() { - globalCommandArgs = append(globalCommandArgs, "-c", "filter.lfs.required=", - "-c", "filter.lfs.smudge=", "-c", "filter.lfs.clean=") - }) - } - } -} diff --git a/routers/init.go b/routers/init.go index 72ccf3526c1fb..656f8d839814f 100644 --- a/routers/init.go +++ b/routers/init.go @@ -102,8 +102,6 @@ func GlobalInitInstalled(ctx context.Context) { mustInitCtx(ctx, git.InitOnceWithSync) log.Info("Git Version: %s (home: %s)", git.VersionInfo(), git.HomeDir()) - - git.CheckLFSVersion() log.Info("AppPath: %s", setting.AppPath) log.Info("AppWorkPath: %s", setting.AppWorkPath) log.Info("Custom path: %s", setting.CustomPath) From 82ede4e7a1fce0a45799f533db011cc04ec682df Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 15 Jul 2022 10:09:53 +0800 Subject: [PATCH 2/6] refactor InitWithSync --- integrations/integration_test.go | 4 +- integrations/migration-test/migration_test.go | 2 +- models/migrations/migrations_test.go | 3 +- models/migrations/v219.go | 1 + models/unittest/testdb.go | 4 +- modules/doctor/mergebase.go | 2 +- modules/doctor/misc.go | 2 +- modules/git/git.go | 64 +++++++++---------- modules/git/git_test.go | 2 +- routers/init.go | 2 +- 10 files changed, 38 insertions(+), 48 deletions(-) diff --git a/integrations/integration_test.go b/integrations/integration_test.go index 5cd2392755b36..cca5b1d1fdd3d 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -175,7 +175,7 @@ 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.InitWithSync(context.Background()); err != nil { log.Fatal("git.InitOnceWithSync: %v", err) } @@ -284,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) @@ -585,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) diff --git a/integrations/migration-test/migration_test.go b/integrations/migration-test/migration_test.go index 64ee0f453ed3e..58352d3c58d75 100644 --- a/integrations/migration-test/migration_test.go +++ b/integrations/migration-test/migration_test.go @@ -82,7 +82,7 @@ func initMigrationTest(t *testing.T) func() { } } - assert.NoError(t, git.InitOnceWithSync(context.Background())) + assert.NoError(t, git.InitWithSync(context.Background())) setting.InitDBConfig() setting.NewLogServices(true) return deferFn diff --git a/models/migrations/migrations_test.go b/models/migrations/migrations_test.go index 7c2782092657f..fc50af7d4f5a5 100644 --- a/models/migrations/migrations_test.go +++ b/models/migrations/migrations_test.go @@ -66,7 +66,7 @@ func TestMain(m *testing.M) { setting.SetCustomPathAndConf("", "", "") setting.LoadForTest() - if err = git.InitOnceWithSync(context.Background()); err != nil { + if err = git.InitWithSync(context.Background()); err != nil { fmt.Printf("Unable to InitOnceWithSync: %v\n", err) os.Exit(1) } @@ -206,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) diff --git a/models/migrations/v219.go b/models/migrations/v219.go index 7b2eaa3292704..7b4f34b704033 100644 --- a/models/migrations/v219.go +++ b/models/migrations/v219.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/timeutil" + "xorm.io/xorm" ) diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index 215d74ce5dd30..1e79505ea84a1 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -120,7 +120,7 @@ func MainTest(m *testing.M, testOpts *TestOptions) { fatalTestError("util.CopyDir: %v\n", err) } - if err = git.InitOnceWithSync(context.Background()); err != nil { + if err = git.InitWithSync(context.Background()); err != nil { fatalTestError("git.Init: %v\n", err) } ownerDirs, err := os.ReadDir(setting.RepoRootPath) @@ -204,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 { diff --git a/modules/doctor/mergebase.go b/modules/doctor/mergebase.go index 2da91cdcc35f6..49af97bea6136 100644 --- a/modules/doctor/mergebase.go +++ b/modules/doctor/mergebase.go @@ -30,7 +30,7 @@ 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 { + if err := git.InitWithSync(ctx); err != nil { return err } numRepos := 0 diff --git a/modules/doctor/misc.go b/modules/doctor/misc.go index 24175fcaf4bec..1d9cc9b9b7a95 100644 --- a/modules/doctor/misc.go +++ b/modules/doctor/misc.go @@ -190,7 +190,7 @@ 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 { + if err := git.InitWithSync(ctx); err != nil { return err } diff --git a/modules/git/git.go b/modules/git/git.go index 7110270b90573..4cbcb0eda2602 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -15,11 +15,11 @@ import ( "regexp" "runtime" "strings" - "sync" "time" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "github.com/hashicorp/go-version" ) @@ -156,6 +156,7 @@ func InitSimple(ctx context.Context) error { } DefaultContext = ctx + globalCommandArgs = nil if setting.Git.Timeout.Default > 0 { defaultCommandExecutionTimeout = time.Duration(setting.Git.Timeout.Default) * time.Second @@ -164,51 +165,44 @@ func InitSimple(ctx context.Context) error { return SetExecutablePath(setting.Git.Path) } -var initOnce sync.Once - -// InitOnceWithSync initializes git module with version check and change global variables, sync gitconfig. -// This method will update the global variables ONLY ONCE (just like the old git.CheckLFSVersion -- which was not ideal too), -// otherwise there will be data-race problem at the moment. -func InitOnceWithSync(ctx context.Context) (err error) { +// InitWithSync initializes git module with version check and change global variables, sync gitconfig. +func InitWithSync(ctx context.Context) (err error) { if err = checkInit(); err != nil { return err } - initOnce.Do(func() { - if err = InitSimple(ctx); err != nil { - return - } + if err = InitSimple(ctx); err != nil { + return + } - // when git works with gnupg (commit signing), there should be a stable home for gnupg commands - if _, ok := os.LookupEnv("GNUPGHOME"); !ok { - _ = os.Setenv("GNUPGHOME", filepath.Join(HomeDir(), ".gnupg")) - } + // when git works with gnupg (commit signing), there should be a stable home for gnupg commands + if _, ok := os.LookupEnv("GNUPGHOME"); !ok { + _ = os.Setenv("GNUPGHOME", filepath.Join(HomeDir(), ".gnupg")) + } - // Since git wire protocol has been released from git v2.18 - if setting.Git.EnableAutoGitWireProtocol && CheckGitVersionAtLeast("2.18") == nil { - globalCommandArgs = append(globalCommandArgs, "-c", "protocol.version=2") - } + // Since git wire protocol has been released from git v2.18 + if setting.Git.EnableAutoGitWireProtocol && CheckGitVersionAtLeast("2.18") == nil { + globalCommandArgs = append(globalCommandArgs, "-c", "protocol.version=2") + } - // By default partial clones are disabled, enable them from git v2.22 - if !setting.Git.DisablePartialClone && CheckGitVersionAtLeast("2.22") == nil { - globalCommandArgs = append(globalCommandArgs, "-c", "uploadpack.allowfilter=true", "-c", "uploadpack.allowAnySHA1InWant=true") - } + // By default partial clones are disabled, enable them from git v2.22 + if !setting.Git.DisablePartialClone && CheckGitVersionAtLeast("2.22") == nil { + globalCommandArgs = append(globalCommandArgs, "-c", "uploadpack.allowfilter=true", "-c", "uploadpack.allowAnySHA1InWant=true") + } - // Explicitly disable credential helper, otherwise Git credentials might leak - if CheckGitVersionAtLeast("2.9") == nil { - globalCommandArgs = append(globalCommandArgs, "-c", "credential.helper=") - } + // Explicitly disable credential helper, otherwise Git credentials might leak + if CheckGitVersionAtLeast("2.9") == nil { + globalCommandArgs = append(globalCommandArgs, "-c", "credential.helper=") + } - SupportProcReceive = CheckGitVersionAtLeast("2.29") == nil + SupportProcReceive = CheckGitVersionAtLeast("2.29") == nil - if setting.LFS.StartServer { - if CheckGitVersionAtLeast("2.1.2") != nil { - err = errors.New("LFS server support requires Git >= 2.1.2") - } else { - globalCommandArgs = append(globalCommandArgs, "-c", "filter.lfs.required=", "-c", "filter.lfs.smudge=", "-c", "filter.lfs.clean=") - } + if setting.LFS.StartServer { + if CheckGitVersionAtLeast("2.1.2") != nil { + return errors.New("LFS server support requires Git >= 2.1.2") } - }) + globalCommandArgs = append(globalCommandArgs, "-c", "filter.lfs.required=", "-c", "filter.lfs.smudge=", "-c", "filter.lfs.clean=") + } if err != nil { return err } diff --git a/modules/git/git_test.go b/modules/git/git_test.go index c5a63de0644c0..d25b453811cdd 100644 --- a/modules/git/git_test.go +++ b/modules/git/git_test.go @@ -28,7 +28,7 @@ func testRun(m *testing.M) error { defer util.RemoveAll(gitHomePath) setting.Git.HomePath = gitHomePath - if err = InitOnceWithSync(context.Background()); err != nil { + if err = InitWithSync(context.Background()); err != nil { return fmt.Errorf("failed to call Init: %w", err) } diff --git a/routers/init.go b/routers/init.go index 656f8d839814f..f64fb059f4fba 100644 --- a/routers/init.go +++ b/routers/init.go @@ -100,7 +100,7 @@ func GlobalInitInstalled(ctx context.Context) { log.Fatal("Gitea is not installed") } - mustInitCtx(ctx, git.InitOnceWithSync) + mustInitCtx(ctx, git.InitWithSync) log.Info("Git Version: %s (home: %s)", git.VersionInfo(), git.HomeDir()) log.Info("AppPath: %s", setting.AppPath) log.Info("AppWorkPath: %s", setting.AppWorkPath) From 711373a47b4752cdd73fec2b73ad9b27e5337fcb Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 15 Jul 2022 10:31:30 +0800 Subject: [PATCH 3/6] fine tune --- modules/git/git.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/modules/git/git.go b/modules/git/git.go index 4cbcb0eda2602..ee39784c3270c 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -23,8 +23,8 @@ import ( "github.com/hashicorp/go-version" ) -// GitVersionRequired is the minimum Git version required -const GitVersionRequired = "2.0.0" +// RequiredVersion is the minimum Git version required +const RequiredVersion = "2.0.0" var ( // GitExecutable is the command name of git @@ -42,7 +42,7 @@ var ( // loadGitVersion returns current Git version from shell. Internal usage only. func loadGitVersion() (*version.Version, error) { - // doesn't need RWMutex because its exec by Init() + // doesn't need RWMutex because it's executed by Init() if gitVersion != nil { return gitVersion, nil } @@ -89,7 +89,7 @@ func SetExecutablePath(path string) error { return fmt.Errorf("unable to load git version: %w", err) } - versionRequired, err := version.NewVersion(GitVersionRequired) + versionRequired, err := version.NewVersion(RequiredVersion) if err != nil { return err } @@ -103,7 +103,7 @@ func SetExecutablePath(path string) error { moreHint = "get git: https://git-scm.com/download/linux and https://ius.io" } } - return fmt.Errorf("installed git version %q is not supported, Gitea requires git version >= %q, %s", gitVersion.Original(), GitVersionRequired, moreHint) + return fmt.Errorf("installed git version %q is not supported, Gitea requires git version >= %q, %s", gitVersion.Original(), RequiredVersion, moreHint) } return nil @@ -130,7 +130,7 @@ func checkInit() error { return errors.New("unable to init Git's HomeDir, incorrect initialization of the setting and git modules") } if DefaultContext != nil { - log.Warn("git module has been initialized already, duplicate init should be fixed") + log.Warn("git module has been initialized already, duplicate init may work but it's better to fix it") } return nil } @@ -139,7 +139,7 @@ func checkInit() error { func HomeDir() string { if setting.Git.HomePath == "" { // strict check, make sure the git module is initialized correctly. - // attention: when the git module is called in gitea sub-command (serv/hook), the log module is not able to show messages to users. + // attention: when the git module is called in gitea sub-command (serv/hook), the log module might not obviously show messages to users/developers. // for example: if there is gitea git hook code calling git.NewCommand before git.InitXxx, the integration test won't show the real failure reasons. log.Fatal("Unable to init Git's HomeDir, incorrect initialization of the setting and git modules") return "" @@ -148,8 +148,7 @@ func HomeDir() string { } // InitSimple initializes git module with a very simple step, no config changes, no global command arguments. -// This method doesn't change anything to filesystem. At the moment, it is only used by "git serv" sub-command, no data-race -// However, in integration test, the sub-command function may be called in the current process, so the InitSimple would be called multiple times, too +// This method doesn't change anything to filesystem. At the moment, it is only used by some Gitea sub-commands. func InitSimple(ctx context.Context) error { if err := checkInit(); err != nil { return err @@ -166,6 +165,7 @@ func InitSimple(ctx context.Context) error { } // InitWithSync initializes git module with version check and change global variables, sync gitconfig. +// It should only be called at the beginning of the program initialization (TestMain/GlobalInitInstalled), otherwise there will be data-race problem. func InitWithSync(ctx context.Context) (err error) { if err = checkInit(); err != nil { return err @@ -203,9 +203,7 @@ func InitWithSync(ctx context.Context) (err error) { } globalCommandArgs = append(globalCommandArgs, "-c", "filter.lfs.required=", "-c", "filter.lfs.smudge=", "-c", "filter.lfs.clean=") } - if err != nil { - return err - } + return syncGitConfig() } From 2c99f9a608bc15f9174d040f70565684a81839ff Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 15 Jul 2022 10:47:15 +0800 Subject: [PATCH 4/6] InitWithSync should only be called in initailization stage. --- cmd/doctor.go | 12 +++++++++--- modules/doctor/mergebase.go | 3 --- modules/doctor/misc.go | 4 ---- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 3f16c6e2a600a..59cd3f635f846 100644 --- a/cmd/doctor.go +++ b/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.InitWithSync(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") { diff --git a/modules/doctor/mergebase.go b/modules/doctor/mergebase.go index 49af97bea6136..46369290a13d7 100644 --- a/modules/doctor/mergebase.go +++ b/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.InitWithSync(ctx); err != nil { - return err - } numRepos := 0 numPRs := 0 numPRsUpdated := 0 diff --git a/modules/doctor/misc.go b/modules/doctor/misc.go index 1d9cc9b9b7a95..2d2bcb910db4c 100644 --- a/modules/doctor/misc.go +++ b/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.InitWithSync(ctx); err != nil { - return err - } - numRepos := 0 numNeedUpdate := 0 numWritten := 0 From f52dc7445205f7e199be51995a7d3ea41c6d6bdf Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 23 Jul 2022 20:13:44 +0800 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: zeripath --- models/migrations/migrations_test.go | 2 +- modules/git/git.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/models/migrations/migrations_test.go b/models/migrations/migrations_test.go index fc50af7d4f5a5..3a2aa33586af8 100644 --- a/models/migrations/migrations_test.go +++ b/models/migrations/migrations_test.go @@ -67,7 +67,7 @@ func TestMain(m *testing.M) { setting.SetCustomPathAndConf("", "", "") setting.LoadForTest() if err = git.InitWithSync(context.Background()); err != nil { - fmt.Printf("Unable to InitOnceWithSync: %v\n", err) + fmt.Printf("Unable to InitWithSync: %v\n", err) os.Exit(1) } setting.InitDBConfig() diff --git a/modules/git/git.go b/modules/git/git.go index ee39784c3270c..57766b2c8bcc1 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -165,7 +165,7 @@ func InitSimple(ctx context.Context) error { } // InitWithSync initializes git module with version check and change global variables, sync gitconfig. -// It should only be called at the beginning of the program initialization (TestMain/GlobalInitInstalled), otherwise there will be data-race problem. +// It should only be called once at the beginning of the program initialization (TestMain/GlobalInitInstalled) as this code makes unsynchronized changes to variables. func InitWithSync(ctx context.Context) (err error) { if err = checkInit(); err != nil { return err From 48d52fab583077747494e6aab67f2808250f3597 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 23 Jul 2022 20:17:18 +0800 Subject: [PATCH 6/6] InitWithSync -> InitFull --- cmd/doctor.go | 2 +- integrations/integration_test.go | 2 +- integrations/migration-test/migration_test.go | 2 +- models/migrations/migrations_test.go | 4 ++-- models/unittest/testdb.go | 2 +- modules/git/git.go | 4 ++-- modules/git/git_test.go | 2 +- routers/init.go | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 59cd3f635f846..1a15dd2941c57 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -129,7 +129,7 @@ func runDoctor(ctx *cli.Context) error { defer cancel() // some doctor sub-commands need to use git command - if err := git.InitWithSync(stdCtx); err != nil { + if err := git.InitFull(stdCtx); err != nil { return err } diff --git a/integrations/integration_test.go b/integrations/integration_test.go index cca5b1d1fdd3d..3c379f5c84ef9 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -175,7 +175,7 @@ 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.InitWithSync(context.Background()); err != nil { + if err := git.InitFull(context.Background()); err != nil { log.Fatal("git.InitOnceWithSync: %v", err) } diff --git a/integrations/migration-test/migration_test.go b/integrations/migration-test/migration_test.go index 58352d3c58d75..80093d66f1ed8 100644 --- a/integrations/migration-test/migration_test.go +++ b/integrations/migration-test/migration_test.go @@ -82,7 +82,7 @@ func initMigrationTest(t *testing.T) func() { } } - assert.NoError(t, git.InitWithSync(context.Background())) + assert.NoError(t, git.InitFull(context.Background())) setting.InitDBConfig() setting.NewLogServices(true) return deferFn diff --git a/models/migrations/migrations_test.go b/models/migrations/migrations_test.go index 3a2aa33586af8..53e4f35395640 100644 --- a/models/migrations/migrations_test.go +++ b/models/migrations/migrations_test.go @@ -66,8 +66,8 @@ func TestMain(m *testing.M) { setting.SetCustomPathAndConf("", "", "") setting.LoadForTest() - if err = git.InitWithSync(context.Background()); err != nil { - fmt.Printf("Unable to InitWithSync: %v\n", err) + if err = git.InitFull(context.Background()); err != nil { + fmt.Printf("Unable to InitFull: %v\n", err) os.Exit(1) } setting.InitDBConfig() diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index 1e79505ea84a1..58656f781f137 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -120,7 +120,7 @@ func MainTest(m *testing.M, testOpts *TestOptions) { fatalTestError("util.CopyDir: %v\n", err) } - if err = git.InitWithSync(context.Background()); err != nil { + if err = git.InitFull(context.Background()); err != nil { fatalTestError("git.Init: %v\n", err) } ownerDirs, err := os.ReadDir(setting.RepoRootPath) diff --git a/modules/git/git.go b/modules/git/git.go index 57766b2c8bcc1..99849f1f09457 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -164,9 +164,9 @@ func InitSimple(ctx context.Context) error { return SetExecutablePath(setting.Git.Path) } -// InitWithSync initializes git module with version check and change global variables, sync gitconfig. +// InitFull initializes git module with version check and change global variables, sync gitconfig. // It should only be called once at the beginning of the program initialization (TestMain/GlobalInitInstalled) as this code makes unsynchronized changes to variables. -func InitWithSync(ctx context.Context) (err error) { +func InitFull(ctx context.Context) (err error) { if err = checkInit(); err != nil { return err } diff --git a/modules/git/git_test.go b/modules/git/git_test.go index d25b453811cdd..091573787871f 100644 --- a/modules/git/git_test.go +++ b/modules/git/git_test.go @@ -28,7 +28,7 @@ func testRun(m *testing.M) error { defer util.RemoveAll(gitHomePath) setting.Git.HomePath = gitHomePath - if err = InitWithSync(context.Background()); err != nil { + if err = InitFull(context.Background()); err != nil { return fmt.Errorf("failed to call Init: %w", err) } diff --git a/routers/init.go b/routers/init.go index f64fb059f4fba..91a3d2f30415f 100644 --- a/routers/init.go +++ b/routers/init.go @@ -100,7 +100,7 @@ func GlobalInitInstalled(ctx context.Context) { log.Fatal("Gitea is not installed") } - mustInitCtx(ctx, git.InitWithSync) + mustInitCtx(ctx, git.InitFull) log.Info("Git Version: %s (home: %s)", git.VersionInfo(), git.HomeDir()) log.Info("AppPath: %s", setting.AppPath) log.Info("AppWorkPath: %s", setting.AppWorkPath)