From b65e5013261bf1a87e6590695d01bd6621141dc1 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 6 Aug 2019 22:27:37 +0100 Subject: [PATCH 01/11] Change tests to make it possible to run TestGit with 1.7.2 --- .../git_helper_for_declarative_test.go | 6 +++ integrations/git_test.go | 41 ++++++++++++------- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/integrations/git_helper_for_declarative_test.go b/integrations/git_helper_for_declarative_test.go index 9190d4bb4e53..a8bf5b89c029 100644 --- a/integrations/git_helper_for_declarative_test.go +++ b/integrations/git_helper_for_declarative_test.go @@ -12,6 +12,7 @@ import ( "net/http" "net/url" "os" + "path" "path/filepath" "testing" "time" @@ -36,7 +37,12 @@ func withKeyFile(t *testing.T, keyname string, callback func(string)) { err = ssh.GenKeyPair(keyFile) assert.NoError(t, err) + err = ioutil.WriteFile(path.Join(tmpDir, "ssh"), []byte("#!/bin/bash\n"+ + "ssh -o \"UserKnownHostsFile=/dev/null\" -o \"StrictHostKeyChecking=no\" -o \"IdentitiesOnly=yes\" -i \""+keyFile+"\" \"$@\""), 0700) + assert.NoError(t, err) + //Setup ssh wrapper + os.Setenv("GIT_SSH", path.Join(tmpDir, "ssh")) os.Setenv("GIT_SSH_COMMAND", "ssh -o \"UserKnownHostsFile=/dev/null\" -o \"StrictHostKeyChecking=no\" -o \"IdentitiesOnly=yes\" -i \""+keyFile+"\"") os.Setenv("GIT_SSH_VARIANT", "ssh") diff --git a/integrations/git_test.go b/integrations/git_test.go index 8578fb86d5d7..751ea51c269b 100644 --- a/integrations/git_test.go +++ b/integrations/git_test.go @@ -19,6 +19,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "github.com/stretchr/testify/assert" @@ -135,6 +136,10 @@ func standardCommitAndPushTest(t *testing.T, dstPath string) (little, big string func lfsCommitAndPushTest(t *testing.T, dstPath string) (littleLFS, bigLFS string) { t.Run("LFS", func(t *testing.T) { PrintCurrentTest(t) + if !setting.LFS.StartServer { + t.Skip() + return + } prefix := "lfs-data-file-" _, err := git.NewCommand("lfs").AddArguments("install").RunInDir(dstPath) assert.NoError(t, err) @@ -185,20 +190,24 @@ func rawTest(t *testing.T, ctx *APITestContext, little, big, littleLFS, bigLFS s resp := session.MakeRequest(t, req, http.StatusOK) assert.Equal(t, littleSize, resp.Body.Len()) - req = NewRequest(t, "GET", path.Join("/", username, reponame, "/raw/branch/master/", littleLFS)) - resp = session.MakeRequest(t, req, http.StatusOK) - assert.NotEqual(t, littleSize, resp.Body.Len()) - assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier) + if setting.LFS.StartServer { + req = NewRequest(t, "GET", path.Join("/", username, reponame, "/raw/branch/master/", littleLFS)) + resp = session.MakeRequest(t, req, http.StatusOK) + assert.NotEqual(t, littleSize, resp.Body.Len()) + assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier) + } if !testing.Short() { req = NewRequest(t, "GET", path.Join("/", username, reponame, "/raw/branch/master/", big)) resp = session.MakeRequest(t, req, http.StatusOK) assert.Equal(t, bigSize, resp.Body.Len()) - req = NewRequest(t, "GET", path.Join("/", username, reponame, "/raw/branch/master/", bigLFS)) - resp = session.MakeRequest(t, req, http.StatusOK) - assert.NotEqual(t, bigSize, resp.Body.Len()) - assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier) + if setting.LFS.StartServer { + req = NewRequest(t, "GET", path.Join("/", username, reponame, "/raw/branch/master/", bigLFS)) + resp = session.MakeRequest(t, req, http.StatusOK) + assert.NotEqual(t, bigSize, resp.Body.Len()) + assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier) + } } }) } @@ -217,18 +226,22 @@ func mediaTest(t *testing.T, ctx *APITestContext, little, big, littleLFS, bigLFS resp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) assert.Equal(t, littleSize, resp.Length) - req = NewRequest(t, "GET", path.Join("/", username, reponame, "/media/branch/master/", littleLFS)) - resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) - assert.Equal(t, littleSize, resp.Length) + if setting.LFS.StartServer { + req = NewRequest(t, "GET", path.Join("/", username, reponame, "/media/branch/master/", littleLFS)) + resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) + assert.Equal(t, littleSize, resp.Length) + } if !testing.Short() { req = NewRequest(t, "GET", path.Join("/", username, reponame, "/media/branch/master/", big)) resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) assert.Equal(t, bigSize, resp.Length) - req = NewRequest(t, "GET", path.Join("/", username, reponame, "/media/branch/master/", bigLFS)) - resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) - assert.Equal(t, bigSize, resp.Length) + if setting.LFS.StartServer { + req = NewRequest(t, "GET", path.Join("/", username, reponame, "/media/branch/master/", bigLFS)) + resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) + assert.Equal(t, bigSize, resp.Length) + } } }) } From 708f032dcc7fca5f6ba85c2f4b916156e8e18f90 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 6 Aug 2019 22:27:56 +0100 Subject: [PATCH 02/11] Make merge run on 1.7.2 --- modules/git/repo_branch.go | 2 +- modules/pull/merge.go | 27 ++++++++++++++++++++------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/modules/git/repo_branch.go b/modules/git/repo_branch.go index 9209f4a764f4..3e1261d294cf 100644 --- a/modules/git/repo_branch.go +++ b/modules/git/repo_branch.go @@ -165,7 +165,7 @@ func (repo *Repository) AddRemote(name, url string, fetch bool) error { // RemoveRemote removes a remote from repository. func (repo *Repository) RemoveRemote(name string) error { - _, err := NewCommand("remote", "remove", name).RunInDir(repo.Path) + _, err := NewCommand("remote", "rm", name).RunInDir(repo.Path) return err } diff --git a/modules/pull/merge.go b/modules/pull/merge.go index 0e9e3b8b1457..947ee49dfdba 100644 --- a/modules/pull/merge.go +++ b/modules/pull/merge.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" + "github.com/mcuadros/go-version" ) // Merge merges pull request to base repository. @@ -100,12 +101,12 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("git remote add [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } + trackingBranch := path.Join(remoteRepoName, pr.HeadBranch) // Fetch head branch - if err := git.NewCommand("fetch", remoteRepoName, fmt.Sprintf("%s:refs/remotes/%s/%s", pr.HeadBranch, remoteRepoName, pr.HeadBranch)).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("fetch", remoteRepoName, pr.HeadBranch+":"+trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } - trackingBranch := path.Join(remoteRepoName, pr.HeadBranch) stagingBranch := fmt.Sprintf("%s_%s", remoteRepoName, pr.HeadBranch) // Enable sparse-checkout @@ -123,21 +124,33 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("Writing sparse-checkout file to %s: %v", sparseCheckoutListPath, err) } + gitConfigCommand := func() *git.Command { + binVersion, err := git.BinVersion() + if err != nil { + log.Fatal("Error retrieving git version: %v", err) + } + + if version.Compare(binVersion, "1.9.2", ">=") { + return git.NewCommand("config", "--local") + } + return git.NewCommand("config") + } + // Switch off LFS process (set required, clean and smudge here also) - if err := git.NewCommand("config", "--local", "filter.lfs.process", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := gitConfigCommand().AddArguments("filter.lfs.process", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git config [filter.lfs.process -> <> ]: %v", errbuf.String()) } - if err := git.NewCommand("config", "--local", "filter.lfs.required", "false").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := gitConfigCommand().AddArguments("filter.lfs.required", "false").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git config [filter.lfs.required -> ]: %v", errbuf.String()) } - if err := git.NewCommand("config", "--local", "filter.lfs.clean", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := gitConfigCommand().AddArguments("filter.lfs.clean", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git config [filter.lfs.clean -> <> ]: %v", errbuf.String()) } - if err := git.NewCommand("config", "--local", "filter.lfs.smudge", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := gitConfigCommand().AddArguments("filter.lfs.smudge", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git config [filter.lfs.smudge -> <> ]: %v", errbuf.String()) } - if err := git.NewCommand("config", "--local", "core.sparseCheckout", "true").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := gitConfigCommand().AddArguments("core.sparseCheckout", "true").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git config [core.sparsecheckout -> true]: %v", errbuf.String()) } From 41685c99fb6f285e9ea62b4d4244de4026a47b66 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 7 Aug 2019 19:30:44 +0100 Subject: [PATCH 03/11] Fix tracking and staging branch name problem --- modules/pull/merge.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/modules/pull/merge.go b/modules/pull/merge.go index 947ee49dfdba..7c27d7e8aa2c 100644 --- a/modules/pull/merge.go +++ b/modules/pull/merge.go @@ -11,7 +11,6 @@ import ( "fmt" "io/ioutil" "os" - "path" "path/filepath" "strings" @@ -101,13 +100,13 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("git remote add [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } - trackingBranch := path.Join(remoteRepoName, pr.HeadBranch) + trackingBranch := "tracking" // Fetch head branch if err := git.NewCommand("fetch", remoteRepoName, pr.HeadBranch+":"+trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } - stagingBranch := fmt.Sprintf("%s_%s", remoteRepoName, pr.HeadBranch) + stagingBranch := "staging" // Enable sparse-checkout sparseCheckoutList, err := getDiffTree(tmpBasePath, pr.BaseBranch, trackingBranch) From 27a85a01fe2432aa7537f725e031fffce96a6346 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 7 Aug 2019 20:11:24 +0100 Subject: [PATCH 04/11] Ensure that git 1.7.2 works on tests --- integrations/git_test.go | 3 +++ integrations/lfs_getobject_test.go | 5 +++++ models/repo.go | 2 +- modules/git/repo_tree.go | 21 ++++++++++++++++----- modules/process/manager.go | 14 ++++++++++++++ modules/pull/merge.go | 14 +++++++++----- modules/repofiles/temp_repo.go | 25 +++++++++++++++++++++---- modules/repofiles/update.go | 25 +++++++++++++------------ modules/repofiles/upload.go | 11 +++++++---- 9 files changed, 89 insertions(+), 31 deletions(-) diff --git a/integrations/git_test.go b/integrations/git_test.go index 751ea51c269b..4e014fe2d728 100644 --- a/integrations/git_test.go +++ b/integrations/git_test.go @@ -136,6 +136,7 @@ func standardCommitAndPushTest(t *testing.T, dstPath string) (little, big string func lfsCommitAndPushTest(t *testing.T, dstPath string) (littleLFS, bigLFS string) { t.Run("LFS", func(t *testing.T) { PrintCurrentTest(t) + setting.CheckLFSVersion() if !setting.LFS.StartServer { t.Skip() return @@ -190,6 +191,7 @@ func rawTest(t *testing.T, ctx *APITestContext, little, big, littleLFS, bigLFS s resp := session.MakeRequest(t, req, http.StatusOK) assert.Equal(t, littleSize, resp.Body.Len()) + setting.CheckLFSVersion() if setting.LFS.StartServer { req = NewRequest(t, "GET", path.Join("/", username, reponame, "/raw/branch/master/", littleLFS)) resp = session.MakeRequest(t, req, http.StatusOK) @@ -226,6 +228,7 @@ func mediaTest(t *testing.T, ctx *APITestContext, little, big, littleLFS, bigLFS resp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) assert.Equal(t, littleSize, resp.Length) + setting.CheckLFSVersion() if setting.LFS.StartServer { req = NewRequest(t, "GET", path.Join("/", username, reponame, "/media/branch/master/", littleLFS)) resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) diff --git a/integrations/lfs_getobject_test.go b/integrations/lfs_getobject_test.go index 567cf13b45b6..da3ddcec706d 100644 --- a/integrations/lfs_getobject_test.go +++ b/integrations/lfs_getobject_test.go @@ -58,6 +58,11 @@ func storeObjectInRepo(t *testing.T, repositoryID int64, content *[]byte) string func doLfs(t *testing.T, content *[]byte, expectGzip bool) { prepareTestEnv(t) + setting.CheckLFSVersion() + if !setting.LFS.StartServer { + t.Skip() + return + } repo, err := models.GetRepositoryByOwnerAndName("user2", "repo1") assert.NoError(t, err) oid := storeObjectInRepo(t, repo.ID, content) diff --git a/models/repo.go b/models/repo.go index 976e8532325e..36f27fef53a6 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1063,7 +1063,7 @@ func CleanUpMigrateInfo(repo *Repository) (*Repository, error) { } } - _, err := git.NewCommand("remote", "remove", "origin").RunInDir(repoPath) + _, err := git.NewCommand("remote", "rm", "origin").RunInDir(repoPath) if err != nil && !strings.HasPrefix(err.Error(), "exit status 128 - fatal: No such remote ") { return repo, fmt.Errorf("CleanUpMigrateInfo: %v", err) } diff --git a/modules/git/repo_tree.go b/modules/git/repo_tree.go index f7c9dff923ca..f6b393d8d86f 100644 --- a/modules/git/repo_tree.go +++ b/modules/git/repo_tree.go @@ -6,11 +6,13 @@ package git import ( + "bytes" "fmt" "os" "strings" "time" + "github.com/mcuadros/go-version" "gopkg.in/src-d/go-git.v4/plumbing" ) @@ -63,6 +65,11 @@ type CommitTreeOpts struct { // CommitTree creates a commit from a given tree id for the user with provided message func (repo *Repository) CommitTree(sig *Signature, tree *Tree, opts CommitTreeOpts) (SHA1, error) { + binVersion, err := BinVersion() + if err != nil { + return SHA1{}, err + } + commitTimeStr := time.Now().Format(time.RFC3339) // Because this may call hooks we should pass in the environment @@ -80,20 +87,24 @@ func (repo *Repository) CommitTree(sig *Signature, tree *Tree, opts CommitTreeOp cmd.AddArguments("-p", parent) } - cmd.AddArguments("-m", opts.Message) + messageBytes := new(bytes.Buffer) + _, _ = messageBytes.WriteString(opts.Message) + _, _ = messageBytes.WriteString("\n") if opts.KeyID != "" { cmd.AddArguments(fmt.Sprintf("-S%s", opts.KeyID)) } - if opts.NoGPGSign { + if version.Compare(binVersion, "2.0.0", ">=") && opts.NoGPGSign { cmd.AddArguments("--no-gpg-sign") } - res, err := cmd.RunInDirWithEnv(repo.Path, env) + stdout := new(bytes.Buffer) + stderr := new(bytes.Buffer) + err = cmd.RunInDirTimeoutEnvFullPipeline(env, -1, repo.Path, stdout, stderr, messageBytes) if err != nil { - return SHA1{}, err + return SHA1{}, concatenateError(err, stderr.String()) } - return NewIDFromString(strings.TrimSpace(res)) + return NewIDFromString(strings.TrimSpace(stdout.String())) } diff --git a/modules/process/manager.go b/modules/process/manager.go index 9ac3af86f106..3e77c0a6a90f 100644 --- a/modules/process/manager.go +++ b/modules/process/manager.go @@ -1,4 +1,5 @@ // Copyright 2014 The Gogs Authors. All rights reserved. +// Copyright 2019 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. @@ -9,6 +10,7 @@ import ( "context" "errors" "fmt" + "io" "os/exec" "sync" "time" @@ -93,6 +95,14 @@ func (pm *Manager) ExecDir(timeout time.Duration, dir, desc, cmdName string, arg // Returns its complete stdout and stderr // outputs and an error, if any (including timeout) func (pm *Manager) ExecDirEnv(timeout time.Duration, dir, desc string, env []string, cmdName string, args ...string) (string, string, error) { + return pm.ExecDirEnvStdIn(timeout, dir, desc, env, nil, cmdName, args...) +} + +// ExecDirEnvStdIn runs a command in given path and environment variables with provided stdIN, and waits for its completion +// up to the given timeout (or DefaultTimeout if -1 is given). +// Returns its complete stdout and stderr +// outputs and an error, if any (including timeout) +func (pm *Manager) ExecDirEnvStdIn(timeout time.Duration, dir, desc string, env []string, stdIn io.Reader, cmdName string, args ...string) (string, string, error) { if timeout == -1 { timeout = 60 * time.Second } @@ -108,6 +118,10 @@ func (pm *Manager) ExecDirEnv(timeout time.Duration, dir, desc string, env []str cmd.Env = env cmd.Stdout = stdOut cmd.Stderr = stdErr + if stdIn != nil { + cmd.Stdin = stdIn + } + if err := cmd.Start(); err != nil { return "", "", err } diff --git a/modules/pull/merge.go b/modules/pull/merge.go index 7c27d7e8aa2c..fb87acd7009a 100644 --- a/modules/pull/merge.go +++ b/modules/pull/merge.go @@ -123,17 +123,21 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("Writing sparse-checkout file to %s: %v", sparseCheckoutListPath, err) } - gitConfigCommand := func() *git.Command { + gitConfigCommand := func() func() *git.Command { binVersion, err := git.BinVersion() if err != nil { log.Fatal("Error retrieving git version: %v", err) } - if version.Compare(binVersion, "1.9.2", ">=") { - return git.NewCommand("config", "--local") + if version.Compare(binVersion, "1.8.0", ">=") { + return func() *git.Command { + return git.NewCommand("config", "--local") + } } - return git.NewCommand("config") - } + return func() *git.Command { + return git.NewCommand("config") + } + }() // Switch off LFS process (set required, clean and smudge here also) if err := gitConfigCommand().AddArguments("filter.lfs.process", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { diff --git a/modules/repofiles/temp_repo.go b/modules/repofiles/temp_repo.go index d640ba80b02b..04f6def78b12 100644 --- a/modules/repofiles/temp_repo.go +++ b/modules/repofiles/temp_repo.go @@ -20,6 +20,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/setting" + "github.com/mcuadros/go-version" ) // TemporaryUploadRepository is a type to wrap our upload repositories as a shallow clone @@ -263,11 +264,15 @@ func (t *TemporaryUploadRepository) CommitTree(author, committer *models.User, t "GIT_COMMITTER_EMAIL="+committerSig.Email, "GIT_COMMITTER_DATE="+commitTimeStr, ) - commitHash, stderr, err := process.GetManager().ExecDirEnv(5*time.Minute, + messageBytes := new(bytes.Buffer) + _, _ = messageBytes.WriteString(message) + _, _ = messageBytes.WriteString("\n") + commitHash, stderr, err := process.GetManager().ExecDirEnvStdIn(5*time.Minute, t.basePath, fmt.Sprintf("commitTree (git commit-tree): %s", t.basePath), env, - git.GitExecutable, "commit-tree", treeHash, "-p", "HEAD", "-m", message) + messageBytes, + git.GitExecutable, "commit-tree", treeHash, "-p", "HEAD") if err != nil { return "", fmt.Errorf("git commit-tree: %s", stderr) } @@ -327,6 +332,11 @@ func (t *TemporaryUploadRepository) DiffIndex() (diff *models.Diff, err error) { // CheckAttribute checks the given attribute of the provided files func (t *TemporaryUploadRepository) CheckAttribute(attribute string, args ...string) (map[string]map[string]string, error) { + binVersion, err := git.BinVersion() + if err != nil { + log.Fatal("Error retrieving git version: %v", err) + } + stdOut := new(bytes.Buffer) stdErr := new(bytes.Buffer) @@ -334,7 +344,14 @@ func (t *TemporaryUploadRepository) CheckAttribute(attribute string, args ...str ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() - cmdArgs := []string{"check-attr", "-z", attribute, "--cached", "--"} + cmdArgs := []string{"check-attr", "-z", attribute} + + // git check-attr --cached first appears in git 1.8.0 + if version.Compare(binVersion, "1.8.0", ">=") { + cmdArgs = append(cmdArgs, "--cached") + } + cmdArgs = append(cmdArgs, "--") + for _, arg := range args { if arg != "" { cmdArgs = append(cmdArgs, arg) @@ -352,7 +369,7 @@ func (t *TemporaryUploadRepository) CheckAttribute(attribute string, args ...str } pid := process.GetManager().Add(desc, cmd) - err := cmd.Wait() + err = cmd.Wait() process.GetManager().Remove(pid) if err != nil { diff --git a/modules/repofiles/update.go b/modules/repofiles/update.go index 47fe89cf6309..4a8aeec638a0 100644 --- a/modules/repofiles/update.go +++ b/modules/repofiles/update.go @@ -311,12 +311,6 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up } } - // Check there is no way this can return multiple infos - filename2attribute2info, err := t.CheckAttribute("filter", treePath) - if err != nil { - return nil, err - } - content := opts.Content if bom { content = string(base.UTF8BOM) + content @@ -339,16 +333,23 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up opts.Content = content var lfsMetaObject *models.LFSMetaObject - if setting.LFS.StartServer && filename2attribute2info[treePath] != nil && filename2attribute2info[treePath]["filter"] == "lfs" { - // OK so we are supposed to LFS this data! - oid, err := models.GenerateLFSOid(strings.NewReader(opts.Content)) + if setting.LFS.StartServer { + // Check there is no way this can return multiple infos + filename2attribute2info, err := t.CheckAttribute("filter", treePath) if err != nil { return nil, err } - lfsMetaObject = &models.LFSMetaObject{Oid: oid, Size: int64(len(opts.Content)), RepositoryID: repo.ID} - content = lfsMetaObject.Pointer() - } + if filename2attribute2info[treePath] != nil && filename2attribute2info[treePath]["filter"] == "lfs" { + // OK so we are supposed to LFS this data! + oid, err := models.GenerateLFSOid(strings.NewReader(opts.Content)) + if err != nil { + return nil, err + } + lfsMetaObject = &models.LFSMetaObject{Oid: oid, Size: int64(len(opts.Content)), RepositoryID: repo.ID} + content = lfsMetaObject.Pointer() + } + } // Add the object to the database objectHash, err := t.HashObject(strings.NewReader(content)) if err != nil { diff --git a/modules/repofiles/upload.go b/modules/repofiles/upload.go index f2ffec7ebcdd..202e66b89a1c 100644 --- a/modules/repofiles/upload.go +++ b/modules/repofiles/upload.go @@ -74,9 +74,12 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep infos[i] = uploadInfo{upload: upload} } - filename2attribute2info, err := t.CheckAttribute("filter", names...) - if err != nil { - return err + var filename2attribute2info map[string]map[string]string + if setting.LFS.StartServer { + filename2attribute2info, err = t.CheckAttribute("filter", names...) + if err != nil { + return err + } } // Copy uploaded files into repository. @@ -88,7 +91,7 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep defer file.Close() var objectHash string - if filename2attribute2info[uploadInfo.upload.Name] != nil && filename2attribute2info[uploadInfo.upload.Name]["filter"] == "lfs" { + if setting.LFS.StartServer && filename2attribute2info[uploadInfo.upload.Name] != nil && filename2attribute2info[uploadInfo.upload.Name]["filter"] == "lfs" { // Handle LFS // FIXME: Inefficient! this should probably happen in models.Upload oid, err := models.GenerateLFSOid(file) From 52d2864d092851cf8e5c69b14674816e9f03d19e Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 8 Aug 2019 19:17:29 +0100 Subject: [PATCH 05/11] ensure that there is no chance for conflicts --- modules/pull/merge.go | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/modules/pull/merge.go b/modules/pull/merge.go index fb87acd7009a..b3bfa229cae1 100644 --- a/modules/pull/merge.go +++ b/modules/pull/merge.go @@ -66,20 +66,17 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor headRepoPath := models.RepoPath(pr.HeadUserName, pr.HeadRepo.Name) - if err := git.Clone(baseGitRepo.Path, tmpBasePath, git.CloneRepoOptions{ - Shared: true, - NoCheckout: true, - Branch: pr.BaseBranch, - }); err != nil { - return fmt.Errorf("git clone: %v", err) + if err := git.InitRepository(tmpBasePath, false); err != nil { + return fmt.Errorf("git init: %v", err) } remoteRepoName := "head_repo" + baseBranch := "base" // Add head repo remote. addCacheRepo := func(staging, cache string) error { p := filepath.Join(staging, ".git", "objects", "info", "alternates") - f, err := os.OpenFile(p, os.O_APPEND|os.O_WRONLY, 0600) + f, err := os.OpenFile(p, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600) if err != nil { return err } @@ -91,11 +88,31 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return nil } - if err := addCacheRepo(tmpBasePath, headRepoPath); err != nil { + if err := addCacheRepo(tmpBasePath, baseGitRepo.Path); err != nil { return fmt.Errorf("addCacheRepo [%s -> %s]: %v", headRepoPath, tmpBasePath, err) } var errbuf strings.Builder + if err := git.NewCommand("remote", "add", "-t", pr.BaseBranch, "-m", pr.BaseBranch, "origin", baseGitRepo.Path).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git remote add [%s -> %s]: %s", baseGitRepo.Path, tmpBasePath, errbuf.String()) + } + + if err := git.NewCommand("fetch", "origin", pr.BaseBranch+":"+baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + } + + if err := git.NewCommand("fetch", "origin", pr.BaseBranch+":original_"+baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + } + + if err := git.NewCommand("symbolic-ref", "HEAD", git.BranchPrefix+baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git symbolic-ref HEAD base [%s]: %s", tmpBasePath, errbuf.String()) + } + + if err := addCacheRepo(tmpBasePath, headRepoPath); err != nil { + return fmt.Errorf("addCacheRepo [%s -> %s]: %v", headRepoPath, tmpBasePath, err) + } + if err := git.NewCommand("remote", "add", remoteRepoName, headRepoPath).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git remote add [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } @@ -109,7 +126,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor stagingBranch := "staging" // Enable sparse-checkout - sparseCheckoutList, err := getDiffTree(tmpBasePath, pr.BaseBranch, trackingBranch) + sparseCheckoutList, err := getDiffTree(tmpBasePath, baseBranch, trackingBranch) if err != nil { return fmt.Errorf("getDiffTree: %v", err) } @@ -232,7 +249,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor if err != nil { return fmt.Errorf("Failed to get full commit id for HEAD: %v", err) } - mergeBaseSHA, err := git.GetFullCommitID(tmpBasePath, "origin/"+pr.BaseBranch) + mergeBaseSHA, err := git.GetFullCommitID(tmpBasePath, "original_"+baseBranch) if err != nil { return fmt.Errorf("Failed to get full commit id for origin/%s: %v", pr.BaseBranch, err) } @@ -265,7 +282,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor ) // Push back to upstream. - if err := git.NewCommand("push", "origin", pr.BaseBranch).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("push", "origin", baseBranch+":"+pr.BaseBranch).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git push: %s", errbuf.String()) } From b4e853ccf092771c13217730590332a78ffac9d9 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 8 Aug 2019 20:24:26 +0100 Subject: [PATCH 06/11] Fix-up missing merge issues --- modules/pull/merge.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/modules/pull/merge.go b/modules/pull/merge.go index b3bfa229cae1..e9c77ff9d931 100644 --- a/modules/pull/merge.go +++ b/modules/pull/merge.go @@ -97,11 +97,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("git remote add [%s -> %s]: %s", baseGitRepo.Path, tmpBasePath, errbuf.String()) } - if err := git.NewCommand("fetch", "origin", pr.BaseBranch+":"+baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) - } - - if err := git.NewCommand("fetch", "origin", pr.BaseBranch+":original_"+baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("fetch", "origin", "--no-tags", pr.BaseBranch+":"+baseBranch, pr.BaseBranch+":original_"+baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } @@ -119,7 +115,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor trackingBranch := "tracking" // Fetch head branch - if err := git.NewCommand("fetch", remoteRepoName, pr.HeadBranch+":"+trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("fetch", "--no-tags", remoteRepoName, pr.HeadBranch+":"+trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } @@ -196,11 +192,11 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("git checkout: %s", errbuf.String()) } // Rebase before merging - if err := git.NewCommand("rebase", "-q", pr.BaseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("rebase", "-q", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git rebase [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } // Checkout base branch again - if err := git.NewCommand("checkout", pr.BaseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("checkout", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git checkout: %s", errbuf.String()) } // Merge fast forward @@ -213,11 +209,11 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("git checkout: %s", errbuf.String()) } // Rebase before merging - if err := git.NewCommand("rebase", "-q", pr.BaseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("rebase", "-q", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git rebase [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) } // Checkout base branch again - if err := git.NewCommand("checkout", pr.BaseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + if err := git.NewCommand("checkout", baseBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { return fmt.Errorf("git checkout: %s", errbuf.String()) } // Prepare merge with commit From a56c972ce927a6e372ae8dab458c2d241e760b3e Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 8 Aug 2019 21:44:43 +0100 Subject: [PATCH 07/11] Final rm --- models/repo_mirror.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/repo_mirror.go b/models/repo_mirror.go index c62834f6fb69..982672ea2b83 100644 --- a/models/repo_mirror.go +++ b/models/repo_mirror.go @@ -134,7 +134,7 @@ func (m *Mirror) FullAddress() string { func (m *Mirror) SaveAddress(addr string) error { repoPath := m.Repo.RepoPath() // Remove old origin - _, err := git.NewCommand("remote", "remove", "origin").RunInDir(repoPath) + _, err := git.NewCommand("remote", "rm", "origin").RunInDir(repoPath) if err != nil && !strings.HasPrefix(err.Error(), "exit status 128 - fatal: No such remote ") { return err } From 5a2e2dbc3036c507eed41a67e60237d1e8d87e56 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 14 Aug 2019 14:42:48 +0100 Subject: [PATCH 08/11] Ensure LFS filters run on the tests --- .../git_helper_for_declarative_test.go | 25 +++++++++++++++++++ integrations/git_test.go | 18 +++++++++++++ 2 files changed, 43 insertions(+) diff --git a/integrations/git_helper_for_declarative_test.go b/integrations/git_helper_for_declarative_test.go index a8bf5b89c029..d3f934d54c1a 100644 --- a/integrations/git_helper_for_declarative_test.go +++ b/integrations/git_helper_for_declarative_test.go @@ -14,6 +14,7 @@ import ( "os" "path" "path/filepath" + "strings" "testing" "time" @@ -59,6 +60,24 @@ func createSSHUrl(gitPath string, u *url.URL) *url.URL { return &u2 } +func allowLFSFilters() []string { + // Now here we should explicitly allow lfs filters to run + globalArgs := git.GlobalCommandArgs + filteredLFSGlobalArgs := make([]string, len(git.GlobalCommandArgs)) + j := 0 + for _, arg := range git.GlobalCommandArgs { + if strings.Contains(arg, "lfs") { + j-- + } else { + filteredLFSGlobalArgs[j] = arg + j++ + } + } + filteredLFSGlobalArgs = filteredLFSGlobalArgs[:j] + git.GlobalCommandArgs = filteredLFSGlobalArgs + return globalArgs +} + func onGiteaRun(t *testing.T, callback func(*testing.T, *url.URL)) { prepareTestEnv(t, 1) s := http.Server{ @@ -84,7 +103,9 @@ func onGiteaRun(t *testing.T, callback func(*testing.T, *url.URL)) { func doGitClone(dstLocalPath string, u *url.URL) func(*testing.T) { return func(t *testing.T) { + oldGlobals := allowLFSFilters() assert.NoError(t, git.Clone(u.String(), dstLocalPath, git.CloneRepoOptions{})) + git.GlobalCommandArgs = oldGlobals assert.True(t, com.IsExist(filepath.Join(dstLocalPath, "README.md"))) } } @@ -145,7 +166,9 @@ func doGitCreateBranch(dstPath, branch string) func(*testing.T) { func doGitCheckoutBranch(dstPath string, args ...string) func(*testing.T) { return func(t *testing.T) { + oldGlobals := allowLFSFilters() _, err := git.NewCommand(append([]string{"checkout"}, args...)...).RunInDir(dstPath) + git.GlobalCommandArgs = oldGlobals assert.NoError(t, err) } } @@ -159,7 +182,9 @@ func doGitMerge(dstPath string, args ...string) func(*testing.T) { func doGitPull(dstPath string, args ...string) func(*testing.T) { return func(t *testing.T) { + oldGlobals := allowLFSFilters() _, err := git.NewCommand(append([]string{"pull"}, args...)...).RunInDir(dstPath) + git.GlobalCommandArgs = oldGlobals assert.NoError(t, err) } } diff --git a/integrations/git_test.go b/integrations/git_test.go index 4e014fe2d728..dbcc26522765 100644 --- a/integrations/git_test.go +++ b/integrations/git_test.go @@ -148,6 +148,21 @@ func lfsCommitAndPushTest(t *testing.T, dstPath string) (littleLFS, bigLFS strin assert.NoError(t, err) err = git.AddChanges(dstPath, false, ".gitattributes") assert.NoError(t, err) + oldGlobals := allowLFSFilters() + err = git.CommitChanges(dstPath, git.CommitChangesOptions{ + Committer: &git.Signature{ + Email: "user2@example.com", + Name: "User Two", + When: time.Now(), + }, + Author: &git.Signature{ + Email: "user2@example.com", + Name: "User Two", + When: time.Now(), + }, + Message: fmt.Sprintf("Testing commit @ %v", time.Now()), + }) + git.GlobalCommandArgs = oldGlobals littleLFS, bigLFS = commitAndPushTest(t, dstPath, prefix) @@ -290,6 +305,8 @@ func generateCommitWithNewData(size int, repoPath, email, fullName, prefix strin } //Commit + // Now here we should explicitly allow lfs filters to run + oldGlobals := allowLFSFilters() err = git.AddChanges(repoPath, false, filepath.Base(tmpFile.Name())) if err != nil { return "", err @@ -307,6 +324,7 @@ func generateCommitWithNewData(size int, repoPath, email, fullName, prefix strin }, Message: fmt.Sprintf("Testing commit @ %v", time.Now()), }) + git.GlobalCommandArgs = oldGlobals return filepath.Base(tmpFile.Name()), err } From afa8f3a37d4fb3af296cf60ec95372e4a915aa36 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 14 Aug 2019 16:11:59 +0100 Subject: [PATCH 09/11] Do not sign commits from temp repo --- modules/repofiles/temp_repo.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/modules/repofiles/temp_repo.go b/modules/repofiles/temp_repo.go index 04f6def78b12..d430d27e0ee9 100644 --- a/modules/repofiles/temp_repo.go +++ b/modules/repofiles/temp_repo.go @@ -254,6 +254,11 @@ func (t *TemporaryUploadRepository) CommitTree(author, committer *models.User, t authorSig := author.NewGitSig() committerSig := committer.NewGitSig() + binVersion, err := git.BinVersion() + if err != nil { + return "", fmt.Errorf("Unable to get git version: %v", err) + } + // FIXME: Should we add SSH_ORIGINAL_COMMAND to this // Because this may call hooks we should pass in the environment env := append(os.Environ(), @@ -267,12 +272,18 @@ func (t *TemporaryUploadRepository) CommitTree(author, committer *models.User, t messageBytes := new(bytes.Buffer) _, _ = messageBytes.WriteString(message) _, _ = messageBytes.WriteString("\n") + + args := []string{"commit-tree", treeHash, "-p", "HEAD"} + if version.Compare(binVersion, "2.0.0", ">=") { + args = append(args, "--no-gpg-sign") + } + commitHash, stderr, err := process.GetManager().ExecDirEnvStdIn(5*time.Minute, t.basePath, fmt.Sprintf("commitTree (git commit-tree): %s", t.basePath), env, messageBytes, - git.GitExecutable, "commit-tree", treeHash, "-p", "HEAD") + git.GitExecutable, args...) if err != nil { return "", fmt.Errorf("git commit-tree: %s", stderr) } From 5eaa3b7d804bd1e5834e11f859125acbe0fd6e8a Mon Sep 17 00:00:00 2001 From: zeripath Date: Thu, 10 Oct 2019 00:44:57 +0100 Subject: [PATCH 10/11] Apply suggestions from code review --- modules/repofiles/temp_repo.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/repofiles/temp_repo.go b/modules/repofiles/temp_repo.go index d430d27e0ee9..9fb5ef0f42ba 100644 --- a/modules/repofiles/temp_repo.go +++ b/modules/repofiles/temp_repo.go @@ -345,7 +345,8 @@ func (t *TemporaryUploadRepository) DiffIndex() (diff *models.Diff, err error) { func (t *TemporaryUploadRepository) CheckAttribute(attribute string, args ...string) (map[string]map[string]string, error) { binVersion, err := git.BinVersion() if err != nil { - log.Fatal("Error retrieving git version: %v", err) + log.Error("Error retrieving git version: %v", err) + return nil, err } stdOut := new(bytes.Buffer) @@ -358,7 +359,7 @@ func (t *TemporaryUploadRepository) CheckAttribute(attribute string, args ...str cmdArgs := []string{"check-attr", "-z", attribute} // git check-attr --cached first appears in git 1.8.0 - if version.Compare(binVersion, "1.8.0", ">=") { + if version.Compare(binVersion, "1.7.8", ">=") { cmdArgs = append(cmdArgs, "--cached") } cmdArgs = append(cmdArgs, "--") From 6fd28a393620c2d0a0f66521d8534d89d03b63e7 Mon Sep 17 00:00:00 2001 From: zeripath Date: Thu, 10 Oct 2019 01:02:55 +0100 Subject: [PATCH 11/11] Update modules/repofiles/temp_repo.go --- modules/repofiles/temp_repo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/repofiles/temp_repo.go b/modules/repofiles/temp_repo.go index 9fb5ef0f42ba..28f3836d0e11 100644 --- a/modules/repofiles/temp_repo.go +++ b/modules/repofiles/temp_repo.go @@ -358,7 +358,7 @@ func (t *TemporaryUploadRepository) CheckAttribute(attribute string, args ...str cmdArgs := []string{"check-attr", "-z", attribute} - // git check-attr --cached first appears in git 1.8.0 + // git check-attr --cached first appears in git 1.7.8 if version.Compare(binVersion, "1.7.8", ">=") { cmdArgs = append(cmdArgs, "--cached") }