From db0d7a16a3e6efe8d6ba6eeed4fb1cf1e5d0f5f2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 10 Jun 2022 10:17:41 +0800 Subject: [PATCH 01/10] clean git support for ver < 2.0 --- modules/git/commit.go | 27 ++++-------- modules/git/repo_attribute.go | 26 ++++-------- modules/git/repo_compare.go | 8 +--- modules/git/repo_language_stats_gogit.go | 44 ++++++++++--------- modules/git/repo_language_stats_nogogit.go | 48 ++++++++++----------- modules/git/repo_tree.go | 4 +- modules/repository/init.go | 22 +++++----- services/gitdiff/gitdiff.go | 48 ++++++++++----------- services/pull/merge.go | 27 ++++-------- services/repository/files/temp_repo.go | 49 ++++++++++------------ 10 files changed, 130 insertions(+), 173 deletions(-) diff --git a/modules/git/commit.go b/modules/git/commit.go index 99fbbd0cfcb1e..82b5e0b25d082 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -206,26 +206,17 @@ func (c *Commit) HasPreviousCommit(commitHash SHA1) (bool, error) { return false, nil } - if err := CheckGitVersionAtLeast("1.8"); err == nil { - _, _, err := NewCommand(c.repo.Ctx, "merge-base", "--is-ancestor", that, this).RunStdString(&RunOpts{Dir: c.repo.Path}) - if err == nil { - return true, nil + _, _, err := NewCommand(c.repo.Ctx, "merge-base", "--is-ancestor", that, this).RunStdString(&RunOpts{Dir: c.repo.Path}) + if err == nil { + return true, nil + } + var exitError *exec.ExitError + if errors.As(err, &exitError) { + if exitError.ProcessState.ExitCode() == 1 && len(exitError.Stderr) == 0 { + return false, nil } - var exitError *exec.ExitError - if errors.As(err, &exitError) { - if exitError.ProcessState.ExitCode() == 1 && len(exitError.Stderr) == 0 { - return false, nil - } - } - return false, err - } - - result, _, err := NewCommand(c.repo.Ctx, "rev-list", "--ancestry-path", "-n1", that+".."+this, "--").RunStdString(&RunOpts{Dir: c.repo.Path}) - if err != nil { - return false, err } - - return len(strings.TrimSpace(result)) > 0, nil + return false, err } // CommitsBeforeLimit returns num commits before current revision diff --git a/modules/git/repo_attribute.go b/modules/git/repo_attribute.go index 38818788f32a3..7cce462fab0de 100644 --- a/modules/git/repo_attribute.go +++ b/modules/git/repo_attribute.go @@ -30,10 +30,10 @@ type CheckAttributeOpts struct { func (repo *Repository) CheckAttribute(opts CheckAttributeOpts) (map[string]map[string]string, error) { env := []string{} - if len(opts.IndexFile) > 0 && CheckGitVersionAtLeast("1.7.8") == nil { + if len(opts.IndexFile) > 0 { env = append(env, "GIT_INDEX_FILE="+opts.IndexFile) } - if len(opts.WorkTree) > 0 && CheckGitVersionAtLeast("1.7.8") == nil { + if len(opts.WorkTree) > 0 { env = append(env, "GIT_WORK_TREE="+opts.WorkTree) } @@ -56,8 +56,7 @@ func (repo *Repository) CheckAttribute(opts CheckAttributeOpts) (map[string]map[ } } - // git check-attr --cached first appears in git 1.7.8 - if opts.CachedOnly && CheckGitVersionAtLeast("1.7.8") == nil { + if opts.CachedOnly { cmdArgs = append(cmdArgs, "--cached") } @@ -125,12 +124,12 @@ type CheckAttributeReader struct { func (c *CheckAttributeReader) Init(ctx context.Context) error { cmdArgs := []string{"check-attr", "--stdin", "-z"} - if len(c.IndexFile) > 0 && CheckGitVersionAtLeast("1.7.8") == nil { + if len(c.IndexFile) > 0 { cmdArgs = append(cmdArgs, "--cached") c.env = append(c.env, "GIT_INDEX_FILE="+c.IndexFile) } - if len(c.WorkTree) > 0 && CheckGitVersionAtLeast("1.7.8") == nil { + if len(c.WorkTree) > 0 { c.env = append(c.env, "GIT_WORK_TREE="+c.WorkTree) } @@ -160,17 +159,10 @@ func (c *CheckAttributeReader) Init(ctx context.Context) error { return err } - if CheckGitVersionAtLeast("1.8.5") == nil { - lw := new(nulSeparatedAttributeWriter) - lw.attributes = make(chan attributeTriple, 5) - lw.closed = make(chan struct{}) - c.stdOut = lw - } else { - lw := new(lineSeparatedAttributeWriter) - lw.attributes = make(chan attributeTriple, 5) - lw.closed = make(chan struct{}) - c.stdOut = lw - } + lw := new(nulSeparatedAttributeWriter) + lw.attributes = make(chan attributeTriple, 5) + lw.closed = make(chan struct{}) + c.stdOut = lw return nil } diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 4b0cc8536b673..3c7af73000e45 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -255,13 +255,7 @@ func (repo *Repository) GetDiff(base, head string, w io.Writer) error { // GetDiffBinary generates and returns patch data between given revisions, including binary diffs. func (repo *Repository) GetDiffBinary(base, head string, w io.Writer) error { - if CheckGitVersionAtLeast("1.7.7") == nil { - return NewCommand(repo.Ctx, "diff", "-p", "--binary", "--histogram", base, head).Run(&RunOpts{ - Dir: repo.Path, - Stdout: w, - }) - } - return NewCommand(repo.Ctx, "diff", "-p", "--binary", "--patience", base, head).Run(&RunOpts{ + return NewCommand(repo.Ctx, "diff", "-p", "--binary", "--histogram", base, head).Run(&RunOpts{ Dir: repo.Path, Stdout: w, }) diff --git a/modules/git/repo_language_stats_gogit.go b/modules/git/repo_language_stats_gogit.go index 3c9f026b7ace2..0cc36a16ad495 100644 --- a/modules/git/repo_language_stats_gogit.go +++ b/modules/git/repo_language_stats_gogit.go @@ -45,30 +45,28 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err var checker *CheckAttributeReader - if CheckGitVersionAtLeast("1.7.8") == nil { - indexFilename, workTree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID) - if err == nil { - defer deleteTemporaryFile() - checker = &CheckAttributeReader{ - Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, - Repo: repo, - IndexFile: indexFilename, - WorkTree: workTree, - } - ctx, cancel := context.WithCancel(DefaultContext) - if err := checker.Init(ctx); err != nil { - log.Error("Unable to open checker for %s. Error: %v", commitID, err) - } else { - go func() { - err = checker.Run() - if err != nil { - log.Error("Unable to open checker for %s. Error: %v", commitID, err) - cancel() - } - }() - } - defer cancel() + indexFilename, workTree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID) + if err == nil { + defer deleteTemporaryFile() + checker = &CheckAttributeReader{ + Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, + Repo: repo, + IndexFile: indexFilename, + WorkTree: workTree, + } + ctx, cancel := context.WithCancel(DefaultContext) + if err := checker.Init(ctx); err != nil { + log.Error("Unable to open checker for %s. Error: %v", commitID, err) + } else { + go func() { + err = checker.Run() + if err != nil { + log.Error("Unable to open checker for %s. Error: %v", commitID, err) + cancel() + } + }() } + defer cancel() } sizes := make(map[string]int64) diff --git a/modules/git/repo_language_stats_nogogit.go b/modules/git/repo_language_stats_nogogit.go index 41b176f816912..4ca0ba3b53503 100644 --- a/modules/git/repo_language_stats_nogogit.go +++ b/modules/git/repo_language_stats_nogogit.go @@ -65,33 +65,31 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err var checker *CheckAttributeReader - if CheckGitVersionAtLeast("1.7.8") == nil { - indexFilename, worktree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID) - if err == nil { - defer deleteTemporaryFile() - checker = &CheckAttributeReader{ - Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, - Repo: repo, - IndexFile: indexFilename, - WorkTree: worktree, - } - ctx, cancel := context.WithCancel(repo.Ctx) - if err := checker.Init(ctx); err != nil { - log.Error("Unable to open checker for %s. Error: %v", commitID, err) - } else { - go func() { - err = checker.Run() - if err != nil { - log.Error("Unable to open checker for %s. Error: %v", commitID, err) - cancel() - } - }() - } - defer func() { - _ = checker.Close() - cancel() + indexFilename, worktree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID) + if err == nil { + defer deleteTemporaryFile() + checker = &CheckAttributeReader{ + Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, + Repo: repo, + IndexFile: indexFilename, + WorkTree: worktree, + } + ctx, cancel := context.WithCancel(repo.Ctx) + if err := checker.Init(ctx); err != nil { + log.Error("Unable to open checker for %s. Error: %v", commitID, err) + } else { + go func() { + err = checker.Run() + if err != nil { + log.Error("Unable to open checker for %s. Error: %v", commitID, err) + cancel() + } }() } + defer func() { + _ = checker.Close() + cancel() + }() } contentBuf := bytes.Buffer{} diff --git a/modules/git/repo_tree.go b/modules/git/repo_tree.go index 2e139daddfc72..2ea3f0187a185 100644 --- a/modules/git/repo_tree.go +++ b/modules/git/repo_tree.go @@ -45,11 +45,11 @@ func (repo *Repository) CommitTree(author, committer *Signature, tree *Tree, opt _, _ = messageBytes.WriteString(opts.Message) _, _ = messageBytes.WriteString("\n") - if CheckGitVersionAtLeast("1.7.9") == nil && (opts.KeyID != "" || opts.AlwaysSign) { + if opts.KeyID != "" || opts.AlwaysSign { cmd.AddArguments(fmt.Sprintf("-S%s", opts.KeyID)) } - if CheckGitVersionAtLeast("2.0.0") == nil && opts.NoGPGSign { + if opts.NoGPGSign { cmd.AddArguments("--no-gpg-sign") } diff --git a/modules/repository/init.go b/modules/repository/init.go index 285fe81dbe86f..db2f66286257f 100644 --- a/modules/repository/init.go +++ b/modules/repository/init.go @@ -322,19 +322,17 @@ func initRepoCommit(ctx context.Context, tmpPath string, repo *repo_model.Reposi "-m", "Initial commit", } - if git.CheckGitVersionAtLeast("1.7.9") == nil { - sign, keyID, signer, _ := asymkey_service.SignInitialCommit(ctx, tmpPath, u) - if sign { - args = append(args, "-S"+keyID) - - if repo.GetTrustModel() == repo_model.CommitterTrustModel || repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { - // need to set the committer to the KeyID owner - committerName = signer.Name - committerEmail = signer.Email - } - } else if git.CheckGitVersionAtLeast("2.0.0") == nil { - args = append(args, "--no-gpg-sign") + sign, keyID, signer, _ := asymkey_service.SignInitialCommit(ctx, tmpPath, u) + if sign { + args = append(args, "-S"+keyID) + + if repo.GetTrustModel() == repo_model.CommitterTrustModel || repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { + // need to set the committer to the KeyID owner + committerName = signer.Name + committerEmail = signer.Email } + } else { + args = append(args, "--no-gpg-sign") } env = append(env, diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index d611ff513e989..36ce943bcca29 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1418,34 +1418,32 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff var checker *git.CheckAttributeReader - if git.CheckGitVersionAtLeast("1.7.8") == nil { - indexFilename, worktree, deleteTemporaryFile, err := gitRepo.ReadTreeToTemporaryIndex(opts.AfterCommitID) - if err == nil { - defer deleteTemporaryFile() - - checker = &git.CheckAttributeReader{ - Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, - Repo: gitRepo, - IndexFile: indexFilename, - WorkTree: worktree, - } - ctx, cancel := context.WithCancel(gitRepo.Ctx) - if err := checker.Init(ctx); err != nil { - log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) - } else { - go func() { - err := checker.Run() - if err != nil && err != ctx.Err() { - log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) - } - cancel() - }() - } - defer func() { - _ = checker.Close() + indexFilename, worktree, deleteTemporaryFile, err := gitRepo.ReadTreeToTemporaryIndex(opts.AfterCommitID) + if err == nil { + defer deleteTemporaryFile() + + checker = &git.CheckAttributeReader{ + Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, + Repo: gitRepo, + IndexFile: indexFilename, + WorkTree: worktree, + } + ctx, cancel := context.WithCancel(gitRepo.Ctx) + if err := checker.Init(ctx); err != nil { + log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) + } else { + go func() { + err := checker.Run() + if err != nil && err != ctx.Err() { + log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) + } cancel() }() } + defer func() { + _ = checker.Close() + cancel() + }() } for _, diffFile := range diff.Files { diff --git a/services/pull/merge.go b/services/pull/merge.go index 76798d73edc28..d3c5578306c23 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -274,15 +274,8 @@ func rawMerge(ctx context.Context, pr *models.PullRequest, doer *user_model.User return "", fmt.Errorf("Unable to write .git/info/sparse-checkout file in tmpBasePath: %v", err) } - var gitConfigCommand func() *git.Command - if git.CheckGitVersionAtLeast("1.8.0") == nil { - gitConfigCommand = func() *git.Command { - return git.NewCommand(ctx, "config", "--local") - } - } else { - gitConfigCommand = func() *git.Command { - return git.NewCommand(ctx, "config") - } + gitConfigCommand := func() *git.Command { + return git.NewCommand(ctx, "config", "--local") } // Switch off LFS process (set required, clean and smudge here also) @@ -364,16 +357,14 @@ func rawMerge(ctx context.Context, pr *models.PullRequest, doer *user_model.User // Determine if we should sign signArg := "" - if git.CheckGitVersionAtLeast("1.7.9") == nil { - sign, keyID, signer, _ := asymkey_service.SignMerge(ctx, pr, doer, tmpBasePath, "HEAD", trackingBranch) - if sign { - signArg = "-S" + keyID - if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { - committer = signer - } - } else if git.CheckGitVersionAtLeast("2.0.0") == nil { - signArg = "--no-gpg-sign" + sign, keyID, signer, _ := asymkey_service.SignMerge(ctx, pr, doer, tmpBasePath, "HEAD", trackingBranch) + if sign { + signArg = "-S" + keyID + if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { + committer = signer } + } else { + signArg = "--no-gpg-sign" } commitTimeStr := time.Now().Format(time.RFC3339) diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index 97a80a96bd6ab..1e60d55613fa0 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -248,34 +248,31 @@ func (t *TemporaryUploadRepository) CommitTreeWithDate(parent string, author, co args = []string{"commit-tree", treeHash} } - // Determine if we should sign - if git.CheckGitVersionAtLeast("1.7.9") == nil { - var sign bool - var keyID string - var signer *git.Signature - if parent != "" { - sign, keyID, signer, _ = asymkey_service.SignCRUDAction(t.ctx, t.repo.RepoPath(), author, t.basePath, parent) - } else { - sign, keyID, signer, _ = asymkey_service.SignInitialCommit(t.ctx, t.repo.RepoPath(), author) - } - if sign { - args = append(args, "-S"+keyID) - if t.repo.GetTrustModel() == repo_model.CommitterTrustModel || t.repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { - if committerSig.Name != authorSig.Name || committerSig.Email != authorSig.Email { - // Add trailers - _, _ = messageBytes.WriteString("\n") - _, _ = messageBytes.WriteString("Co-authored-by: ") - _, _ = messageBytes.WriteString(committerSig.String()) - _, _ = messageBytes.WriteString("\n") - _, _ = messageBytes.WriteString("Co-committed-by: ") - _, _ = messageBytes.WriteString(committerSig.String()) - _, _ = messageBytes.WriteString("\n") - } - committerSig = signer + var sign bool + var keyID string + var signer *git.Signature + if parent != "" { + sign, keyID, signer, _ = asymkey_service.SignCRUDAction(t.ctx, t.repo.RepoPath(), author, t.basePath, parent) + } else { + sign, keyID, signer, _ = asymkey_service.SignInitialCommit(t.ctx, t.repo.RepoPath(), author) + } + if sign { + args = append(args, "-S"+keyID) + if t.repo.GetTrustModel() == repo_model.CommitterTrustModel || t.repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { + if committerSig.Name != authorSig.Name || committerSig.Email != authorSig.Email { + // Add trailers + _, _ = messageBytes.WriteString("\n") + _, _ = messageBytes.WriteString("Co-authored-by: ") + _, _ = messageBytes.WriteString(committerSig.String()) + _, _ = messageBytes.WriteString("\n") + _, _ = messageBytes.WriteString("Co-committed-by: ") + _, _ = messageBytes.WriteString(committerSig.String()) + _, _ = messageBytes.WriteString("\n") } - } else if git.CheckGitVersionAtLeast("2.0.0") == nil { - args = append(args, "--no-gpg-sign") + committerSig = signer } + } else { + args = append(args, "--no-gpg-sign") } if signoff { From d01ca3a8caa4bd8bcd448418b8ae0d62a455be34 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 10 Jun 2022 10:56:11 +0800 Subject: [PATCH 02/10] fine tune tests for markup (which requires git module) --- modules/git/git.go | 9 ++------- modules/markup/html_test.go | 9 +++++++++ modules/markup/markdown/markdown_test.go | 8 ++++++++ 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/modules/git/git.go b/modules/git/git.go index 5817bd2c7fe2d..2acc612de0e1f 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -10,7 +10,6 @@ import ( "fmt" "os" "os/exec" - "path/filepath" "runtime" "strings" "sync" @@ -143,12 +142,8 @@ func InitSimple(ctx context.Context) error { // HomeDir is the home dir for git to store the global config file used by Gitea internally func HomeDir() string { if setting.RepoRootPath == "" { - // TODO: now, some unit test code call the git module directly without initialization, which is incorrect. - // at the moment, we just use a temp HomeDir to prevent from conflicting with user's git config - // in the future, the git module should be initialized first before use. - tmpHomeDir := filepath.Join(os.TempDir(), "gitea-temp-home") - log.Error("Git's HomeDir is empty (RepoRootPath is empty), the git module is not initialized correctly, using a temp HomeDir (%s) temporarily", tmpHomeDir) - return tmpHomeDir + log.Fatal("Can not get Git's HomeDir (RepoRootPath is empty), the setting and git modules are not initialized correctly") + return "" } return setting.RepoRootPath } diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index f6aabc6272e4d..81b0c4e4406aa 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -5,12 +5,14 @@ package markup_test import ( + "context" "io" "strings" "testing" "code.gitea.io/gitea/modules/emoji" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" . "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/setting" @@ -25,6 +27,13 @@ var localMetas = map[string]string{ "repoPath": "../../integrations/gitea-repositories-meta/user13/repo11.git/", } +func TestMain(m *testing.M) { + setting.LoadAllowEmpty() + if err := git.InitSimple(context.Background()); err != nil { + log.Fatal("git init failed, err: %v", err) + } +} + func TestRender_Commits(t *testing.T) { setting.AppURL = TestAppURL test := func(input, expected string) { diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index a069d402bb506..732fe1a6beff9 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -5,6 +5,7 @@ package markdown_test import ( + "context" "strings" "testing" @@ -31,6 +32,13 @@ var localMetas = map[string]string{ "repoPath": "../../../integrations/gitea-repositories-meta/user13/repo11.git/", } +func TestMain(m *testing.M) { + setting.LoadAllowEmpty() + if err := git.InitSimple(context.Background()); err != nil { + log.Fatal("git init failed, err: %v", err) + } +} + func TestRender_StandardLinks(t *testing.T) { setting.AppURL = AppURL setting.AppSubURL = AppSubURL From 5e8c6b4767b84e2d870c8302a72fab2ad08a7583 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 10 Jun 2022 11:16:15 +0800 Subject: [PATCH 03/10] remove unnecessary comments --- modules/git/git.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/modules/git/git.go b/modules/git/git.go index 2acc612de0e1f..bf5f328e78356 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -23,9 +23,6 @@ import ( var ( // GitVersionRequired is the minimum Git version required - // At the moment, all code for git 1.x are not changed, if some users want to test with old git client - // or bypass the check, they still have a chance to edit this variable manually. - // If everything works fine, the code for git 1.x could be removed in a separate PR before 1.17 frozen. GitVersionRequired = "2.0.0" // GitExecutable is the command name of git From e8fad3759ac48dd3e4b74303488a959fcc664b19 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 10 Jun 2022 13:00:09 +0800 Subject: [PATCH 04/10] try to fix tests --- modules/git/git.go | 9 +++- modules/git/repo_language_stats_gogit.go | 44 ++++++++++---------- modules/git/repo_language_stats_nogogit.go | 48 +++++++++++----------- services/gitdiff/gitdiff.go | 48 +++++++++++----------- 4 files changed, 80 insertions(+), 69 deletions(-) diff --git a/modules/git/git.go b/modules/git/git.go index bf5f328e78356..6adfbc9e650e7 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -10,6 +10,7 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "runtime" "strings" "sync" @@ -139,8 +140,12 @@ func InitSimple(ctx context.Context) error { // HomeDir is the home dir for git to store the global config file used by Gitea internally func HomeDir() string { if setting.RepoRootPath == "" { - log.Fatal("Can not get Git's HomeDir (RepoRootPath is empty), the setting and git modules are not initialized correctly") - return "" + // TODO: now, some unit test code call the git module directly without initialization, which is incorrect. + // at the moment, we just use a temp HomeDir to prevent from conflicting with user's git config + // in the future, the git module should be initialized first before use. + tmpHomeDir := filepath.Join(os.TempDir(), "gitea-temp-home") + log.Error("Git's HomeDir is empty (RepoRootPath is empty), the git module is not initialized correctly, using a temp HomeDir (%s) temporarily", tmpHomeDir) + return tmpHomeDir } return setting.RepoRootPath } diff --git a/modules/git/repo_language_stats_gogit.go b/modules/git/repo_language_stats_gogit.go index 0cc36a16ad495..81e24d9e2baf6 100644 --- a/modules/git/repo_language_stats_gogit.go +++ b/modules/git/repo_language_stats_gogit.go @@ -45,28 +45,30 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err var checker *CheckAttributeReader - indexFilename, workTree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID) - if err == nil { - defer deleteTemporaryFile() - checker = &CheckAttributeReader{ - Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, - Repo: repo, - IndexFile: indexFilename, - WorkTree: workTree, - } - ctx, cancel := context.WithCancel(DefaultContext) - if err := checker.Init(ctx); err != nil { - log.Error("Unable to open checker for %s. Error: %v", commitID, err) - } else { - go func() { - err = checker.Run() - if err != nil { - log.Error("Unable to open checker for %s. Error: %v", commitID, err) - cancel() - } - }() + { + indexFilename, workTree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID) + if err == nil { + defer deleteTemporaryFile() + checker = &CheckAttributeReader{ + Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, + Repo: repo, + IndexFile: indexFilename, + WorkTree: workTree, + } + ctx, cancel := context.WithCancel(DefaultContext) + if err := checker.Init(ctx); err != nil { + log.Error("Unable to open checker for %s. Error: %v", commitID, err) + } else { + go func() { + err = checker.Run() + if err != nil { + log.Error("Unable to open checker for %s. Error: %v", commitID, err) + cancel() + } + }() + } + defer cancel() } - defer cancel() } sizes := make(map[string]int64) diff --git a/modules/git/repo_language_stats_nogogit.go b/modules/git/repo_language_stats_nogogit.go index 4ca0ba3b53503..636545dd2156d 100644 --- a/modules/git/repo_language_stats_nogogit.go +++ b/modules/git/repo_language_stats_nogogit.go @@ -65,31 +65,33 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err var checker *CheckAttributeReader - indexFilename, worktree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID) - if err == nil { - defer deleteTemporaryFile() - checker = &CheckAttributeReader{ - Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, - Repo: repo, - IndexFile: indexFilename, - WorkTree: worktree, - } - ctx, cancel := context.WithCancel(repo.Ctx) - if err := checker.Init(ctx); err != nil { - log.Error("Unable to open checker for %s. Error: %v", commitID, err) - } else { - go func() { - err = checker.Run() - if err != nil { - log.Error("Unable to open checker for %s. Error: %v", commitID, err) - cancel() - } + { + indexFilename, worktree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID) + if err == nil { + defer deleteTemporaryFile() + checker = &CheckAttributeReader{ + Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, + Repo: repo, + IndexFile: indexFilename, + WorkTree: worktree, + } + ctx, cancel := context.WithCancel(repo.Ctx) + if err := checker.Init(ctx); err != nil { + log.Error("Unable to open checker for %s. Error: %v", commitID, err) + } else { + go func() { + err = checker.Run() + if err != nil { + log.Error("Unable to open checker for %s. Error: %v", commitID, err) + cancel() + } + }() + } + defer func() { + _ = checker.Close() + cancel() }() } - defer func() { - _ = checker.Close() - cancel() - }() } contentBuf := bytes.Buffer{} diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 36ce943bcca29..cbdc43dcbfae6 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1418,32 +1418,34 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff var checker *git.CheckAttributeReader - indexFilename, worktree, deleteTemporaryFile, err := gitRepo.ReadTreeToTemporaryIndex(opts.AfterCommitID) - if err == nil { - defer deleteTemporaryFile() - - checker = &git.CheckAttributeReader{ - Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, - Repo: gitRepo, - IndexFile: indexFilename, - WorkTree: worktree, - } - ctx, cancel := context.WithCancel(gitRepo.Ctx) - if err := checker.Init(ctx); err != nil { - log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) - } else { - go func() { - err := checker.Run() - if err != nil && err != ctx.Err() { - log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) - } + { + indexFilename, worktree, deleteTemporaryFile, err := gitRepo.ReadTreeToTemporaryIndex(opts.AfterCommitID) + if err == nil { + defer deleteTemporaryFile() + + checker = &git.CheckAttributeReader{ + Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, + Repo: gitRepo, + IndexFile: indexFilename, + WorkTree: worktree, + } + ctx, cancel := context.WithCancel(gitRepo.Ctx) + if err := checker.Init(ctx); err != nil { + log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) + } else { + go func() { + err := checker.Run() + if err != nil && err != ctx.Err() { + log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) + } + cancel() + }() + } + defer func() { + _ = checker.Close() cancel() }() } - defer func() { - _ = checker.Close() - cancel() - }() } for _, diffFile := range diff.Files { From b32101f3d5af5f73ea24fd40840e021bb128d820 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 11 Jun 2022 12:38:05 +0800 Subject: [PATCH 05/10] try test again --- integrations/integration_test.go | 9 +++++++-- integrations/migration-test/migration_test.go | 2 +- models/migrations/migrations_test.go | 2 +- models/unittest/testdb.go | 4 +++- modules/git/git.go | 3 +++ modules/git/repo_language_stats_gogit.go | 1 + modules/git/repo_language_stats_nogogit.go | 1 + services/gitdiff/gitdiff.go | 1 + 8 files changed, 18 insertions(+), 5 deletions(-) diff --git a/integrations/integration_test.go b/integrations/integration_test.go index ce21eb2ef7bc8..b0004927f7b57 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -174,7 +174,12 @@ func initIntegrationTest() { setting.LoadForTest() 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 { + log.Fatal("git.InitOnceWithSync: %v", err) + } git.CheckLFSVersion() + setting.InitDBConfig() if err := storage.Init(); err != nil { fmt.Printf("Init storage failed: %v", err) @@ -275,7 +280,7 @@ 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())) + 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) @@ -576,7 +581,7 @@ 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())) + 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 83c31d80187e4..20a5c903a92cc 100644 --- a/integrations/migration-test/migration_test.go +++ b/integrations/migration-test/migration_test.go @@ -62,7 +62,6 @@ func initMigrationTest(t *testing.T) func() { assert.True(t, len(setting.RepoRootPath) != 0) 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())) ownerDirs, err := os.ReadDir(setting.RepoRootPath) if err != nil { assert.NoError(t, err, "unable to read the new repo root: %v\n", err) @@ -83,6 +82,7 @@ func initMigrationTest(t *testing.T) func() { } } + assert.NoError(t, git.InitOnceWithSync(context.Background())) git.CheckLFSVersion() setting.InitDBConfig() setting.NewLogServices(true) diff --git a/models/migrations/migrations_test.go b/models/migrations/migrations_test.go index 0c8e74f734c56..c52c302fc5459 100644 --- a/models/migrations/migrations_test.go +++ b/models/migrations/migrations_test.go @@ -203,7 +203,7 @@ 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())) + 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/unittest/testdb.go b/models/unittest/testdb.go index d1a44985100e6..baea46dbce68f 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -117,9 +117,11 @@ func MainTest(m *testing.M, testOpts *TestOptions) { if err = CopyDir(filepath.Join(testOpts.GiteaRootPath, "integrations", "gitea-repositories-meta"), setting.RepoRootPath); err != nil { fatalTestError("util.CopyDir: %v\n", err) } + 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 { @@ -202,7 +204,7 @@ 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())) + 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) diff --git a/modules/git/git.go b/modules/git/git.go index 656461ef0320f..a5887619c4c50 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -134,6 +134,9 @@ func HomeDir() string { tmpHomeDir := filepath.Join(os.TempDir(), "gitea-temp-home") log.Error("Git's HomeDir is empty (RepoRootPath is empty), the git module is not initialized correctly, using a temp HomeDir (%s) temporarily", tmpHomeDir) return tmpHomeDir + + // the Fatal will cause some CI failures (root reason is still unknown), need to be investigated more in the future + // log.Fatal("Can not get Git's HomeDir (RepoRootPath is empty), the setting and git modules are not initialized correctly") } return setting.RepoRootPath } diff --git a/modules/git/repo_language_stats_gogit.go b/modules/git/repo_language_stats_gogit.go index 81e24d9e2baf6..4214a22180dfa 100644 --- a/modules/git/repo_language_stats_gogit.go +++ b/modules/git/repo_language_stats_gogit.go @@ -45,6 +45,7 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err var checker *CheckAttributeReader + // this code block is refactored from old code "if (git version ...)", keep the block to limit the variable scope { indexFilename, workTree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID) if err == nil { diff --git a/modules/git/repo_language_stats_nogogit.go b/modules/git/repo_language_stats_nogogit.go index 636545dd2156d..3304e1d07789b 100644 --- a/modules/git/repo_language_stats_nogogit.go +++ b/modules/git/repo_language_stats_nogogit.go @@ -65,6 +65,7 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err var checker *CheckAttributeReader + // this code block is refactored from old code "if (git version ...)", keep the block to limit the variable scope { indexFilename, worktree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID) if err == nil { diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index cbdc43dcbfae6..c00c67762bdd2 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1418,6 +1418,7 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff var checker *git.CheckAttributeReader + // this code block is refactored from old code "if (git version ...)", keep the block to limit the variable scope { indexFilename, worktree, deleteTemporaryFile, err := gitRepo.ReadTreeToTemporaryIndex(opts.AfterCommitID) if err == nil { From 187a27f00c5be5b6517b6a61f491948ac767c01e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 11 Jun 2022 18:05:43 +0800 Subject: [PATCH 06/10] use const for GitVersionRequired instead of var --- modules/git/git.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/git/git.go b/modules/git/git.go index a5887619c4c50..fee037c9464ed 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -22,10 +22,10 @@ import ( "github.com/hashicorp/go-version" ) -var ( - // GitVersionRequired is the minimum Git version required - GitVersionRequired = "2.0.0" +// GitVersionRequired is the minimum Git version required +const GitVersionRequired = "2.0.0" +var ( // GitExecutable is the command name of git // Could be updated to an absolute path while initialization GitExecutable = "git" From 09e29f4058bb10bc20067745d1b33b8b5409b1a1 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 11 Jun 2022 19:09:24 +0800 Subject: [PATCH 07/10] try to fix integration test --- cmd/hook.go | 4 +-- models/migrations/migrations_test.go | 4 +++ modules/git/git.go | 49 ++++++++++++++++------------ 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/cmd/hook.go b/cmd/hook.go index 8078763b183fd..73386038b375a 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -308,6 +308,8 @@ func runHookPostReceive(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() + setup("hooks/post-receive.log", c.Bool("debug")) + // First of all run update-server-info no matter what if _, _, err := git.NewCommand(ctx, "update-server-info").RunStdString(nil); err != nil { return fmt.Errorf("Failed to call 'git update-server-info': %v", err) @@ -318,8 +320,6 @@ func runHookPostReceive(c *cli.Context) error { return nil } - setup("hooks/post-receive.log", c.Bool("debug")) - if len(os.Getenv("SSH_ORIGINAL_COMMAND")) == 0 { if setting.OnlyAllowPushIfGiteaEnvironmentSet { return fail(`Rejecting changes as Gitea environment not set. diff --git a/models/migrations/migrations_test.go b/models/migrations/migrations_test.go index c52c302fc5459..46782f24a1003 100644 --- a/models/migrations/migrations_test.go +++ b/models/migrations/migrations_test.go @@ -66,6 +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) + os.Exit(1) + } git.CheckLFSVersion() setting.InitDBConfig() setting.NewLogServices(true) diff --git a/modules/git/git.go b/modules/git/git.go index fee037c9464ed..d789a576ba13e 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -7,10 +7,10 @@ package git import ( "context" + "errors" "fmt" "os" "os/exec" - "path/filepath" "runtime" "strings" "sync" @@ -30,9 +30,8 @@ var ( // Could be updated to an absolute path while initialization GitExecutable = "git" - // DefaultContext is the default context to run git commands in - // will be overwritten by InitXxx with HammerContext - DefaultContext = context.Background() + // DefaultContext is the default context to run git commands in, must be initialized by git.InitXxx + DefaultContext context.Context // SupportProcReceive version >= 2.29.0 SupportProcReceive bool @@ -125,39 +124,43 @@ func VersionInfo() string { return fmt.Sprintf(format, args...) } +func checkInit() error { + if setting.RepoRootPath == "" { + return errors.New("can not init Git's HomeDir (RepoRootPath is empty), the setting and git modules are not initialized correctly") + } + if DefaultContext != nil { + log.Warn("git module has been initialized already, duplicate init should be fixed") + } + return nil +} + // HomeDir is the home dir for git to store the global config file used by Gitea internally func HomeDir() string { if setting.RepoRootPath == "" { - // TODO: now, some unit test code call the git module directly without initialization, which is incorrect. - // at the moment, we just use a temp HomeDir to prevent from conflicting with user's git config - // in the future, the git module should be initialized first before use. - tmpHomeDir := filepath.Join(os.TempDir(), "gitea-temp-home") - log.Error("Git's HomeDir is empty (RepoRootPath is empty), the git module is not initialized correctly, using a temp HomeDir (%s) temporarily", tmpHomeDir) - return tmpHomeDir - - // the Fatal will cause some CI failures (root reason is still unknown), need to be investigated more in the future - // log.Fatal("Can not get Git's HomeDir (RepoRootPath is empty), the setting and git modules are not initialized correctly") + // 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. + // 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("can not get Git's HomeDir (RepoRootPath is empty), the setting and git modules are not initialized correctly") + return "" } return setting.RepoRootPath } // 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 func InitSimple(ctx context.Context) error { + if err := checkInit(); err != nil { + return err + } + DefaultContext = ctx if setting.Git.Timeout.Default > 0 { defaultCommandExecutionTimeout = time.Duration(setting.Git.Timeout.Default) * time.Second } - if err := SetExecutablePath(setting.Git.Path); err != nil { - return err - } - - // force cleanup args - globalCommandArgs = []string{} - - return nil + return SetExecutablePath(setting.Git.Path) } var initOnce sync.Once @@ -166,6 +169,10 @@ var initOnce sync.Once // This method will update the global variables ONLY ONCE (just like git.CheckLFSVersion -- which is 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 { + return err + } + initOnce.Do(func() { err = InitSimple(ctx) if err != nil { From 038929e7070c081dca58069acdbf39c83518970c Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 13 Jun 2022 20:39:58 +0100 Subject: [PATCH 08/10] Refactor CheckAttributeReader to make a *git.Repository version --- modules/git/repo_attribute.go | 34 ++++++++++++++++++++++ modules/git/repo_language_stats_gogit.go | 32 ++------------------ modules/git/repo_language_stats_nogogit.go | 34 ++-------------------- services/gitdiff/gitdiff.go | 34 ++-------------------- 4 files changed, 40 insertions(+), 94 deletions(-) diff --git a/modules/git/repo_attribute.go b/modules/git/repo_attribute.go index 7cce462fab0de..596a91e80382a 100644 --- a/modules/git/repo_attribute.go +++ b/modules/git/repo_attribute.go @@ -392,3 +392,37 @@ func (wr *lineSeparatedAttributeWriter) Close() error { close(wr.closed) return nil } + +// Create a check attribute reader for the current repository and provided commit ID +func (repo *Repository) CheckAttributeReader(commitID string) (*CheckAttributeReader, context.CancelFunc) { + indexFilename, worktree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID) + if err != nil { + return nil, func() {} + } + + checker := &CheckAttributeReader{ + Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, + Repo: repo, + IndexFile: indexFilename, + WorkTree: worktree, + } + ctx, cancel := context.WithCancel(repo.Ctx) + if err := checker.Init(ctx); err != nil { + log.Error("Unable to open checker for %s. Error: %v", commitID, err) + } else { + go func() { + err := checker.Run() + if err != nil && err != ctx.Err() { + log.Error("Unable to open checker for %s. Error: %v", commitID, err) + } + cancel() + }() + } + deferable := func() { + _ = checker.Close() + cancel() + deleteTemporaryFile() + } + + return checker, deferable +} diff --git a/modules/git/repo_language_stats_gogit.go b/modules/git/repo_language_stats_gogit.go index 4214a22180dfa..34b0dc45d3749 100644 --- a/modules/git/repo_language_stats_gogit.go +++ b/modules/git/repo_language_stats_gogit.go @@ -8,12 +8,10 @@ package git import ( "bytes" - "context" "io" "strings" "code.gitea.io/gitea/modules/analyze" - "code.gitea.io/gitea/modules/log" "github.com/go-enry/go-enry/v2" "github.com/go-git/go-git/v5" @@ -43,34 +41,8 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err return nil, err } - var checker *CheckAttributeReader - - // this code block is refactored from old code "if (git version ...)", keep the block to limit the variable scope - { - indexFilename, workTree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID) - if err == nil { - defer deleteTemporaryFile() - checker = &CheckAttributeReader{ - Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, - Repo: repo, - IndexFile: indexFilename, - WorkTree: workTree, - } - ctx, cancel := context.WithCancel(DefaultContext) - if err := checker.Init(ctx); err != nil { - log.Error("Unable to open checker for %s. Error: %v", commitID, err) - } else { - go func() { - err = checker.Run() - if err != nil { - log.Error("Unable to open checker for %s. Error: %v", commitID, err) - cancel() - } - }() - } - defer cancel() - } - } + checker, deferable := repo.CheckAttributeReader(commitID) + defer deferable() sizes := make(map[string]int64) err = tree.Files().ForEach(func(f *object.File) error { diff --git a/modules/git/repo_language_stats_nogogit.go b/modules/git/repo_language_stats_nogogit.go index 3304e1d07789b..d237924f92a4c 100644 --- a/modules/git/repo_language_stats_nogogit.go +++ b/modules/git/repo_language_stats_nogogit.go @@ -9,7 +9,6 @@ package git import ( "bufio" "bytes" - "context" "io" "math" "strings" @@ -63,37 +62,8 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err return nil, err } - var checker *CheckAttributeReader - - // this code block is refactored from old code "if (git version ...)", keep the block to limit the variable scope - { - indexFilename, worktree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID) - if err == nil { - defer deleteTemporaryFile() - checker = &CheckAttributeReader{ - Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, - Repo: repo, - IndexFile: indexFilename, - WorkTree: worktree, - } - ctx, cancel := context.WithCancel(repo.Ctx) - if err := checker.Init(ctx); err != nil { - log.Error("Unable to open checker for %s. Error: %v", commitID, err) - } else { - go func() { - err = checker.Run() - if err != nil { - log.Error("Unable to open checker for %s. Error: %v", commitID, err) - cancel() - } - }() - } - defer func() { - _ = checker.Close() - cancel() - }() - } - } + checker, deferable := repo.CheckAttributeReader(commitID) + defer deferable() contentBuf := bytes.Buffer{} var content []byte diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index e49f473965b48..37dc0e114dac7 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1417,38 +1417,8 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff } diff.Start = opts.SkipTo - var checker *git.CheckAttributeReader - - // this code block is refactored from old code "if (git version ...)", keep the block to limit the variable scope - { - indexFilename, worktree, deleteTemporaryFile, err := gitRepo.ReadTreeToTemporaryIndex(opts.AfterCommitID) - if err == nil { - defer deleteTemporaryFile() - - checker = &git.CheckAttributeReader{ - Attributes: []string{"linguist-vendored", "linguist-generated", "linguist-language", "gitlab-language"}, - Repo: gitRepo, - IndexFile: indexFilename, - WorkTree: worktree, - } - ctx, cancel := context.WithCancel(gitRepo.Ctx) - if err := checker.Init(ctx); err != nil { - log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) - } else { - go func() { - err := checker.Run() - if err != nil && err != ctx.Err() { - log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) - } - cancel() - }() - } - defer func() { - _ = checker.Close() - cancel() - }() - } - } + checker, deferable := gitRepo.CheckAttributeReader(opts.AfterCommitID) + defer deferable() for _, diffFile := range diff.Files { From 64959f0c6aac9e1abe889cb1d456a7b31a50a251 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 14 Jun 2022 08:43:02 +0800 Subject: [PATCH 09/10] update document for commit signing with Gitea's internal gitconfig --- docs/content/doc/advanced/signing.en-us.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/docs/content/doc/advanced/signing.en-us.md b/docs/content/doc/advanced/signing.en-us.md index aaaeb71034821..9f56ae13d3d3d 100644 --- a/docs/content/doc/advanced/signing.en-us.md +++ b/docs/content/doc/advanced/signing.en-us.md @@ -83,8 +83,7 @@ The first option to discuss is the `SIGNING_KEY`. There are three main options: - `none` - this prevents Gitea from signing any commits -- `default` - Gitea will default to the key configured within - `git config` +- `default` - Gitea will default to the key configured within `git config` - `KEYID` - Gitea will sign commits with the gpg key with the ID `KEYID`. In this case you should provide a `SIGNING_NAME` and `SIGNING_EMAIL` to be displayed for this key. @@ -98,6 +97,11 @@ repositories, `SIGNING_KEY=default` could be used to provide different signing keys on a per-repository basis. However, this is clearly not an ideal UI and therefore subject to change. +**Since 1.17**, Gitea uses its internal git config (instead of current user's `{UserHome}/.gitconfig` for < 1.17). +If you have your own customized git config for Gitea, you should set these configs in system git config (aka `/etc/gitconfig`) +or the Gitea internal git config (aka `{[repository].ROOT}/.gitconfig`). + + ### `INITIAL_COMMIT` This option determines whether Gitea should sign the initial commit @@ -118,7 +122,7 @@ The possible values are: - `never`: Never sign - `pubkey`: Only sign if the user has a public key -- `twofa`: Only sign if the user logs in with two factor authentication +- `twofa`: Only sign if the user logs in with two-factor authentication - `parentsigned`: Only sign if the parent commit is signed. - `always`: Always sign @@ -132,7 +136,7 @@ editor or API CRUD actions. The possible values are: - `never`: Never sign - `pubkey`: Only sign if the user has a public key -- `twofa`: Only sign if the user logs in with two factor authentication +- `twofa`: Only sign if the user logs in with two-factor authentication - `parentsigned`: Only sign if the parent commit is signed. - `always`: Always sign @@ -146,7 +150,7 @@ The possible options are: - `never`: Never sign - `pubkey`: Only sign if the user has a public key -- `twofa`: Only sign if the user logs in with two factor authentication +- `twofa`: Only sign if the user logs in with two-factor authentication - `basesigned`: Only sign if the parent commit in the base repo is signed. - `headsigned`: Only sign if the head commit in the head branch is signed. - `commitssigned`: Only sign if all the commits in the head branch to the merge point are signed. From e4dcba60fafdeb4e329ff464cb12c118a6d76e56 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 14 Jun 2022 13:09:04 +0800 Subject: [PATCH 10/10] update document for commit signing with Gitea's internal gitconfig --- docs/content/doc/advanced/signing.en-us.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/content/doc/advanced/signing.en-us.md b/docs/content/doc/advanced/signing.en-us.md index 9f56ae13d3d3d..8ae2f94e9ece9 100644 --- a/docs/content/doc/advanced/signing.en-us.md +++ b/docs/content/doc/advanced/signing.en-us.md @@ -97,9 +97,10 @@ repositories, `SIGNING_KEY=default` could be used to provide different signing keys on a per-repository basis. However, this is clearly not an ideal UI and therefore subject to change. -**Since 1.17**, Gitea uses its internal git config (instead of current user's `{UserHome}/.gitconfig` for < 1.17). +**Since 1.17**, Gitea runs git in its own home directory `[repository].ROOT` and uses its own config `{[repository].ROOT}/.gitconfig`. If you have your own customized git config for Gitea, you should set these configs in system git config (aka `/etc/gitconfig`) -or the Gitea internal git config (aka `{[repository].ROOT}/.gitconfig`). +or the Gitea internal git config `{[repository].ROOT}/.gitconfig`. +Related home files for git command (like `.gnupg`) should also be put in Gitea's git home directory `[repository].ROOT`. ### `INITIAL_COMMIT`