From 80adbebbc8e0814d0fa0ac660d5c9452e458067b Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 25 Jan 2022 19:15:58 +0100 Subject: [PATCH] Unexport git.GlobalCommandArgs (#18376) Unexport the git.GlobalCommandArgs variable. --- integrations/git_clone_wiki_test.go | 2 +- .../git_helper_for_declarative_test.go | 24 +++-------------- integrations/git_test.go | 4 +-- modules/git/command.go | 26 +++++++++++++++---- modules/git/commit.go | 6 ++--- modules/git/git.go | 10 +++---- modules/git/lfs.go | 2 +- modules/git/repo.go | 4 +-- 8 files changed, 39 insertions(+), 39 deletions(-) diff --git a/integrations/git_clone_wiki_test.go b/integrations/git_clone_wiki_test.go index 16cee254b..a73174f6a 100644 --- a/integrations/git_clone_wiki_test.go +++ b/integrations/git_clone_wiki_test.go @@ -41,7 +41,7 @@ func TestRepoCloneWiki(t *testing.T) { u, _ = url.Parse(r) u.User = url.UserPassword("user2", userPassword) t.Run("Clone", func(t *testing.T) { - assert.NoError(t, git.CloneWithArgs(context.Background(), u.String(), dstPath, allowLFSFilters(), git.CloneRepoOptions{})) + assert.NoError(t, git.CloneWithArgs(context.Background(), u.String(), dstPath, git.AllowLFSFiltersArgs(), git.CloneRepoOptions{})) assertFileEqual(t, filepath.Join(dstPath, "Home.md"), []byte("# Home page\n\nThis is the home page!\n")) assertFileExist(t, filepath.Join(dstPath, "Page-With-Image.md")) assertFileExist(t, filepath.Join(dstPath, "Page-With-Spaced-Name.md")) diff --git a/integrations/git_helper_for_declarative_test.go b/integrations/git_helper_for_declarative_test.go index de96f633c..e1b6b779e 100644 --- a/integrations/git_helper_for_declarative_test.go +++ b/integrations/git_helper_for_declarative_test.go @@ -14,7 +14,6 @@ import ( "path" "path/filepath" "strconv" - "strings" "testing" "time" @@ -60,21 +59,6 @@ func createSSHUrl(gitPath string, u *url.URL) *url.URL { return &u2 } -func allowLFSFilters() []string { - // Now here we should explicitly allow lfs filters to run - filteredLFSGlobalArgs := make([]string, len(git.GlobalCommandArgs)) - j := 0 - for _, arg := range git.GlobalCommandArgs { - if strings.Contains(arg, "lfs") { - j-- - } else { - filteredLFSGlobalArgs[j] = arg - j++ - } - } - return filteredLFSGlobalArgs[:j] -} - func onGiteaRunTB(t testing.TB, callback func(testing.TB, *url.URL), prepare ...bool) { if len(prepare) == 0 || prepare[0] { defer prepareTestEnv(t, 1)() @@ -115,7 +99,7 @@ func onGiteaRun(t *testing.T, callback func(*testing.T, *url.URL), prepare ...bo func doGitClone(dstLocalPath string, u *url.URL) func(*testing.T) { return func(t *testing.T) { - assert.NoError(t, git.CloneWithArgs(context.Background(), u.String(), dstLocalPath, allowLFSFilters(), git.CloneRepoOptions{})) + assert.NoError(t, git.CloneWithArgs(context.Background(), u.String(), dstLocalPath, git.AllowLFSFiltersArgs(), git.CloneRepoOptions{})) exist, err := util.IsExist(filepath.Join(dstLocalPath, "README.md")) assert.NoError(t, err) assert.True(t, exist) @@ -124,7 +108,7 @@ func doGitClone(dstLocalPath string, u *url.URL) func(*testing.T) { func doPartialGitClone(dstLocalPath string, u *url.URL) func(*testing.T) { return func(t *testing.T) { - assert.NoError(t, git.CloneWithArgs(context.Background(), u.String(), dstLocalPath, allowLFSFilters(), git.CloneRepoOptions{ + assert.NoError(t, git.CloneWithArgs(context.Background(), u.String(), dstLocalPath, git.AllowLFSFiltersArgs(), git.CloneRepoOptions{ Filter: "blob:none", })) exist, err := util.IsExist(filepath.Join(dstLocalPath, "README.md")) @@ -197,7 +181,7 @@ func doGitCreateBranch(dstPath, branch string) func(*testing.T) { func doGitCheckoutBranch(dstPath string, args ...string) func(*testing.T) { return func(t *testing.T) { - _, err := git.NewCommandNoGlobals(append(append(allowLFSFilters(), "checkout"), args...)...).RunInDir(dstPath) + _, err := git.NewCommandNoGlobals(append(append(git.AllowLFSFiltersArgs(), "checkout"), args...)...).RunInDir(dstPath) assert.NoError(t, err) } } @@ -211,7 +195,7 @@ func doGitMerge(dstPath string, args ...string) func(*testing.T) { func doGitPull(dstPath string, args ...string) func(*testing.T) { return func(t *testing.T) { - _, err := git.NewCommandNoGlobals(append(append(allowLFSFilters(), "pull"), args...)...).RunInDir(dstPath) + _, err := git.NewCommandNoGlobals(append(append(git.AllowLFSFiltersArgs(), "pull"), args...)...).RunInDir(dstPath) assert.NoError(t, err) } } diff --git a/integrations/git_test.go b/integrations/git_test.go index 93ff9d254..e5a85eb75 100644 --- a/integrations/git_test.go +++ b/integrations/git_test.go @@ -167,7 +167,7 @@ func lfsCommitAndPushTest(t *testing.T, dstPath string) (littleLFS, bigLFS strin err = git.AddChanges(dstPath, false, ".gitattributes") assert.NoError(t, err) - err = git.CommitChangesWithArgs(dstPath, allowLFSFilters(), git.CommitChangesOptions{ + err = git.CommitChangesWithArgs(dstPath, git.AllowLFSFiltersArgs(), git.CommitChangesOptions{ Committer: &git.Signature{ Email: "user2@example.com", Name: "User Two", @@ -346,7 +346,7 @@ func generateCommitWithNewData(size int, repoPath, email, fullName, prefix strin // Commit // Now here we should explicitly allow lfs filters to run - globalArgs := allowLFSFilters() + globalArgs := git.AllowLFSFiltersArgs() err = git.AddChangesWithArgs(repoPath, globalArgs, false, filepath.Base(tmpFile.Name())) if err != nil { return "", err diff --git a/modules/git/command.go b/modules/git/command.go index 3cf85c2d8..e8afaa0e4 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -20,8 +20,8 @@ import ( ) var ( - // GlobalCommandArgs global command args for external package setting - GlobalCommandArgs []string + // globalCommandArgs global command args for external package setting + globalCommandArgs []string // defaultCommandExecutionTimeout default command execution timeout duration defaultCommandExecutionTimeout = 360 * time.Second @@ -52,9 +52,9 @@ func NewCommand(args ...string) *Command { // NewCommandContext creates and returns a new Git Command based on given command and arguments. func NewCommandContext(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) + // 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...), @@ -278,3 +278,19 @@ func (c *Command) RunTimeout(timeout time.Duration) (string, error) { func (c *Command) Run() (string, error) { return c.RunTimeout(-1) } + +// 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/commit.go b/modules/git/commit.go index 0ba53897f..37b2e71cc 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -84,7 +84,7 @@ func (c *Commit) GetCommitByPath(relpath string) (*Commit, error) { // AddChanges marks local changes to be ready for commit. func AddChanges(repoPath string, all bool, files ...string) error { - return AddChangesWithArgs(repoPath, GlobalCommandArgs, all, files...) + return AddChangesWithArgs(repoPath, globalCommandArgs, all, files...) } // AddChangesWithArgs marks local changes to be ready for commit. @@ -108,8 +108,8 @@ type CommitChangesOptions struct { // CommitChanges commits local changes with given committer, author and message. // If author is nil, it will be the same as committer. func CommitChanges(repoPath string, opts CommitChangesOptions) error { - cargs := make([]string, len(GlobalCommandArgs)) - copy(cargs, GlobalCommandArgs) + cargs := make([]string, len(globalCommandArgs)) + copy(cargs, globalCommandArgs) return CommitChangesWithArgs(repoPath, cargs, opts) } diff --git a/modules/git/git.go b/modules/git/git.go index 294d33f91..217e6955c 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -134,21 +134,21 @@ func Init(ctx context.Context) error { } // force cleanup args - GlobalCommandArgs = []string{} + globalCommandArgs = []string{} if CheckGitVersionAtLeast("2.9") == nil { // Explicitly disable credential helper, otherwise Git credentials might leak - GlobalCommandArgs = append(GlobalCommandArgs, "-c", "credential.helper=") + globalCommandArgs = append(globalCommandArgs, "-c", "credential.helper=") } // Since git wire protocol has been released from git v2.18 if setting.Git.EnableAutoGitWireProtocol && CheckGitVersionAtLeast("2.18") == nil { - GlobalCommandArgs = append(GlobalCommandArgs, "-c", "protocol.version=2") + globalCommandArgs = append(globalCommandArgs, "-c", "protocol.version=2") } // By default partial clones are disabled, enable them from git v2.22 if !setting.Git.DisablePartialClone && CheckGitVersionAtLeast("2.22") == nil { - GlobalCommandArgs = append(GlobalCommandArgs, "-c", "uploadpack.allowfilter=true") + globalCommandArgs = append(globalCommandArgs, "-c", "uploadpack.allowfilter=true") } // Save current git version on init to gitVersion otherwise it would require an RWMutex @@ -213,7 +213,7 @@ func Init(ctx context.Context) error { if err := checkAndSetConfig("core.protectntfs", "false", true); err != nil { return err } - GlobalCommandArgs = append(GlobalCommandArgs, "-c", "core.protectntfs=false") + globalCommandArgs = append(globalCommandArgs, "-c", "core.protectntfs=false") } return nil } diff --git a/modules/git/lfs.go b/modules/git/lfs.go index 3a809d393..cdd9d15b2 100644 --- a/modules/git/lfs.go +++ b/modules/git/lfs.go @@ -29,7 +29,7 @@ func CheckLFSVersion() { logger.Error("LFS server support needs at least Git v2.1.2") } else { once.Do(func() { - GlobalCommandArgs = append(GlobalCommandArgs, "-c", "filter.lfs.required=", + globalCommandArgs = append(globalCommandArgs, "-c", "filter.lfs.required=", "-c", "filter.lfs.smudge=", "-c", "filter.lfs.clean=") }) } diff --git a/modules/git/repo.go b/modules/git/repo.go index 563640511..663e13dc1 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -106,8 +106,8 @@ type CloneRepoOptions struct { // Clone clones original repository to target path. func Clone(ctx context.Context, from, to string, opts CloneRepoOptions) error { - cargs := make([]string, len(GlobalCommandArgs)) - copy(cargs, GlobalCommandArgs) + cargs := make([]string, len(globalCommandArgs)) + copy(cargs, globalCommandArgs) return CloneWithArgs(ctx, from, to, cargs, opts) }