From 9d013c90bdef0760a1e841577253a7ad30dfeafe Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 1 Jan 2022 18:01:55 +0800 Subject: [PATCH 01/30] Move git command as a standalone package so that it's easier to move the repositories to another machine --- cmd/hook.go | 6 ++ cmd/serv.go | 3 + modules/git/cmd/command.go | 43 +++++++++ modules/git/cmd/local.go | 161 +++++++++++++++++++++++++++++++++ modules/git/command.go | 166 ++++++---------------------------- modules/git/git.go | 21 +++-- modules/git/remote.go | 2 +- modules/git/repo_attribute.go | 2 +- services/pull/merge.go | 10 +- 9 files changed, 260 insertions(+), 154 deletions(-) create mode 100644 modules/git/cmd/command.go create mode 100644 modules/git/cmd/local.go diff --git a/cmd/hook.go b/cmd/hook.go index 8078763b183f..6e535170901b 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -218,6 +218,9 @@ Gitea or set your environment appropriately.`, "") } } + if err := git.Init(ctx); err != nil { + return fail("Git init failed", err.Error()) + } supportProcRecive := false if git.CheckGitVersionAtLeast("2.29") == nil { supportProcRecive = true @@ -498,6 +501,9 @@ Gitea or set your environment appropriately.`, "") ctx, cancel := installSignals() defer cancel() + if err := git.Init(ctx); err != nil { + return fail("Git init failed", err.Error()) + } if git.CheckGitVersionAtLeast("2.29") != nil { return fail("Internal Server Error", "git not support proc-receive.") } diff --git a/cmd/serv.go b/cmd/serv.go index 6ba3e9de01ff..3e4e4d3dc868 100644 --- a/cmd/serv.go +++ b/cmd/serv.go @@ -151,6 +151,9 @@ func runServ(c *cli.Context) error { } if len(words) < 2 { + if err := git.Init(ctx); err != nil { + return fail("Git init failed", err.Error()) + } if git.CheckGitVersionAtLeast("2.29") == nil { // for AGit Flow if cmd == "ssh_info" { diff --git a/modules/git/cmd/command.go b/modules/git/cmd/command.go new file mode 100644 index 000000000000..8342439809a4 --- /dev/null +++ b/modules/git/cmd/command.go @@ -0,0 +1,43 @@ +// Copyright 2022 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 cmd + +import ( + "context" + "io" + "time" +) + +// defaultCommandExecutionTimeout default command execution timeout duration +var defaultCommandExecutionTimeout = 360 * time.Second + +// SetDefaultCommandTimout set the default timeout +func SetDefaultCommandTimout(timeout time.Duration) { + defaultCommandExecutionTimeout = timeout +} + +// RunOpts represents parameters to run the command +type RunOpts struct { + Env []string + Timeout time.Duration + Dir string + Stdout, Stderr io.Writer + Stdin io.Reader + PipelineFunc func(context.Context, context.CancelFunc) error +} + +// Command represents a git command +type Command interface { + String() string + SetParentContext(ctx context.Context) Command + SetDescription(desc string) Command + AddArguments(args ...string) Command + Run(*RunOpts) error +} + +// Service represents a service to create git command +type Service interface { + NewCommand(ctx context.Context, gloablArgsLength int, args ...string) Command +} diff --git a/modules/git/cmd/local.go b/modules/git/cmd/local.go new file mode 100644 index 000000000000..c0abcfcb40a5 --- /dev/null +++ b/modules/git/cmd/local.go @@ -0,0 +1,161 @@ +// Copyright 2022 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 cmd + +import ( + "context" + "fmt" + "os" + "os/exec" + "strings" + + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/process" + "code.gitea.io/gitea/modules/util" +) + +// LocalCommand represents a local git command +type LocalCommand struct { + service *LocalService + args []string + parentContext context.Context + desc string + globalArgsLength int +} + +var _ Command = &LocalCommand{} + +func (c *LocalCommand) String() string { + if len(c.args) == 0 { + return c.service.GitExecutable + } + return fmt.Sprintf("%s %s", c.service.GitExecutable, strings.Join(c.args, " ")) +} + +// SetParentContext sets the parent context for this command +func (c *LocalCommand) SetParentContext(ctx context.Context) Command { + c.parentContext = ctx + return c +} + +// SetDescription sets the description for this command which be returned on +// c.String() +func (c *LocalCommand) SetDescription(desc string) Command { + c.desc = desc + return c +} + +// AddArguments adds new argument(s) to the command. +func (c *LocalCommand) AddArguments(args ...string) Command { + c.args = append(c.args, args...) + return c +} + +// defaultLocale is the default LC_ALL to run git commands in. +const defaultLocale = "C" + +// Run runs the command with the RunOpts +func (c *LocalCommand) Run(opts *RunOpts) error { + if opts == nil { + opts = &RunOpts{} + } + if opts.Timeout <= 0 { + opts.Timeout = defaultCommandExecutionTimeout + } + + if len(opts.Dir) == 0 { + log.Debug("%s", c) + } else { + log.Debug("%s: %v", opts.Dir, c) + } + + desc := c.desc + if desc == "" { + args := c.args[c.globalArgsLength:] + var argSensitiveURLIndexes []int + for i, arg := range c.args { + if strings.Contains(arg, "://") && strings.Contains(arg, "@") { + argSensitiveURLIndexes = append(argSensitiveURLIndexes, i) + } + } + if len(argSensitiveURLIndexes) > 0 { + args = make([]string, len(c.args)) + copy(args, c.args) + for _, urlArgIndex := range argSensitiveURLIndexes { + args[urlArgIndex] = util.SanitizeCredentialURLs(args[urlArgIndex]) + } + } + desc = fmt.Sprintf("%s %s [repo_path: %s]", c.service.GitExecutable, strings.Join(args, " "), opts.Dir) + } + + ctx, cancel, finished := process.GetManager().AddContextTimeout(c.parentContext, opts.Timeout, desc) + defer finished() + + cmd := exec.CommandContext(ctx, c.service.GitExecutable, c.args...) + if opts.Env == nil { + cmd.Env = os.Environ() + } else { + cmd.Env = opts.Env + } + + cmd.Env = append( + cmd.Env, + fmt.Sprintf("LC_ALL=%s", defaultLocale), + // avoid prompting for credentials interactively, supported since git v2.3 + "GIT_TERMINAL_PROMPT=0", + // ignore replace references (https://git-scm.com/docs/git-replace) + "GIT_NO_REPLACE_OBJECTS=1", + ) + + process.SetSysProcAttribute(cmd) + cmd.Dir = opts.Dir + cmd.Stdout = opts.Stdout + cmd.Stderr = opts.Stderr + cmd.Stdin = opts.Stdin + if err := cmd.Start(); err != nil { + return err + } + + if opts.PipelineFunc != nil { + err := opts.PipelineFunc(ctx, cancel) + if err != nil { + cancel() + _ = cmd.Wait() + return err + } + } + + if err := cmd.Wait(); err != nil && ctx.Err() != context.DeadlineExceeded { + return err + } + + return ctx.Err() +} + +// LocalService represents a command service to create local git commands +type LocalService struct { + GitExecutable string // git binary location + RepoRootPath string // repository storage root directory +} + +var _ Service = &LocalService{} + +// NewLocalService returns a local service +func NewLocalService(gitExecutable, repoRootPath string) *LocalService { + return &LocalService{ + GitExecutable: gitExecutable, + RepoRootPath: repoRootPath, + } +} + +// NewCommand creates and returns a new Git Command based on given command and arguments. +func (s *LocalService) NewCommand(ctx context.Context, gloablArgsLength int, args ...string) Command { + return &LocalCommand{ + service: s, + args: args, + parentContext: ctx, + globalArgsLength: gloablArgsLength, + } +} diff --git a/modules/git/command.go b/modules/git/command.go index f6344dbfd1ae..a82d2ffdb642 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -8,180 +8,67 @@ package git import ( "bytes" "context" - "fmt" - "io" - "os" - "os/exec" "strings" - "time" "unsafe" - "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/process" - "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/modules/git/cmd" ) var ( // globalCommandArgs global command args for external package setting globalCommandArgs []string - // defaultCommandExecutionTimeout default command execution timeout duration - defaultCommandExecutionTimeout = 360 * time.Second + // cmdService represents a command service + cmdService cmd.Service ) -// DefaultLocale is the default LC_ALL to run git commands in. -const DefaultLocale = "C" - -// Command represents a command with its subcommands or arguments. -type Command struct { - name string - args []string - parentContext context.Context - desc string - globalArgsLength int -} - -func (c *Command) String() string { - if len(c.args) == 0 { - return c.name - } - return fmt.Sprintf("%s %s", c.name, strings.Join(c.args, " ")) +// CommandProxy represents a command proxy with its subcommands or arguments. +type CommandProxy struct { + cmd.Command } // NewCommand creates and returns a new Git Command based on given command and arguments. -func NewCommand(ctx context.Context, args ...string) *Command { - // Make an explicit copy of globalCommandArgs, otherwise append might overwrite it - cargs := make([]string, len(globalCommandArgs)) - copy(cargs, globalCommandArgs) - return &Command{ - name: GitExecutable, - args: append(cargs, args...), - parentContext: ctx, - globalArgsLength: len(globalCommandArgs), +// NewCommand creates and returns a new Git Command based on given command and arguments. +func NewCommand(ctx context.Context, args ...string) *CommandProxy { + // Make an explicit copy of GlobalCommandArgs, otherwise append might overwrite it + cargs := make([]string, len(GlobalCommandArgs)) + copy(cargs, GlobalCommandArgs) + return &CommandProxy{ + Command: cmdService.NewCommand(ctx, len(cargs), append(cargs, args...)...), } } // NewCommandNoGlobals creates and returns a new Git Command based on given command and arguments only with the specify args and don't care global command args -func NewCommandNoGlobals(args ...string) *Command { +func NewCommandNoGlobals(args ...string) *CommandProxy { return NewCommandContextNoGlobals(DefaultContext, args...) } // NewCommandContextNoGlobals creates and returns a new Git Command based on given command and arguments only with the specify args and don't care global command args -func NewCommandContextNoGlobals(ctx context.Context, args ...string) *Command { - return &Command{ - name: GitExecutable, - args: args, - parentContext: ctx, +func NewCommandContextNoGlobals(ctx context.Context, args ...string) *CommandProxy { + return &CommandProxy{ + Command: cmdService.NewCommand(ctx, 0, args...), } } // SetParentContext sets the parent context for this command -func (c *Command) SetParentContext(ctx context.Context) *Command { - c.parentContext = ctx +func (c *CommandProxy) SetParentContext(ctx context.Context) *CommandProxy { + c.Command.SetParentContext(ctx) return c } // SetDescription sets the description for this command which be returned on // c.String() -func (c *Command) SetDescription(desc string) *Command { - c.desc = desc +func (c *CommandProxy) SetDescription(desc string) *CommandProxy { + c.Command.SetDescription(desc) return c } // AddArguments adds new argument(s) to the command. -func (c *Command) AddArguments(args ...string) *Command { - c.args = append(c.args, args...) +func (c *CommandProxy) AddArguments(args ...string) *CommandProxy { + c.Command.AddArguments(args...) return c } -// RunOpts represents parameters to run the command -type RunOpts struct { - Env []string - Timeout time.Duration - Dir string - Stdout, Stderr io.Writer - Stdin io.Reader - PipelineFunc func(context.Context, context.CancelFunc) error -} - -// Run runs the command with the RunOpts -func (c *Command) Run(opts *RunOpts) error { - if opts == nil { - opts = &RunOpts{} - } - if opts.Timeout <= 0 { - opts.Timeout = defaultCommandExecutionTimeout - } - - if len(opts.Dir) == 0 { - log.Debug("%s", c) - } else { - log.Debug("%s: %v", opts.Dir, c) - } - - desc := c.desc - if desc == "" { - args := c.args[c.globalArgsLength:] - var argSensitiveURLIndexes []int - for i, arg := range c.args { - if strings.Contains(arg, "://") && strings.Contains(arg, "@") { - argSensitiveURLIndexes = append(argSensitiveURLIndexes, i) - } - } - if len(argSensitiveURLIndexes) > 0 { - args = make([]string, len(c.args)) - copy(args, c.args) - for _, urlArgIndex := range argSensitiveURLIndexes { - args[urlArgIndex] = util.SanitizeCredentialURLs(args[urlArgIndex]) - } - } - desc = fmt.Sprintf("%s %s [repo_path: %s]", c.name, strings.Join(args, " "), opts.Dir) - } - - ctx, cancel, finished := process.GetManager().AddContextTimeout(c.parentContext, opts.Timeout, desc) - defer finished() - - cmd := exec.CommandContext(ctx, c.name, c.args...) - if opts.Env == nil { - cmd.Env = os.Environ() - } else { - cmd.Env = opts.Env - } - - cmd.Env = append( - cmd.Env, - fmt.Sprintf("LC_ALL=%s", DefaultLocale), - // avoid prompting for credentials interactively, supported since git v2.3 - "GIT_TERMINAL_PROMPT=0", - // ignore replace references (https://git-scm.com/docs/git-replace) - "GIT_NO_REPLACE_OBJECTS=1", - ) - - process.SetSysProcAttribute(cmd) - cmd.Dir = opts.Dir - cmd.Stdout = opts.Stdout - cmd.Stderr = opts.Stderr - cmd.Stdin = opts.Stdin - if err := cmd.Start(); err != nil { - return err - } - - if opts.PipelineFunc != nil { - err := opts.PipelineFunc(ctx, cancel) - if err != nil { - cancel() - _ = cmd.Wait() - return err - } - } - - if err := cmd.Wait(); err != nil && ctx.Err() != context.DeadlineExceeded { - return err - } - - return ctx.Err() -} - type RunStdError interface { error Stderr() string @@ -213,8 +100,11 @@ func bytesToString(b []byte) string { return *(*string)(unsafe.Pointer(&b)) // that's what Golang's strings.Builder.String() does (go/src/strings/builder.go) } +// RunOpts is an alias of cmd.RunOpts +type RunOpts = cmd.RunOpts + // RunStdString runs the command with options and returns stdout/stderr as string. and store stderr to returned error (err combined with stderr). -func (c *Command) RunStdString(opts *RunOpts) (stdout, stderr string, runErr RunStdError) { +func (c *CommandProxy) RunStdString(opts *RunOpts) (stdout, stderr string, runErr RunStdError) { stdoutBytes, stderrBytes, err := c.RunStdBytes(opts) stdout = bytesToString(stdoutBytes) stderr = bytesToString(stderrBytes) @@ -226,7 +116,7 @@ func (c *Command) RunStdString(opts *RunOpts) (stdout, stderr string, runErr Run } // RunStdBytes runs the command with options and returns stdout/stderr as bytes. and store stderr to returned error (err combined with stderr). -func (c *Command) RunStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunStdError) { +func (c *CommandProxy) RunStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunStdError) { if opts == nil { opts = &RunOpts{} } diff --git a/modules/git/git.go b/modules/git/git.go index 8fad07033006..79449ea42f4d 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -14,6 +14,7 @@ import ( "strings" "time" + "code.gitea.io/gitea/modules/git/cmd" "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/setting" @@ -92,8 +93,11 @@ func SetExecutablePath(path string) error { return fmt.Errorf("git not found: %w", err) } GitExecutable = absPath + return nil +} - err = LoadGitVersion() +func checkGitVersion() error { + err := LoadGitVersion() if err != nil { return fmt.Errorf("unable to load git version: %w", err) } @@ -114,7 +118,6 @@ func SetExecutablePath(path string) error { } return fmt.Errorf("installed git version %q is not supported, Gitea requires git version >= %q, %s", gitVersion.Original(), GitVersionRequired, moreHint) } - return nil } @@ -136,13 +139,18 @@ func Init(ctx context.Context) error { DefaultContext = ctx if setting.Git.Timeout.Default > 0 { - defaultCommandExecutionTimeout = time.Duration(setting.Git.Timeout.Default) * time.Second + cmd.SetDefaultCommandTimout(time.Duration(setting.Git.Timeout.Default) * time.Second) } - if err := SetExecutablePath(setting.Git.Path); err != nil { return err } + cmdService = cmd.NewLocalService(GitExecutable, setting.RepoRootPath) + + if err := checkGitVersion(); err != nil { + return err + } + // force cleanup args globalCommandArgs = []string{} @@ -161,11 +169,6 @@ func Init(ctx context.Context) error { globalCommandArgs = append(globalCommandArgs, "-c", "uploadpack.allowfilter=true", "-c", "uploadpack.allowAnySHA1InWant=true") } - // Save current git version on init to gitVersion otherwise it would require an RWMutex - if err := LoadGitVersion(); err != nil { - return err - } - // Git requires setting user.name and user.email in order to commit changes - if they're not set just add some defaults for configKey, defaultValue := range map[string]string{"user.name": "Gitea", "user.email": "gitea@fake.local"} { if err := checkAndSetConfig(configKey, defaultValue, false); err != nil { diff --git a/modules/git/remote.go b/modules/git/remote.go index 536b1681cecf..24a4e1b6386b 100644 --- a/modules/git/remote.go +++ b/modules/git/remote.go @@ -15,7 +15,7 @@ func GetRemoteAddress(ctx context.Context, repoPath, remoteName string) (*url.UR if err != nil { return nil, err } - var cmd *Command + var cmd *CommandProxy if CheckGitVersionAtLeast("2.7") == nil { cmd = NewCommand(ctx, "remote", "get-url", remoteName) } else { diff --git a/modules/git/repo_attribute.go b/modules/git/repo_attribute.go index a18c80c3f12f..14a7d827ab1a 100644 --- a/modules/git/repo_attribute.go +++ b/modules/git/repo_attribute.go @@ -120,7 +120,7 @@ type CheckAttributeReader struct { stdinReader io.ReadCloser stdinWriter *os.File stdOut attributeWriter - cmd *Command + cmd *CommandProxy env []string ctx context.Context cancel context.CancelFunc diff --git a/services/pull/merge.go b/services/pull/merge.go index fcced65cdf8c..6986e3573e6e 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -280,13 +280,13 @@ 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 + var gitConfigCommand func() *git.CommandProxy if git.CheckGitVersionAtLeast("1.8.0") == nil { - gitConfigCommand = func() *git.Command { + gitConfigCommand = func() *git.CommandProxy { return git.NewCommand(ctx, "config", "--local") } } else { - gitConfigCommand = func() *git.Command { + gitConfigCommand = func() *git.CommandProxy { return git.NewCommand(ctx, "config") } } @@ -600,7 +600,7 @@ func rawMerge(ctx context.Context, pr *models.PullRequest, doer *user_model.User pr.ID, ) - var pushCmd *git.Command + var pushCmd *git.CommandProxy if mergeStyle == repo_model.MergeStyleRebaseUpdate { // force push the rebase result to head branch pushCmd = git.NewCommand(ctx, "push", "-f", "head_repo", stagingBranch+":"+git.BranchPrefix+pr.HeadBranch) @@ -668,7 +668,7 @@ func commitAndSignNoAuthor(ctx context.Context, pr *models.PullRequest, message, return nil } -func runMergeCommand(pr *models.PullRequest, mergeStyle repo_model.MergeStyle, cmd *git.Command, tmpBasePath string) error { +func runMergeCommand(pr *models.PullRequest, mergeStyle repo_model.MergeStyle, cmd *git.CommandProxy, tmpBasePath string) error { var outbuf, errbuf strings.Builder if err := cmd.Run(&git.RunOpts{ Dir: tmpBasePath, From 31412d8efebab618a26bcefaec2e9aaedd06854e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 2 Jan 2022 13:54:18 +0800 Subject: [PATCH 02/30] Fix unit test --- models/unittest/testdb.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index 117614a7a4f3..92b514a3ccb4 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/util" @@ -110,6 +111,10 @@ func MainTest(m *testing.M, testOpts *TestOptions) { fatalTestError("storage.Init: %v\n", err) } + if err = git.Init(context.Background()); err != nil { + fatalTestError("git.Init: %v\n", err) + } + if err = util.RemoveAll(repoRootPath); err != nil { fatalTestError("util.RemoveAll: %v\n", err) } From 3e5f51da476a4a02538a977896695e947608a7a5 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 2 Jan 2022 17:34:05 +0800 Subject: [PATCH 03/30] Fix bug --- modules/markup/html_test.go | 6 ++++++ modules/markup/markdown/markdown_test.go | 14 +++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index f6aabc6272e4..91ad19473f30 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -6,9 +6,11 @@ package markup_test import ( "io" + "path/filepath" "strings" "testing" + "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/emoji" "code.gitea.io/gitea/modules/git" . "code.gitea.io/gitea/modules/markup" @@ -25,6 +27,10 @@ var localMetas = map[string]string{ "repoPath": "../../integrations/gitea-repositories-meta/user13/repo11.git/", } +func TestMain(m *testing.M) { + unittest.MainTest(m, filepath.Join("..", "..")) +} + 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 a069d402bb50..903ca6ad1948 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -5,9 +5,11 @@ package markdown_test import ( + "path/filepath" "strings" "testing" + "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" @@ -18,11 +20,13 @@ import ( "github.com/stretchr/testify/assert" ) -const ( - AppURL = "http://localhost:3000/" - Repo = "gogits/gogs" - AppSubURL = AppURL + Repo + "/" -) +func TestMain(m *testing.M) { + unittest.MainTest(m, filepath.Join("..", "..", "..")) +} + +const AppURL = "http://localhost:3000/" +const Repo = "gogits/gogs" +const AppSubURL = AppURL + Repo + "/" // these values should match the Repo const above var localMetas = map[string]string{ From 9611ff81982a891f631a4f3c14c73742f7d9c214 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 4 Jan 2022 00:19:15 +0800 Subject: [PATCH 04/30] Fix test --- integrations/integration_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/integrations/integration_test.go b/integrations/integration_test.go index 687591d5fa3f..f88b94702c29 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -174,6 +174,7 @@ func initIntegrationTest() { setting.LoadForTest() setting.Repository.DefaultBranch = "master" // many test code still assume that default branch is called "master" _ = util.RemoveAll(repo_module.LocalCopyPath()) + git.Init(context.Background()) git.CheckLFSVersion() setting.InitDBConfig() if err := storage.Init(); err != nil { From 8c78439b779fa3070363053e0e45fe5e4bc5e7cb Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 4 Jan 2022 13:29:58 +0800 Subject: [PATCH 05/30] Fix test --- integrations/migration-test/migration_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/integrations/migration-test/migration_test.go b/integrations/migration-test/migration_test.go index 6a431504a0f2..bb99f05ae122 100644 --- a/integrations/migration-test/migration_test.go +++ b/integrations/migration-test/migration_test.go @@ -82,6 +82,7 @@ func initMigrationTest(t *testing.T) func() { } } + git.Init(context.Background()) git.CheckLFSVersion() setting.InitDBConfig() setting.NewLogServices(true) From b46cf69d39272110bed15761d7bc6005d238dcea Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 5 Jan 2022 13:29:51 +0800 Subject: [PATCH 06/30] Fix test --- models/migrations/migrations_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/models/migrations/migrations_test.go b/models/migrations/migrations_test.go index a1fd49a8b9f2..a8895d236ece 100644 --- a/models/migrations/migrations_test.go +++ b/models/migrations/migrations_test.go @@ -66,6 +66,7 @@ func TestMain(m *testing.M) { setting.SetCustomPathAndConf("", "", "") setting.LoadForTest() + git.Init(context.Background()) git.CheckLFSVersion() setting.InitDBConfig() setting.NewLogServices(true) From b19f50fda569def2aef6a19d51b1f9a982ad97a8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 12 Feb 2022 21:25:35 +0800 Subject: [PATCH 07/30] Merge --- modules/git/command.go | 29 ++++++++++++++++++++---- modules/markup/markdown/markdown_test.go | 8 ++++--- services/repository/files/temp_repo.go | 9 +------- 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/modules/git/command.go b/modules/git/command.go index a82d2ffdb642..bdffc91bc821 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -8,8 +8,14 @@ package git import ( "bytes" "context" +<<<<<<< HEAD "strings" "unsafe" +======= + "io" + "strings" + "time" +>>>>>>> 678661cbf (Merge) "code.gitea.io/gitea/modules/git/cmd" ) @@ -27,12 +33,11 @@ type CommandProxy struct { cmd.Command } -// NewCommand creates and returns a new Git Command based on given command and arguments. // NewCommand creates and returns a new Git Command based on given command and arguments. func NewCommand(ctx context.Context, args ...string) *CommandProxy { - // Make an explicit copy of GlobalCommandArgs, otherwise append might overwrite it - cargs := make([]string, len(GlobalCommandArgs)) - copy(cargs, GlobalCommandArgs) + // Make an explicit copy of globalCommandArgs, otherwise append might overwrite it + cargs := make([]string, len(globalCommandArgs)) + copy(cargs, globalCommandArgs) return &CommandProxy{ Command: cmdService.NewCommand(ctx, len(cargs), append(cargs, args...)...), } @@ -152,3 +157,19 @@ func AllowLFSFiltersArgs() []string { } return filteredLFSGlobalArgs[:j] } + +// AllowLFSFiltersArgs return globalCommandArgs with lfs filter, it should only be used for tests +func AllowLFSFiltersArgs() []string { + // Now here we should explicitly allow lfs filters to run + filteredLFSGlobalArgs := make([]string, len(globalCommandArgs)) + j := 0 + for _, arg := range globalCommandArgs { + if strings.Contains(arg, "lfs") { + j-- + } else { + filteredLFSGlobalArgs[j] = arg + j++ + } + } + return filteredLFSGlobalArgs[:j] +} diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index 903ca6ad1948..3b77bc460e67 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -24,9 +24,11 @@ func TestMain(m *testing.M) { unittest.MainTest(m, filepath.Join("..", "..", "..")) } -const AppURL = "http://localhost:3000/" -const Repo = "gogits/gogs" -const AppSubURL = AppURL + Repo + "/" +const ( + AppURL = "http://localhost:3000/" + Repo = "gogits/gogs" + AppSubURL = AppURL + Repo + "/" +) // these values should match the Repo const above var localMetas = map[string]string{ diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index 9c7d9aafec71..06dec0f5647a 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -18,6 +18,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" + git_cmd "code.gitea.io/gitea/modules/git/cmd" "code.gitea.io/gitea/modules/log" repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" @@ -149,14 +150,6 @@ func (t *TemporaryUploadRepository) RemoveFilesFromIndex(filenames ...string) er Stdin: stdIn, Stdout: stdOut, Stderr: stdErr, - }); err != nil { - log.Error("Unable to update-index for temporary repo: %s (%s) Error: %v\nstdout: %s\nstderr: %s", t.repo.FullName(), t.basePath, err, stdOut.String(), stdErr.String()) - return fmt.Errorf("Unable to update-index for temporary repo: %s Error: %v\nstdout: %s\nstderr: %s", t.repo.FullName(), err, stdOut.String(), stdErr.String()) - } - return nil -} - -// HashObject writes the provided content to the object db and returns its hash func (t *TemporaryUploadRepository) HashObject(content io.Reader) (string, error) { stdOut := new(bytes.Buffer) stdErr := new(bytes.Buffer) From 609a4a14bd7f4419f50cd844a692526a69cbe409 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 13 Feb 2022 01:04:45 +0800 Subject: [PATCH 08/30] Move focus --- modules/git/git.go | 81 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 60 insertions(+), 21 deletions(-) diff --git a/modules/git/git.go b/modules/git/git.go index 79449ea42f4d..35c34de429f6 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -15,6 +15,7 @@ import ( "time" "code.gitea.io/gitea/modules/git/cmd" + git_cmd "code.gitea.io/gitea/modules/git/cmd" "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/setting" @@ -239,47 +240,72 @@ func CheckGitVersionAtLeast(atLeast string) error { } func checkAndSetConfig(key, defaultValue string, forceToDefault bool) error { - stdout, stderr, err := process.GetManager().Exec("git.Init(get setting)", GitExecutable, "config", "--get", key) - if err != nil { + stdout := strings.Builder{} + stderr := strings.Builder{} + if err := NewCommand(DefaultContext, "config", "--get", key). + SetDescription("git.Init(get setting)"). + Run(&git_cmd.Context{ + Timeout: -1, + Stdout: &stdout, + Stderr: &stderr, + }); err != nil { perr, ok := err.(*process.Error) if !ok { - return fmt.Errorf("Failed to get git %s(%v) errType %T: %s", key, err, err, stderr) + return fmt.Errorf("failed to get git %s(%v) errType %T: %s", key, err, err, stderr.String()) } eerr, ok := perr.Err.(*exec.ExitError) if !ok || eerr.ExitCode() != 1 { - return fmt.Errorf("Failed to get git %s(%v) errType %T: %s", key, err, err, stderr) + return fmt.Errorf("failed to get git %s(%v) errType %T: %s", key, err, err, stderr.String()) } } - currValue := strings.TrimSpace(stdout) - + currValue := strings.TrimSpace(stdout.String()) if currValue == defaultValue || (!forceToDefault && len(currValue) > 0) { return nil } - if _, stderr, err = process.GetManager().Exec(fmt.Sprintf("git.Init(set %s)", key), "git", "config", "--global", key, defaultValue); err != nil { - return fmt.Errorf("Failed to set git %s(%s): %s", key, err, stderr) + stderr.Reset() + + if err := NewCommand(DefaultContext, "config", "--global", key, defaultValue). + SetDescription(fmt.Sprintf("git.Init(set %s)", key)). + Run(&git_cmd.Context{ + Timeout: -1, + Stderr: &stderr, + }); err != nil { + return fmt.Errorf("failed to set git %s(%s): %s", key, err, stderr.String()) } return nil } func checkAndAddConfig(key, value string) error { - _, stderr, err := process.GetManager().Exec("git.Init(get setting)", GitExecutable, "config", "--get", key, value) - if err != nil { + stdout := strings.Builder{} + stderr := strings.Builder{} + if err := NewCommand(DefaultContext, "config", "--get", key). + SetDescription("git.Init(get setting)"). + Run(&git_cmd.Context{ + Timeout: -1, + Stdout: &stdout, + Stderr: &stderr, + }); err != nil { perr, ok := err.(*process.Error) if !ok { - return fmt.Errorf("Failed to get git %s(%v) errType %T: %s", key, err, err, stderr) + return fmt.Errorf("failed to get git %s(%v) errType %T: %s", key, err, err, stderr.String()) } eerr, ok := perr.Err.(*exec.ExitError) if !ok || eerr.ExitCode() != 1 { - return fmt.Errorf("Failed to get git %s(%v) errType %T: %s", key, err, err, stderr) + return fmt.Errorf("failed to get git %s(%v) errType %T: %s", key, err, err, stderr.String()) } if eerr.ExitCode() == 1 { - if _, stderr, err = process.GetManager().Exec(fmt.Sprintf("git.Init(set %s)", key), "git", "config", "--global", "--add", key, value); err != nil { - return fmt.Errorf("Failed to set git %s(%s): %s", key, err, stderr) + stderr.Reset() + if err := NewCommand(DefaultContext, "config", "--global", "--add", key, value). + SetDescription(fmt.Sprintf("git.Init(set %s)", key)). + Run(&git_cmd.Context{ + Timeout: -1, + Stderr: &stderr, + }); err != nil { + return fmt.Errorf("failed to set git %s(%s): %s", key, err, stderr.String()) } - return nil } } @@ -287,23 +313,36 @@ func checkAndAddConfig(key, value string) error { } func checkAndRemoveConfig(key, value string) error { - _, stderr, err := process.GetManager().Exec("git.Init(get setting)", GitExecutable, "config", "--get", key, value) - if err != nil { + stdout := strings.Builder{} + stderr := strings.Builder{} + if err := NewCommand(DefaultContext, "config", "--get", key). + SetDescription("git.Init(get setting)"). + Run(&git_cmd.Context{ + Timeout: -1, + Stdout: &stdout, + Stderr: &stderr, + }); err != nil { perr, ok := err.(*process.Error) if !ok { - return fmt.Errorf("Failed to get git %s(%v) errType %T: %s", key, err, err, stderr) + return fmt.Errorf("failed to get git %s(%v) errType %T: %s", key, err, err, stderr.String()) } eerr, ok := perr.Err.(*exec.ExitError) if !ok || eerr.ExitCode() != 1 { - return fmt.Errorf("Failed to get git %s(%v) errType %T: %s", key, err, err, stderr) + return fmt.Errorf("failed to get git %s(%v) errType %T: %s", key, err, err, stderr.String()) } if eerr.ExitCode() == 1 { return nil } } - if _, stderr, err = process.GetManager().Exec(fmt.Sprintf("git.Init(set %s)", key), "git", "config", "--global", "--unset-all", key, value); err != nil { - return fmt.Errorf("Failed to set git %s(%s): %s", key, err, stderr) + stderr.Reset() + if err := NewCommand(DefaultContext, "config", "--global", "--unset-all", key, value). + SetDescription(fmt.Sprintf("git.Init(set %s)", key)). + Run(&git_cmd.Context{ + Timeout: -1, + Stderr: &stderr, + }); err != nil { + return fmt.Errorf("failed to set git %s(%s): %s", key, err, stderr.String()) } return nil From f8dd1dec46a5cf68394c18b080e585d30ac71ccd Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 13 Feb 2022 16:00:19 +0800 Subject: [PATCH 09/30] Fix lint --- modules/git/git.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/modules/git/git.go b/modules/git/git.go index 35c34de429f6..3649fe71e020 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -14,7 +14,6 @@ import ( "strings" "time" - "code.gitea.io/gitea/modules/git/cmd" git_cmd "code.gitea.io/gitea/modules/git/cmd" "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/setting" @@ -140,13 +139,13 @@ func Init(ctx context.Context) error { DefaultContext = ctx if setting.Git.Timeout.Default > 0 { - cmd.SetDefaultCommandTimout(time.Duration(setting.Git.Timeout.Default) * time.Second) + git_cmd.SetDefaultCommandTimout(time.Duration(setting.Git.Timeout.Default) * time.Second) } if err := SetExecutablePath(setting.Git.Path); err != nil { return err } - cmdService = cmd.NewLocalService(GitExecutable, setting.RepoRootPath) + cmdService = git_cmd.NewLocalService(GitExecutable, setting.RepoRootPath) if err := checkGitVersion(); err != nil { return err From c176e592cbefa81bd888d35b18036df99f83fcef Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 16 Feb 2022 16:08:18 +0800 Subject: [PATCH 10/30] Fix test --- models/unittest/testdb.go | 5 ----- modules/git/command.go | 38 +++++++++++++------------------------- modules/git/git.go | 2 -- 3 files changed, 13 insertions(+), 32 deletions(-) diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index 92b514a3ccb4..117614a7a4f3 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -14,7 +14,6 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/base" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/util" @@ -111,10 +110,6 @@ func MainTest(m *testing.M, testOpts *TestOptions) { fatalTestError("storage.Init: %v\n", err) } - if err = git.Init(context.Background()); err != nil { - fatalTestError("git.Init: %v\n", err) - } - if err = util.RemoveAll(repoRootPath); err != nil { fatalTestError("util.RemoveAll: %v\n", err) } diff --git a/modules/git/command.go b/modules/git/command.go index bdffc91bc821..5496df93c421 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -8,16 +8,12 @@ package git import ( "bytes" "context" -<<<<<<< HEAD "strings" + "sync" "unsafe" -======= - "io" - "strings" - "time" ->>>>>>> 678661cbf (Merge) "code.gitea.io/gitea/modules/git/cmd" + "code.gitea.io/gitea/modules/setting" ) var ( @@ -25,9 +21,17 @@ var ( globalCommandArgs []string // cmdService represents a command service - cmdService cmd.Service + cmdService cmd.Service + cmdServiceOnce sync.Once ) +func getCmdService() cmd.Service { + cmdServiceOnce.Do(func() { + cmdService = cmd.NewLocalService(GitExecutable, setting.RepoRootPath) + }) + return cmdService +} + // CommandProxy represents a command proxy with its subcommands or arguments. type CommandProxy struct { cmd.Command @@ -39,7 +43,7 @@ func NewCommand(ctx context.Context, args ...string) *CommandProxy { cargs := make([]string, len(globalCommandArgs)) copy(cargs, globalCommandArgs) return &CommandProxy{ - Command: cmdService.NewCommand(ctx, len(cargs), append(cargs, args...)...), + Command: getCmdService().NewCommand(ctx, len(cargs), append(cargs, args...)...), } } @@ -51,7 +55,7 @@ func NewCommandNoGlobals(args ...string) *CommandProxy { // NewCommandContextNoGlobals creates and returns a new Git Command based on given command and arguments only with the specify args and don't care global command args func NewCommandContextNoGlobals(ctx context.Context, args ...string) *CommandProxy { return &CommandProxy{ - Command: cmdService.NewCommand(ctx, 0, args...), + Command: getCmdService().NewCommand(ctx, 0, args...), } } @@ -157,19 +161,3 @@ func AllowLFSFiltersArgs() []string { } return filteredLFSGlobalArgs[:j] } - -// AllowLFSFiltersArgs return globalCommandArgs with lfs filter, it should only be used for tests -func AllowLFSFiltersArgs() []string { - // Now here we should explicitly allow lfs filters to run - filteredLFSGlobalArgs := make([]string, len(globalCommandArgs)) - j := 0 - for _, arg := range globalCommandArgs { - if strings.Contains(arg, "lfs") { - j-- - } else { - filteredLFSGlobalArgs[j] = arg - j++ - } - } - return filteredLFSGlobalArgs[:j] -} diff --git a/modules/git/git.go b/modules/git/git.go index 3649fe71e020..c1b11234ae44 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -145,8 +145,6 @@ func Init(ctx context.Context) error { return err } - cmdService = git_cmd.NewLocalService(GitExecutable, setting.RepoRootPath) - if err := checkGitVersion(); err != nil { return err } From b52f7976f4cf43fad3aabd6ff455417ed6ba0c05 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 16 Feb 2022 16:22:54 +0800 Subject: [PATCH 11/30] Remove unnecessary change --- cmd/hook.go | 6 ------ cmd/serv.go | 3 --- integrations/integration_test.go | 3 --- models/migrations/migrations_test.go | 1 - 4 files changed, 13 deletions(-) diff --git a/cmd/hook.go b/cmd/hook.go index 6e535170901b..8078763b183f 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -218,9 +218,6 @@ Gitea or set your environment appropriately.`, "") } } - if err := git.Init(ctx); err != nil { - return fail("Git init failed", err.Error()) - } supportProcRecive := false if git.CheckGitVersionAtLeast("2.29") == nil { supportProcRecive = true @@ -501,9 +498,6 @@ Gitea or set your environment appropriately.`, "") ctx, cancel := installSignals() defer cancel() - if err := git.Init(ctx); err != nil { - return fail("Git init failed", err.Error()) - } if git.CheckGitVersionAtLeast("2.29") != nil { return fail("Internal Server Error", "git not support proc-receive.") } diff --git a/cmd/serv.go b/cmd/serv.go index 3e4e4d3dc868..6ba3e9de01ff 100644 --- a/cmd/serv.go +++ b/cmd/serv.go @@ -151,9 +151,6 @@ func runServ(c *cli.Context) error { } if len(words) < 2 { - if err := git.Init(ctx); err != nil { - return fail("Git init failed", err.Error()) - } if git.CheckGitVersionAtLeast("2.29") == nil { // for AGit Flow if cmd == "ssh_info" { diff --git a/integrations/integration_test.go b/integrations/integration_test.go index f88b94702c29..ef5fc8abbdaf 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -26,7 +26,6 @@ import ( "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/base" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" @@ -174,8 +173,6 @@ func initIntegrationTest() { setting.LoadForTest() setting.Repository.DefaultBranch = "master" // many test code still assume that default branch is called "master" _ = util.RemoveAll(repo_module.LocalCopyPath()) - git.Init(context.Background()) - git.CheckLFSVersion() setting.InitDBConfig() if err := storage.Init(); err != nil { fmt.Printf("Init storage failed: %v", err) diff --git a/models/migrations/migrations_test.go b/models/migrations/migrations_test.go index a8895d236ece..a1fd49a8b9f2 100644 --- a/models/migrations/migrations_test.go +++ b/models/migrations/migrations_test.go @@ -66,7 +66,6 @@ func TestMain(m *testing.M) { setting.SetCustomPathAndConf("", "", "") setting.LoadForTest() - git.Init(context.Background()) git.CheckLFSVersion() setting.InitDBConfig() setting.NewLogServices(true) From 11569c3c58a16332f996ddce131a8b53999724b0 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 16 Feb 2022 17:37:51 +0800 Subject: [PATCH 12/30] fix test --- integrations/migration-test/migration_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/integrations/migration-test/migration_test.go b/integrations/migration-test/migration_test.go index bb99f05ae122..6a431504a0f2 100644 --- a/integrations/migration-test/migration_test.go +++ b/integrations/migration-test/migration_test.go @@ -82,7 +82,6 @@ func initMigrationTest(t *testing.T) func() { } } - git.Init(context.Background()) git.CheckLFSVersion() setting.InitDBConfig() setting.NewLogServices(true) From 3c9c1fe9179d52abfe36a2dd8e238b53750365e9 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 16 Feb 2022 19:33:09 +0800 Subject: [PATCH 13/30] Fix test --- modules/git/git.go | 4 +--- modules/markup/html_test.go | 6 ------ modules/markup/markdown/markdown_test.go | 6 ------ 3 files changed, 1 insertion(+), 15 deletions(-) diff --git a/modules/git/git.go b/modules/git/git.go index c1b11234ae44..8f130821d5f7 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -310,13 +310,11 @@ func checkAndAddConfig(key, value string) error { } func checkAndRemoveConfig(key, value string) error { - stdout := strings.Builder{} stderr := strings.Builder{} - if err := NewCommand(DefaultContext, "config", "--get", key). + if err := NewCommand(DefaultContext, "config", "--get", key, value). SetDescription("git.Init(get setting)"). Run(&git_cmd.Context{ Timeout: -1, - Stdout: &stdout, Stderr: &stderr, }); err != nil { perr, ok := err.(*process.Error) diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index 91ad19473f30..f6aabc6272e4 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -6,11 +6,9 @@ package markup_test import ( "io" - "path/filepath" "strings" "testing" - "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/emoji" "code.gitea.io/gitea/modules/git" . "code.gitea.io/gitea/modules/markup" @@ -27,10 +25,6 @@ var localMetas = map[string]string{ "repoPath": "../../integrations/gitea-repositories-meta/user13/repo11.git/", } -func TestMain(m *testing.M) { - unittest.MainTest(m, filepath.Join("..", "..")) -} - 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 3b77bc460e67..a069d402bb50 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -5,11 +5,9 @@ package markdown_test import ( - "path/filepath" "strings" "testing" - "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" @@ -20,10 +18,6 @@ import ( "github.com/stretchr/testify/assert" ) -func TestMain(m *testing.M) { - unittest.MainTest(m, filepath.Join("..", "..", "..")) -} - const ( AppURL = "http://localhost:3000/" Repo = "gogits/gogs" From 9d4cc7ee02b906eb70246b9a9bba0fae4ef203c0 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 28 Feb 2022 22:32:14 +0800 Subject: [PATCH 14/30] Fix test --- services/release/release_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/release/release_test.go b/services/release/release_test.go index 0f5b74f70d57..4a866372e74a 100644 --- a/services/release/release_test.go +++ b/services/release/release_test.go @@ -5,6 +5,7 @@ package release import ( + "context" "path/filepath" "strings" "testing" @@ -25,6 +26,7 @@ func TestMain(m *testing.M) { unittest.MainTest(m, &unittest.TestOptions{ GiteaRootPath: filepath.Join("..", ".."), }) + git.Init(context.Background()) } func TestRelease_Create(t *testing.T) { From e03150af39f3f253b54dee2c42c44fe23d67f7aa Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 2 Mar 2022 13:18:44 +0800 Subject: [PATCH 15/30] Fix test --- models/unittest/testdb.go | 5 +++++ services/release/release_test.go | 2 -- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index 117614a7a4f3..92b514a3ccb4 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/util" @@ -110,6 +111,10 @@ func MainTest(m *testing.M, testOpts *TestOptions) { fatalTestError("storage.Init: %v\n", err) } + if err = git.Init(context.Background()); err != nil { + fatalTestError("git.Init: %v\n", err) + } + if err = util.RemoveAll(repoRootPath); err != nil { fatalTestError("util.RemoveAll: %v\n", err) } diff --git a/services/release/release_test.go b/services/release/release_test.go index 4a866372e74a..0f5b74f70d57 100644 --- a/services/release/release_test.go +++ b/services/release/release_test.go @@ -5,7 +5,6 @@ package release import ( - "context" "path/filepath" "strings" "testing" @@ -26,7 +25,6 @@ func TestMain(m *testing.M) { unittest.MainTest(m, &unittest.TestOptions{ GiteaRootPath: filepath.Join("..", ".."), }) - git.Init(context.Background()) } func TestRelease_Create(t *testing.T) { From d8c93ae62f64b1f17a33bf1c2849984e2150ee31 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 21 Mar 2022 11:07:26 +0800 Subject: [PATCH 16/30] Don't allow config git globally on two or more goroutines --- modules/git/git.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/modules/git/git.go b/modules/git/git.go index 8f130821d5f7..8394fe94c1a8 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -12,6 +12,7 @@ import ( "os/exec" "runtime" "strings" + "sync" "time" git_cmd "code.gitea.io/gitea/modules/git/cmd" @@ -236,7 +237,13 @@ func CheckGitVersionAtLeast(atLeast string) error { return nil } +var gitConfigLock sync.Mutex + +// checkAndSetConfig to avoid config conflict, only allow one go routine call at the same time func checkAndSetConfig(key, defaultValue string, forceToDefault bool) error { + gitConfigLock.Lock() + defer gitConfigLock.Unlock() + stdout := strings.Builder{} stderr := strings.Builder{} if err := NewCommand(DefaultContext, "config", "--get", key). From b2bedf8e4dae554e9686a7399a5f8e912a7cfa21 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 21 Mar 2022 17:07:49 +0800 Subject: [PATCH 17/30] Fix test --- modules/git/git.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/modules/git/git.go b/modules/git/git.go index 8394fe94c1a8..e190e0722834 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -253,11 +253,7 @@ func checkAndSetConfig(key, defaultValue string, forceToDefault bool) error { Stdout: &stdout, Stderr: &stderr, }); err != nil { - perr, ok := err.(*process.Error) - if !ok { - return fmt.Errorf("failed to get git %s(%v) errType %T: %s", key, err, err, stderr.String()) - } - eerr, ok := perr.Err.(*exec.ExitError) + eerr, ok := err.(*exec.ExitError) if !ok || eerr.ExitCode() != 1 { return fmt.Errorf("failed to get git %s(%v) errType %T: %s", key, err, err, stderr.String()) } From 9dc2ade5cea86f40f82b492010e3f973ee4ec7e0 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 21 Mar 2022 19:32:39 +0800 Subject: [PATCH 18/30] Fix test --- modules/git/git.go | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/modules/git/git.go b/modules/git/git.go index e190e0722834..310e0d793cc0 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -16,7 +16,6 @@ import ( "time" git_cmd "code.gitea.io/gitea/modules/git/cmd" - "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/setting" "github.com/hashicorp/go-version" @@ -288,11 +287,7 @@ func checkAndAddConfig(key, value string) error { Stdout: &stdout, Stderr: &stderr, }); err != nil { - perr, ok := err.(*process.Error) - if !ok { - return fmt.Errorf("failed to get git %s(%v) errType %T: %s", key, err, err, stderr.String()) - } - eerr, ok := perr.Err.(*exec.ExitError) + eerr, ok := err.(*exec.ExitError) if !ok || eerr.ExitCode() != 1 { return fmt.Errorf("failed to get git %s(%v) errType %T: %s", key, err, err, stderr.String()) } @@ -320,11 +315,7 @@ func checkAndRemoveConfig(key, value string) error { Timeout: -1, Stderr: &stderr, }); err != nil { - perr, ok := err.(*process.Error) - if !ok { - return fmt.Errorf("failed to get git %s(%v) errType %T: %s", key, err, err, stderr.String()) - } - eerr, ok := perr.Err.(*exec.ExitError) + eerr, ok := err.(*exec.ExitError) if !ok || eerr.ExitCode() != 1 { return fmt.Errorf("failed to get git %s(%v) errType %T: %s", key, err, err, stderr.String()) } From 2bd97c2ee03f033f0b749892728c62182b6d4675 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 23 Mar 2022 10:52:54 +0800 Subject: [PATCH 19/30] Fix test --- integrations/integration_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/integrations/integration_test.go b/integrations/integration_test.go index ef5fc8abbdaf..b46e75cc1f4e 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -26,6 +26,7 @@ import ( "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" @@ -173,9 +174,18 @@ func initIntegrationTest() { setting.LoadForTest() setting.Repository.DefaultBranch = "master" // many test code still assume that default branch is called "master" _ = util.RemoveAll(repo_module.LocalCopyPath()) + + util.RemoveAll(setting.RepoRootPath) + util.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"), setting.RepoRootPath) + + if err := git.Init(context.Background()); err != nil { + fmt.Printf("Init git failed: %v\n", err) + os.Exit(1) + } + git.CheckLFSVersion() setting.InitDBConfig() if err := storage.Init(); err != nil { - fmt.Printf("Init storage failed: %v", err) + fmt.Printf("Init storage failed: %v\n", err) os.Exit(1) } From 57f630730537b504b329283bbfa15de2c511b8b7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 23 Mar 2022 14:50:10 +0800 Subject: [PATCH 20/30] Remove unnecessary change --- integrations/integration_test.go | 7 ------- models/unittest/testdb.go | 5 ----- modules/git/cmd/command.go | 8 -------- modules/git/cmd/local.go | 15 +++++++++------ modules/git/command.go | 2 +- modules/git/git.go | 3 --- 6 files changed, 10 insertions(+), 30 deletions(-) diff --git a/integrations/integration_test.go b/integrations/integration_test.go index b46e75cc1f4e..299c506c3648 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -175,13 +175,6 @@ func initIntegrationTest() { setting.Repository.DefaultBranch = "master" // many test code still assume that default branch is called "master" _ = util.RemoveAll(repo_module.LocalCopyPath()) - util.RemoveAll(setting.RepoRootPath) - util.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"), setting.RepoRootPath) - - if err := git.Init(context.Background()); err != nil { - fmt.Printf("Init git failed: %v\n", err) - os.Exit(1) - } git.CheckLFSVersion() setting.InitDBConfig() if err := storage.Init(); err != nil { diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index 92b514a3ccb4..117614a7a4f3 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -14,7 +14,6 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/base" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/util" @@ -111,10 +110,6 @@ func MainTest(m *testing.M, testOpts *TestOptions) { fatalTestError("storage.Init: %v\n", err) } - if err = git.Init(context.Background()); err != nil { - fatalTestError("git.Init: %v\n", err) - } - if err = util.RemoveAll(repoRootPath); err != nil { fatalTestError("util.RemoveAll: %v\n", err) } diff --git a/modules/git/cmd/command.go b/modules/git/cmd/command.go index 8342439809a4..f7d70e27a311 100644 --- a/modules/git/cmd/command.go +++ b/modules/git/cmd/command.go @@ -10,14 +10,6 @@ import ( "time" ) -// defaultCommandExecutionTimeout default command execution timeout duration -var defaultCommandExecutionTimeout = 360 * time.Second - -// SetDefaultCommandTimout set the default timeout -func SetDefaultCommandTimout(timeout time.Duration) { - defaultCommandExecutionTimeout = timeout -} - // RunOpts represents parameters to run the command type RunOpts struct { Env []string diff --git a/modules/git/cmd/local.go b/modules/git/cmd/local.go index c0abcfcb40a5..d4f792ebd0c1 100644 --- a/modules/git/cmd/local.go +++ b/modules/git/cmd/local.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "strings" + "time" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/process" @@ -62,7 +63,7 @@ func (c *LocalCommand) Run(opts *RunOpts) error { opts = &RunOpts{} } if opts.Timeout <= 0 { - opts.Timeout = defaultCommandExecutionTimeout + opts.Timeout = c.service.defaultTimeout } if len(opts.Dir) == 0 { @@ -136,17 +137,19 @@ func (c *LocalCommand) Run(opts *RunOpts) error { // LocalService represents a command service to create local git commands type LocalService struct { - GitExecutable string // git binary location - RepoRootPath string // repository storage root directory + GitExecutable string // git binary location + RepoRootPath string // repository storage root directory + defaultTimeout time.Duration } var _ Service = &LocalService{} // NewLocalService returns a local service -func NewLocalService(gitExecutable, repoRootPath string) *LocalService { +func NewLocalService(gitExecutable, repoRootPath string, defaultTimeout time.Duration) *LocalService { return &LocalService{ - GitExecutable: gitExecutable, - RepoRootPath: repoRootPath, + GitExecutable: gitExecutable, + RepoRootPath: repoRootPath, + defaultTimeout: defaultTimeout, } } diff --git a/modules/git/command.go b/modules/git/command.go index 5496df93c421..b680e6aaad8d 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -27,7 +27,7 @@ var ( func getCmdService() cmd.Service { cmdServiceOnce.Do(func() { - cmdService = cmd.NewLocalService(GitExecutable, setting.RepoRootPath) + cmdService = cmd.NewLocalService(GitExecutable, setting.RepoRootPath, time.Duration(setting.Git.Timeout.Default)*time.Second) }) return cmdService } diff --git a/modules/git/git.go b/modules/git/git.go index 310e0d793cc0..c80edc7a5786 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -138,9 +138,6 @@ func VersionInfo() string { func Init(ctx context.Context) error { DefaultContext = ctx - if setting.Git.Timeout.Default > 0 { - git_cmd.SetDefaultCommandTimout(time.Duration(setting.Git.Timeout.Default) * time.Second) - } if err := SetExecutablePath(setting.Git.Path); err != nil { return err } From 100c5a59ada24b0d3ebd2738745b8819e39b8c9d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 23 Mar 2022 15:32:01 +0800 Subject: [PATCH 21/30] Remove unnecessary change --- modules/git/git.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/modules/git/git.go b/modules/git/git.go index c80edc7a5786..1abe87c1c84f 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -93,11 +93,8 @@ func SetExecutablePath(path string) error { return fmt.Errorf("git not found: %w", err) } GitExecutable = absPath - return nil -} -func checkGitVersion() error { - err := LoadGitVersion() + err = LoadGitVersion() if err != nil { return fmt.Errorf("unable to load git version: %w", err) } @@ -118,6 +115,7 @@ func checkGitVersion() error { } return fmt.Errorf("installed git version %q is not supported, Gitea requires git version >= %q, %s", gitVersion.Original(), GitVersionRequired, moreHint) } + return nil } @@ -142,10 +140,6 @@ func Init(ctx context.Context) error { return err } - if err := checkGitVersion(); err != nil { - return err - } - // force cleanup args globalCommandArgs = []string{} @@ -164,6 +158,11 @@ func Init(ctx context.Context) error { globalCommandArgs = append(globalCommandArgs, "-c", "uploadpack.allowfilter=true", "-c", "uploadpack.allowAnySHA1InWant=true") } + // Save current git version on init to gitVersion otherwise it would require an RWMutex + if err := LoadGitVersion(); err != nil { + return err + } + // Git requires setting user.name and user.email in order to commit changes - if they're not set just add some defaults for configKey, defaultValue := range map[string]string{"user.name": "Gitea", "user.email": "gitea@fake.local"} { if err := checkAndSetConfig(configKey, defaultValue, false); err != nil { @@ -275,6 +274,9 @@ func checkAndSetConfig(key, defaultValue string, forceToDefault bool) error { } func checkAndAddConfig(key, value string) error { + gitConfigLock.Lock() + defer gitConfigLock.Unlock() + stdout := strings.Builder{} stderr := strings.Builder{} if err := NewCommand(DefaultContext, "config", "--get", key). @@ -305,6 +307,9 @@ func checkAndAddConfig(key, value string) error { } func checkAndRemoveConfig(key, value string) error { + gitConfigLock.Lock() + defer gitConfigLock.Unlock() + stderr := strings.Builder{} if err := NewCommand(DefaultContext, "config", "--get", key, value). SetDescription("git.Init(get setting)"). From d5736d9767cceb6ffa9df04ad3fb49a357c0220f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 23 Mar 2022 15:54:35 +0800 Subject: [PATCH 22/30] Fix test --- integrations/integration_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/integrations/integration_test.go b/integrations/integration_test.go index 299c506c3648..559d295de5d3 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -175,6 +175,8 @@ func initIntegrationTest() { setting.Repository.DefaultBranch = "master" // many test code still assume that default branch is called "master" _ = util.RemoveAll(repo_module.LocalCopyPath()) + os.MkdirAll(setting.RepoRootPath, os.ModePerm) + git.CheckLFSVersion() setting.InitDBConfig() if err := storage.Init(); err != nil { From f1deefff4744c032e2245d9f169ae28215820820 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 29 Mar 2022 15:13:40 +0800 Subject: [PATCH 23/30] Fix bug --- modules/git/cmd/local.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/cmd/local.go b/modules/git/cmd/local.go index d4f792ebd0c1..90650decb9ce 100644 --- a/modules/git/cmd/local.go +++ b/modules/git/cmd/local.go @@ -88,7 +88,7 @@ func (c *LocalCommand) Run(opts *RunOpts) error { args[urlArgIndex] = util.SanitizeCredentialURLs(args[urlArgIndex]) } } - desc = fmt.Sprintf("%s %s [repo_path: %s]", c.service.GitExecutable, strings.Join(args, " "), opts.Dir) + desc = fmt.Sprintf("%s %s [repo_path: %s]", c.args[0], strings.Join(args, " "), opts.Dir) } ctx, cancel, finished := process.GetManager().AddContextTimeout(c.parentContext, opts.Timeout, desc) From 7413f241479a322e97952967a64530f71d8707d3 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 1 Apr 2022 14:54:32 +0800 Subject: [PATCH 24/30] Fix bug --- modules/git/command.go | 1 + modules/git/git.go | 12 ++++++------ services/repository/files/temp_repo.go | 8 +++++++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/modules/git/command.go b/modules/git/command.go index b680e6aaad8d..225de1ee8030 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -10,6 +10,7 @@ import ( "context" "strings" "sync" + "time" "unsafe" "code.gitea.io/gitea/modules/git/cmd" diff --git a/modules/git/git.go b/modules/git/git.go index 1abe87c1c84f..19eb0fd6e351 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -243,7 +243,7 @@ func checkAndSetConfig(key, defaultValue string, forceToDefault bool) error { stderr := strings.Builder{} if err := NewCommand(DefaultContext, "config", "--get", key). SetDescription("git.Init(get setting)"). - Run(&git_cmd.Context{ + Run(&git_cmd.RunOpts{ Timeout: -1, Stdout: &stdout, Stderr: &stderr, @@ -263,7 +263,7 @@ func checkAndSetConfig(key, defaultValue string, forceToDefault bool) error { if err := NewCommand(DefaultContext, "config", "--global", key, defaultValue). SetDescription(fmt.Sprintf("git.Init(set %s)", key)). - Run(&git_cmd.Context{ + Run(&git_cmd.RunOpts{ Timeout: -1, Stderr: &stderr, }); err != nil { @@ -281,7 +281,7 @@ func checkAndAddConfig(key, value string) error { stderr := strings.Builder{} if err := NewCommand(DefaultContext, "config", "--get", key). SetDescription("git.Init(get setting)"). - Run(&git_cmd.Context{ + Run(&git_cmd.RunOpts{ Timeout: -1, Stdout: &stdout, Stderr: &stderr, @@ -294,7 +294,7 @@ func checkAndAddConfig(key, value string) error { stderr.Reset() if err := NewCommand(DefaultContext, "config", "--global", "--add", key, value). SetDescription(fmt.Sprintf("git.Init(set %s)", key)). - Run(&git_cmd.Context{ + Run(&git_cmd.RunOpts{ Timeout: -1, Stderr: &stderr, }); err != nil { @@ -313,7 +313,7 @@ func checkAndRemoveConfig(key, value string) error { stderr := strings.Builder{} if err := NewCommand(DefaultContext, "config", "--get", key, value). SetDescription("git.Init(get setting)"). - Run(&git_cmd.Context{ + Run(&git_cmd.RunOpts{ Timeout: -1, Stderr: &stderr, }); err != nil { @@ -329,7 +329,7 @@ func checkAndRemoveConfig(key, value string) error { stderr.Reset() if err := NewCommand(DefaultContext, "config", "--global", "--unset-all", key, value). SetDescription(fmt.Sprintf("git.Init(set %s)", key)). - Run(&git_cmd.Context{ + Run(&git_cmd.RunOpts{ Timeout: -1, Stderr: &stderr, }); err != nil { diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index 06dec0f5647a..27261c9a5a34 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -18,7 +18,6 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" - git_cmd "code.gitea.io/gitea/modules/git/cmd" "code.gitea.io/gitea/modules/log" repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" @@ -150,6 +149,13 @@ func (t *TemporaryUploadRepository) RemoveFilesFromIndex(filenames ...string) er Stdin: stdIn, Stdout: stdOut, Stderr: stdErr, + }); err != nil { + log.Error("Unable to update-index for temporary repo: %s (%s) Error: %v\nstdout: %s\nstderr: %s", t.repo.FullName(), t.basePath, err, stdOut.String(), stdErr.String()) + return fmt.Errorf("Unable to update-index for temporary repo: %s Error: %v\nstdout: %s\nstderr: %s", t.repo.FullName(), err, stdOut.String(), stdErr.String()) + } + return nil +} + func (t *TemporaryUploadRepository) HashObject(content io.Reader) (string, error) { stdOut := new(bytes.Buffer) stdErr := new(bytes.Buffer) From ad77361a0ec5acd8080345fd4d1bcd2f5bc736c3 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 1 Apr 2022 14:58:03 +0800 Subject: [PATCH 25/30] Get back missed comment --- services/repository/files/temp_repo.go | 1 + 1 file changed, 1 insertion(+) diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index 27261c9a5a34..9c7d9aafec71 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -156,6 +156,7 @@ func (t *TemporaryUploadRepository) RemoveFilesFromIndex(filenames ...string) er return nil } +// HashObject writes the provided content to the object db and returns its hash func (t *TemporaryUploadRepository) HashObject(content io.Reader) (string, error) { stdOut := new(bytes.Buffer) stdErr := new(bytes.Buffer) From 7e2466b2a77d71892797af674c727802a59e2f66 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 2 Apr 2022 16:55:46 +0800 Subject: [PATCH 26/30] Fix test --- modules/setting/setting.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 5e317b39ea28..149a2f86a78e 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -768,7 +768,11 @@ func loadFromConf(allowEmpty bool, extraConfig string) { } StaticRootPath = sec.Key("STATIC_ROOT_PATH").MustString(StaticRootPath) StaticCacheTime = sec.Key("STATIC_CACHE_TIME").MustDuration(6 * time.Hour) - AppDataPath = sec.Key("APP_DATA_PATH").MustString(path.Join(AppWorkPath, "data")) + appDataPath := sec.Key("APP_DATA_PATH").MustString(path.Join(AppWorkPath, "data")) + AppDataPath, err = filepath.Abs(appDataPath) + if err != nil { + log.Fatal("Invalid APP_DATA_PATH '%s': %s", appDataPath, err) + } EnableGzip = sec.Key("ENABLE_GZIP").MustBool() EnablePprof = sec.Key("ENABLE_PPROF").MustBool(false) From 56c85a318e2778f43c9192a6c7184207a72be4c9 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 2 Apr 2022 20:17:16 +0800 Subject: [PATCH 27/30] Move Git executable to local command --- modules/git/batch_reader.go | 6 +++--- modules/git/blame.go | 3 ++- modules/git/cmd/local.go | 16 +++++++++++++++- modules/git/command.go | 2 +- modules/git/git.go | 22 ++++------------------ routers/web/repo/http.go | 2 +- 6 files changed, 26 insertions(+), 25 deletions(-) diff --git a/modules/git/batch_reader.go b/modules/git/batch_reader.go index 902fa897185f..17c66f4887a8 100644 --- a/modules/git/batch_reader.go +++ b/modules/git/batch_reader.go @@ -33,7 +33,7 @@ type WriteCloserError interface { func EnsureValidGitRepository(ctx context.Context, repoPath string) error { stderr := strings.Builder{} err := NewCommand(ctx, "rev-parse"). - SetDescription(fmt.Sprintf("%s rev-parse [repo_path: %s]", GitExecutable, repoPath)). + SetDescription(fmt.Sprintf("rev-parse [repo_path: %s]", repoPath)). Run(&RunOpts{ Dir: repoPath, Stderr: &stderr, @@ -69,7 +69,7 @@ func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError, go func() { stderr := strings.Builder{} err := NewCommand(ctx, "cat-file", "--batch-check"). - SetDescription(fmt.Sprintf("%s cat-file --batch-check [repo_path: %s] (%s:%d)", GitExecutable, repoPath, filename, line)). + SetDescription(fmt.Sprintf("cat-file --batch-check [repo_path: %s] (%s:%d)", repoPath, filename, line)). Run(&RunOpts{ Dir: repoPath, Stdin: batchStdinReader, @@ -119,7 +119,7 @@ func CatFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufi go func() { stderr := strings.Builder{} err := NewCommand(ctx, "cat-file", "--batch"). - SetDescription(fmt.Sprintf("%s cat-file --batch [repo_path: %s] (%s:%d)", GitExecutable, repoPath, filename, line)). + SetDescription(fmt.Sprintf("cat-file --batch [repo_path: %s] (%s:%d)", repoPath, filename, line)). Run(&RunOpts{ Dir: repoPath, Stdin: batchStdinReader, diff --git a/modules/git/blame.go b/modules/git/blame.go index 1653ecbf854a..f7cc2904b6b7 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -14,6 +14,7 @@ import ( "regexp" "code.gitea.io/gitea/modules/process" + "code.gitea.io/gitea/modules/setting" ) // BlamePart represents block of blame - continuous lines with one sha @@ -114,7 +115,7 @@ func (r *BlameReader) Close() error { // CreateBlameReader creates reader for given repository, commit and file func CreateBlameReader(ctx context.Context, repoPath, commitID, file string) (*BlameReader, error) { - return createBlameReader(ctx, repoPath, GitExecutable, "blame", commitID, "--porcelain", "--", file) + return createBlameReader(ctx, repoPath, setting.Git.Path, "blame", commitID, "--porcelain", "--", file) } func createBlameReader(ctx context.Context, dir string, command ...string) (*BlameReader, error) { diff --git a/modules/git/cmd/local.go b/modules/git/cmd/local.go index 90650decb9ce..eb8dee74b6b5 100644 --- a/modules/git/cmd/local.go +++ b/modules/git/cmd/local.go @@ -90,6 +90,7 @@ func (c *LocalCommand) Run(opts *RunOpts) error { } desc = fmt.Sprintf("%s %s [repo_path: %s]", c.args[0], strings.Join(args, " "), opts.Dir) } + desc = fmt.Sprintf("[%s] %s", c.service.GitExecutable, desc) ctx, cancel, finished := process.GetManager().AddContextTimeout(c.parentContext, opts.Timeout, desc) defer finished() @@ -135,6 +136,10 @@ func (c *LocalCommand) Run(opts *RunOpts) error { return ctx.Err() } +// defaultGitExecutable is the command name of git +// Could be updated to an absolute path while initialization +const defaultGitExecutable = "git" + // LocalService represents a command service to create local git commands type LocalService struct { GitExecutable string // git binary location @@ -146,8 +151,17 @@ var _ Service = &LocalService{} // NewLocalService returns a local service func NewLocalService(gitExecutable, repoRootPath string, defaultTimeout time.Duration) *LocalService { + // If path is empty, we use the default value of GitExecutable "git" to search for the location of git. + if gitExecutable == "" { + gitExecutable = defaultGitExecutable + } + absPath, err := exec.LookPath(gitExecutable) + if err != nil { + panic(fmt.Sprintf("Git not found: %v", err)) + } + return &LocalService{ - GitExecutable: gitExecutable, + GitExecutable: absPath, RepoRootPath: repoRootPath, defaultTimeout: defaultTimeout, } diff --git a/modules/git/command.go b/modules/git/command.go index 225de1ee8030..e98ee7b9b543 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -28,7 +28,7 @@ var ( func getCmdService() cmd.Service { cmdServiceOnce.Do(func() { - cmdService = cmd.NewLocalService(GitExecutable, setting.RepoRootPath, time.Duration(setting.Git.Timeout.Default)*time.Second) + cmdService = cmd.NewLocalService(setting.Git.Path, setting.RepoRootPath, time.Duration(setting.Git.Timeout.Default)*time.Second) }) return cmdService } diff --git a/modules/git/git.go b/modules/git/git.go index 19eb0fd6e351..563ddba1d336 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -28,10 +28,6 @@ var ( // 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 - // 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 Init with HammerContext DefaultContext = context.Background() @@ -82,19 +78,9 @@ func LoadGitVersion() error { return err } -// SetExecutablePath changes the path of git executable and checks the file permission and version. -func SetExecutablePath(path string) error { - // If path is empty, we use the default value of GitExecutable "git" to search for the location of git. - if path != "" { - GitExecutable = path - } - absPath, err := exec.LookPath(GitExecutable) - if err != nil { - return fmt.Errorf("git not found: %w", err) - } - GitExecutable = absPath - - err = LoadGitVersion() +// checkGitVersion checks the file permission and version. +func checkGitVersion() error { + err := LoadGitVersion() if err != nil { return fmt.Errorf("unable to load git version: %w", err) } @@ -136,7 +122,7 @@ func VersionInfo() string { func Init(ctx context.Context) error { DefaultContext = ctx - if err := SetExecutablePath(setting.Git.Path); err != nil { + if err := checkGitVersion(); err != nil { return err } diff --git a/routers/web/repo/http.go b/routers/web/repo/http.go index 6a85bca16b2f..ca41172b50b7 100644 --- a/routers/web/repo/http.go +++ b/routers/web/repo/http.go @@ -472,7 +472,7 @@ func serviceRPC(ctx gocontext.Context, h serviceHandler, service string) { var stderr bytes.Buffer cmd := git.NewCommand(h.r.Context(), service, "--stateless-rpc", h.dir) - cmd.SetDescription(fmt.Sprintf("%s %s %s [repo_path: %s]", git.GitExecutable, service, "--stateless-rpc", h.dir)) + cmd.SetDescription(fmt.Sprintf("%s %s [repo_path: %s]", service, "--stateless-rpc", h.dir)) if err := cmd.Run(&git.RunOpts{ Dir: h.dir, Env: append(os.Environ(), h.environ...), From 764faefbd7fea3db05c4c1d59c65b11fef8cb394 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 2 Apr 2022 20:47:29 +0800 Subject: [PATCH 28/30] Remove unnecessary timout --- modules/git/git.go | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/modules/git/git.go b/modules/git/git.go index 563ddba1d336..6a443ea2e5fa 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -144,11 +144,6 @@ func Init(ctx context.Context) error { globalCommandArgs = append(globalCommandArgs, "-c", "uploadpack.allowfilter=true", "-c", "uploadpack.allowAnySHA1InWant=true") } - // Save current git version on init to gitVersion otherwise it would require an RWMutex - if err := LoadGitVersion(); err != nil { - return err - } - // Git requires setting user.name and user.email in order to commit changes - if they're not set just add some defaults for configKey, defaultValue := range map[string]string{"user.name": "Gitea", "user.email": "gitea@fake.local"} { if err := checkAndSetConfig(configKey, defaultValue, false); err != nil { @@ -230,9 +225,8 @@ func checkAndSetConfig(key, defaultValue string, forceToDefault bool) error { if err := NewCommand(DefaultContext, "config", "--get", key). SetDescription("git.Init(get setting)"). Run(&git_cmd.RunOpts{ - Timeout: -1, - Stdout: &stdout, - Stderr: &stderr, + Stdout: &stdout, + Stderr: &stderr, }); err != nil { eerr, ok := err.(*exec.ExitError) if !ok || eerr.ExitCode() != 1 { @@ -250,8 +244,7 @@ func checkAndSetConfig(key, defaultValue string, forceToDefault bool) error { if err := NewCommand(DefaultContext, "config", "--global", key, defaultValue). SetDescription(fmt.Sprintf("git.Init(set %s)", key)). Run(&git_cmd.RunOpts{ - Timeout: -1, - Stderr: &stderr, + Stderr: &stderr, }); err != nil { return fmt.Errorf("failed to set git %s(%s): %s", key, err, stderr.String()) } @@ -268,9 +261,8 @@ func checkAndAddConfig(key, value string) error { if err := NewCommand(DefaultContext, "config", "--get", key). SetDescription("git.Init(get setting)"). Run(&git_cmd.RunOpts{ - Timeout: -1, - Stdout: &stdout, - Stderr: &stderr, + Stdout: &stdout, + Stderr: &stderr, }); err != nil { eerr, ok := err.(*exec.ExitError) if !ok || eerr.ExitCode() != 1 { @@ -281,8 +273,7 @@ func checkAndAddConfig(key, value string) error { if err := NewCommand(DefaultContext, "config", "--global", "--add", key, value). SetDescription(fmt.Sprintf("git.Init(set %s)", key)). Run(&git_cmd.RunOpts{ - Timeout: -1, - Stderr: &stderr, + Stderr: &stderr, }); err != nil { return fmt.Errorf("failed to set git %s(%s): %s", key, err, stderr.String()) } @@ -300,8 +291,7 @@ func checkAndRemoveConfig(key, value string) error { if err := NewCommand(DefaultContext, "config", "--get", key, value). SetDescription("git.Init(get setting)"). Run(&git_cmd.RunOpts{ - Timeout: -1, - Stderr: &stderr, + Stderr: &stderr, }); err != nil { eerr, ok := err.(*exec.ExitError) if !ok || eerr.ExitCode() != 1 { @@ -316,8 +306,7 @@ func checkAndRemoveConfig(key, value string) error { if err := NewCommand(DefaultContext, "config", "--global", "--unset-all", key, value). SetDescription(fmt.Sprintf("git.Init(set %s)", key)). Run(&git_cmd.RunOpts{ - Timeout: -1, - Stderr: &stderr, + Stderr: &stderr, }); err != nil { return fmt.Errorf("failed to set git %s(%s): %s", key, err, stderr.String()) } From 9a6ab2dfe8b6f3bb57c1d2171ba452667e5d19a2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 5 Jun 2022 10:40:42 +0800 Subject: [PATCH 29/30] Use git command instead of os/exec --- modules/git/blame.go | 72 +++++++++++++++------------------------ modules/git/blame_test.go | 50 +++++---------------------- 2 files changed, 37 insertions(+), 85 deletions(-) diff --git a/modules/git/blame.go b/modules/git/blame.go index f7cc2904b6b7..56e0d0a1c99c 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -10,11 +10,7 @@ import ( "fmt" "io" "os" - "os/exec" "regexp" - - "code.gitea.io/gitea/modules/process" - "code.gitea.io/gitea/modules/setting" ) // BlamePart represents block of blame - continuous lines with one sha @@ -25,12 +21,11 @@ type BlamePart struct { // BlameReader returns part of file blame one by one type BlameReader struct { - cmd *exec.Cmd - output io.ReadCloser - reader *bufio.Reader - lastSha *string - cancel context.CancelFunc // Cancels the context that this reader runs in - finished process.FinishedFunc // Tells the process manager we're finished and it can remove the associated process from the process table + cmd *CommandProxy + output io.WriteCloser + reader io.ReadCloser + done chan error + lastSha *string } var shaLineRegex = regexp.MustCompile("^([a-z0-9]{40})") @@ -39,7 +34,7 @@ var shaLineRegex = regexp.MustCompile("^([a-z0-9]{40})") func (r *BlameReader) NextPart() (*BlamePart, error) { var blamePart *BlamePart - reader := r.reader + reader := bufio.NewReader(r.reader) if r.lastSha != nil { blamePart = &BlamePart{*r.lastSha, make([]string, 0)} @@ -101,51 +96,40 @@ func (r *BlameReader) NextPart() (*BlamePart, error) { // Close BlameReader - don't run NextPart after invoking that func (r *BlameReader) Close() error { - defer r.finished() // Only remove the process from the process table when the underlying command is closed - r.cancel() // However, first cancel our own context early - + err := <-r.done + _ = r.reader.Close() _ = r.output.Close() - if err := r.cmd.Wait(); err != nil { - return fmt.Errorf("Wait: %v", err) - } - - return nil + return err } // CreateBlameReader creates reader for given repository, commit and file func CreateBlameReader(ctx context.Context, repoPath, commitID, file string) (*BlameReader, error) { - return createBlameReader(ctx, repoPath, setting.Git.Path, "blame", commitID, "--porcelain", "--", file) -} + cmd := NewCommandContextNoGlobals(ctx, "blame", commitID, "--porcelain", "--", file) + cmd.SetDescription(fmt.Sprintf("GetBlame [repo_path: %s]", repoPath)) -func createBlameReader(ctx context.Context, dir string, command ...string) (*BlameReader, error) { - // Here we use the provided context - this should be tied to the request performing the blame so that it does not hang around. - ctx, cancel, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("GetBlame [repo_path: %s]", dir)) - - cmd := exec.CommandContext(ctx, command[0], command[1:]...) - cmd.Dir = dir - cmd.Stderr = os.Stderr - process.SetSysProcAttribute(cmd) - - stdout, err := cmd.StdoutPipe() + reader, stdout, err := os.Pipe() if err != nil { - defer finished() - return nil, fmt.Errorf("StdoutPipe: %v", err) + return nil, err } - if err = cmd.Start(); err != nil { - defer finished() - _ = stdout.Close() - return nil, fmt.Errorf("Start: %v", err) - } + done := make(chan error, 1) - reader := bufio.NewReader(stdout) + go func(cmd *CommandProxy, dir string, stdout io.WriteCloser, done chan error) { + if err := cmd.Run(&RunOpts{ + Dir: dir, + Stdout: stdout, + Stderr: os.Stderr, + }); err == nil { + stdout.Close() + } + done <- err + }(cmd, repoPath, stdout, done) return &BlameReader{ - cmd: cmd, - output: stdout, - reader: reader, - cancel: cancel, - finished: finished, + cmd: cmd, + output: stdout, + reader: reader, + done: done, }, nil } diff --git a/modules/git/blame_test.go b/modules/git/blame_test.go index 4bee8cd27a96..77ed81880ce8 100644 --- a/modules/git/blame_test.go +++ b/modules/git/blame_test.go @@ -6,7 +6,6 @@ package git import ( "context" - "os" "testing" "github.com/stretchr/testify/assert" @@ -65,7 +64,7 @@ summary Add code of delete user previous be0ba9ea88aff8a658d0495d36accf944b74888d gogs.go filename gogs.go // license that can be found in the LICENSE file. - + e2aa991e10ffd924a828ec149951f2f20eecead2 6 6 2 author Lunny Xiao author-mail @@ -84,61 +83,30 @@ e2aa991e10ffd924a828ec149951f2f20eecead2 7 7 ` func TestReadingBlameOutput(t *testing.T) { - tempFile, err := os.CreateTemp("", ".txt") - if err != nil { - panic(err) - } - - defer tempFile.Close() - - if _, err = tempFile.WriteString(exampleBlame); err != nil { - panic(err) - } ctx, cancel := context.WithCancel(context.Background()) defer cancel() - blameReader, err := createBlameReader(ctx, "", "cat", tempFile.Name()) - if err != nil { - panic(err) - } + blameReader, err := CreateBlameReader(ctx, "./tests/repos/repo5_pulls", "f32b0a9dfd09a60f616f29158f772cedd89942d2", "README.md") + assert.NoError(t, err) defer blameReader.Close() parts := []*BlamePart{ { - "4b92a6c2df28054ad766bc262f308db9f6066596", - []string{ - "// Copyright 2014 The Gogs Authors. All rights reserved.", - }, - }, - { - "ce21ed6c3490cdfad797319cbb1145e2330a8fef", - []string{ - "// Copyright 2016 The Gitea Authors. All rights reserved.", - }, - }, - { - "4b92a6c2df28054ad766bc262f308db9f6066596", + "72866af952e98d02a73003501836074b286a78f6", []string{ - "// Use of this source code is governed by a MIT-style", - "// license that can be found in the LICENSE file.", - "", + "# test_repo", + "Test repository for testing migration from github to gitea", }, }, { - "e2aa991e10ffd924a828ec149951f2f20eecead2", - []string{ - "// Gitea (git with a cup of tea) is a painless self-hosted Git Service.", - "package main // import \"code.gitea.io/gitea\"", - }, + "f32b0a9dfd09a60f616f29158f772cedd89942d2", + []string{}, }, - nil, } for _, part := range parts { actualPart, err := blameReader.NextPart() - if err != nil { - panic(err) - } + assert.NoError(t, err) assert.Equal(t, part, actualPart) } } From 03c104599d65daf13b7fdeb225c97dd478f81f6e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 5 Jun 2022 10:52:55 +0800 Subject: [PATCH 30/30] Fix test --- modules/git/blame_test.go | 71 --------------------------------------- 1 file changed, 71 deletions(-) diff --git a/modules/git/blame_test.go b/modules/git/blame_test.go index 77ed81880ce8..6c8958839e69 100644 --- a/modules/git/blame_test.go +++ b/modules/git/blame_test.go @@ -11,77 +11,6 @@ import ( "github.com/stretchr/testify/assert" ) -const exampleBlame = ` -4b92a6c2df28054ad766bc262f308db9f6066596 1 1 1 -author Unknown -author-mail -author-time 1392833071 -author-tz -0500 -committer Unknown -committer-mail -committer-time 1392833071 -committer-tz -0500 -summary Add code of delete user -previous be0ba9ea88aff8a658d0495d36accf944b74888d gogs.go -filename gogs.go - // Copyright 2014 The Gogs Authors. All rights reserved. -ce21ed6c3490cdfad797319cbb1145e2330a8fef 2 2 1 -author Joubert RedRat -author-mail -author-time 1482322397 -author-tz -0200 -committer Lunny Xiao -committer-mail -committer-time 1482322397 -committer-tz +0800 -summary Remove remaining Gogs reference on locales and cmd (#430) -previous 618407c018cdf668ceedde7454c42fb22ba422d8 main.go -filename main.go - // Copyright 2016 The Gitea Authors. All rights reserved. -4b92a6c2df28054ad766bc262f308db9f6066596 2 3 2 -author Unknown -author-mail -author-time 1392833071 -author-tz -0500 -committer Unknown -committer-mail -committer-time 1392833071 -committer-tz -0500 -summary Add code of delete user -previous be0ba9ea88aff8a658d0495d36accf944b74888d gogs.go -filename gogs.go - // Use of this source code is governed by a MIT-style -4b92a6c2df28054ad766bc262f308db9f6066596 3 4 -author Unknown -author-mail -author-time 1392833071 -author-tz -0500 -committer Unknown -committer-mail -committer-time 1392833071 -committer-tz -0500 -summary Add code of delete user -previous be0ba9ea88aff8a658d0495d36accf944b74888d gogs.go -filename gogs.go - // license that can be found in the LICENSE file. - -e2aa991e10ffd924a828ec149951f2f20eecead2 6 6 2 -author Lunny Xiao -author-mail -author-time 1478872595 -author-tz +0800 -committer Sandro Santilli -committer-mail -committer-time 1478872595 -committer-tz +0100 -summary ask for go get from code.gitea.io/gitea and change gogs to gitea on main file (#146) -previous 5fc370e332171b8658caed771b48585576f11737 main.go -filename main.go - // Gitea (git with a cup of tea) is a painless self-hosted Git Service. -e2aa991e10ffd924a828ec149951f2f20eecead2 7 7 - package main // import "code.gitea.io/gitea" -` - func TestReadingBlameOutput(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel()