Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexport var git.GlobalCommandArgs #18376

Merged
merged 7 commits into from
Jan 25, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion integrations/git_clone_wiki_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
24 changes: 4 additions & 20 deletions integrations/git_helper_for_declarative_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"path"
"path/filepath"
"strconv"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -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)()
Expand Down Expand Up @@ -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)
Expand All @@ -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"))
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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)
}
}
4 changes: 2 additions & 2 deletions integrations/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down
26 changes: 21 additions & 5 deletions modules/git/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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...),
Expand Down Expand Up @@ -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]
}
6 changes: 3 additions & 3 deletions modules/git/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
}

Expand Down
10 changes: 5 additions & 5 deletions modules/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion modules/git/lfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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=")
})
}
Expand Down
4 changes: 2 additions & 2 deletions modules/git/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down