From 4a16c7ff4f7ee73cc9babd374c99887dcaff2650 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 31 May 2019 16:18:56 +0100 Subject: [PATCH 01/12] move merge out of models --- models/pull.go | 284 +------------------------------ modules/merge/merge.go | 327 ++++++++++++++++++++++++++++++++++++ routers/api/v1/repo/pull.go | 3 +- routers/repo/pull.go | 3 +- 4 files changed, 334 insertions(+), 283 deletions(-) create mode 100644 modules/merge/merge.go diff --git a/models/pull.go b/models/pull.go index fe18765fc0c78..17719e316b843 100644 --- a/models/pull.go +++ b/models/pull.go @@ -7,7 +7,6 @@ package models import ( "bufio" - "bytes" "fmt" "io/ioutil" "os" @@ -18,7 +17,6 @@ import ( "time" "code.gitea.io/gitea/modules/base" - "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/process" @@ -361,284 +359,8 @@ func (pr *PullRequest) CheckUserAllowedToMerge(doer *User) (err error) { return nil } -func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { - getDiffTreeFromBranch := func(repoPath, baseBranch, headBranch string) (string, error) { - var outbuf, errbuf strings.Builder - // Compute the diff-tree for sparse-checkout - // The branch argument must be enclosed with double-quotes ("") in case it contains slashes (e.g "feature/test") - if err := git.NewCommand("diff-tree", "--no-commit-id", "--name-only", "-r", "--root", baseBranch, headBranch).RunInDirPipeline(repoPath, &outbuf, &errbuf); err != nil { - return "", fmt.Errorf("git diff-tree [%s base:%s head:%s]: %s", repoPath, baseBranch, headBranch, errbuf.String()) - } - return outbuf.String(), nil - } - - list, err := getDiffTreeFromBranch(repoPath, baseBranch, headBranch) - if err != nil { - return "", err - } - - // Prefixing '/' for each entry, otherwise all files with the same name in subdirectories would be matched. - out := bytes.Buffer{} - scanner := bufio.NewScanner(strings.NewReader(list)) - for scanner.Scan() { - fmt.Fprintf(&out, "/%s\n", scanner.Text()) - } - return out.String(), nil -} - -// Merge merges pull request to base repository. -// FIXME: add repoWorkingPull make sure two merges does not happen at same time. -func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle MergeStyle, message string) (err error) { - if err = pr.GetHeadRepo(); err != nil { - return fmt.Errorf("GetHeadRepo: %v", err) - } else if err = pr.GetBaseRepo(); err != nil { - return fmt.Errorf("GetBaseRepo: %v", err) - } - - prUnit, err := pr.BaseRepo.GetUnit(UnitTypePullRequests) - if err != nil { - return err - } - prConfig := prUnit.PullRequestsConfig() - - if err := pr.CheckUserAllowedToMerge(doer); err != nil { - return fmt.Errorf("CheckUserAllowedToMerge: %v", err) - } - - // Check if merge style is correct and allowed - if !prConfig.IsMergeStyleAllowed(mergeStyle) { - return ErrInvalidMergeStyle{pr.BaseRepo.ID, mergeStyle} - } - - defer func() { - go HookQueue.Add(pr.BaseRepo.ID) - go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false) - }() - - // Clone base repo. - tmpBasePath, err := CreateTemporaryPath("merge") - if err != nil { - return err - } - defer RemoveTemporaryPath(tmpBasePath) - - headRepoPath := 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) - } - - remoteRepoName := "head_repo" - - // 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) - if err != nil { - return err - } - defer f.Close() - data := filepath.Join(cache, "objects") - if _, err := fmt.Fprintln(f, data); err != nil { - return err - } - return nil - } - - if err := addCacheRepo(tmpBasePath, headRepoPath); err != nil { - return fmt.Errorf("addCacheRepo [%s -> %s]: %v", headRepoPath, tmpBasePath, err) - } - - var errbuf strings.Builder - 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()) - } - - // Fetch head branch - if err := git.NewCommand("fetch", remoteRepoName).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 - sparseCheckoutList, err := getDiffTree(tmpBasePath, pr.BaseBranch, trackingBranch) - if err != nil { - return fmt.Errorf("getDiffTree: %v", err) - } - - infoPath := filepath.Join(tmpBasePath, ".git", "info") - if err := os.MkdirAll(infoPath, 0700); err != nil { - return fmt.Errorf("creating directory failed [%s]: %v", infoPath, err) - } - sparseCheckoutListPath := filepath.Join(infoPath, "sparse-checkout") - if err := ioutil.WriteFile(sparseCheckoutListPath, []byte(sparseCheckoutList), 0600); err != nil { - return fmt.Errorf("Writing sparse-checkout file to %s: %v", sparseCheckoutListPath, err) - } - - if err := git.NewCommand("config", "--local", "core.sparseCheckout", "true").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git config [core.sparsecheckout -> true]: %v", errbuf.String()) - } - - // Read base branch index - if err := git.NewCommand("read-tree", "HEAD").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git read-tree HEAD: %s", errbuf.String()) - } - - // Merge commits. - switch mergeStyle { - case MergeStyleMerge: - if err := git.NewCommand("merge", "--no-ff", "--no-commit", trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git merge --no-ff --no-commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) - } - - sig := doer.NewGitSig() - if err := git.NewCommand("commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), "-m", message).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) - } - case MergeStyleRebase: - // Checkout head branch - if err := git.NewCommand("checkout", "-b", stagingBranch, trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - 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 { - 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 { - return fmt.Errorf("git checkout: %s", errbuf.String()) - } - // Merge fast forward - if err := git.NewCommand("merge", "--ff-only", "-q", stagingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git merge --ff-only [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) - } - case MergeStyleRebaseMerge: - // Checkout head branch - if err := git.NewCommand("checkout", "-b", stagingBranch, trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - 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 { - 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 { - return fmt.Errorf("git checkout: %s", errbuf.String()) - } - // Prepare merge with commit - if err := git.NewCommand("merge", "--no-ff", "--no-commit", "-q", stagingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git merge --no-ff [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) - } - - // Set custom message and author and create merge commit - sig := doer.NewGitSig() - if err := git.NewCommand("commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), "-m", message).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) - } - - case MergeStyleSquash: - // Merge with squash - if err := git.NewCommand("merge", "-q", "--squash", trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git merge --squash [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) - } - sig := pr.Issue.Poster.NewGitSig() - if err := git.NewCommand("commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), "-m", message).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) - } - default: - return ErrInvalidMergeStyle{pr.BaseRepo.ID, mergeStyle} - } - - env := PushingEnvironment(doer, pr.BaseRepo) - - // Push back to upstream. - if err := git.NewCommand("push", baseGitRepo.Path, pr.BaseBranch).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git push: %s", errbuf.String()) - } - - pr.MergedCommitID, err = baseGitRepo.GetBranchCommitID(pr.BaseBranch) - if err != nil { - return fmt.Errorf("GetBranchCommit: %v", err) - } - - pr.MergedUnix = util.TimeStampNow() - pr.Merger = doer - pr.MergerID = doer.ID - - if err = pr.setMerged(); err != nil { - log.Error("setMerged [%d]: %v", pr.ID, err) - } - - if err = MergePullRequestAction(doer, pr.Issue.Repo, pr.Issue); err != nil { - log.Error("MergePullRequestAction [%d]: %v", pr.ID, err) - } - - // Reset cached commit count - cache.Remove(pr.Issue.Repo.GetCommitsCountCacheKey(pr.BaseBranch, true)) - - // Reload pull request information. - if err = pr.LoadAttributes(); err != nil { - log.Error("LoadAttributes: %v", err) - return nil - } - - mode, _ := AccessLevel(doer, pr.Issue.Repo) - if err = PrepareWebhooks(pr.Issue.Repo, HookEventPullRequest, &api.PullRequestPayload{ - Action: api.HookIssueClosed, - Index: pr.Index, - PullRequest: pr.APIFormat(), - Repository: pr.Issue.Repo.APIFormat(mode), - Sender: doer.APIFormat(), - }); err != nil { - log.Error("PrepareWebhooks: %v", err) - } else { - go HookQueue.Add(pr.Issue.Repo.ID) - } - - l, err := baseGitRepo.CommitsBetweenIDs(pr.MergedCommitID, pr.MergeBase) - if err != nil { - log.Error("CommitsBetweenIDs: %v", err) - return nil - } - - // It is possible that head branch is not fully sync with base branch for merge commits, - // so we need to get latest head commit and append merge commit manually - // to avoid strange diff commits produced. - mergeCommit, err := baseGitRepo.GetBranchCommit(pr.BaseBranch) - if err != nil { - log.Error("GetBranchCommit: %v", err) - return nil - } - if mergeStyle == MergeStyleMerge { - l.PushFront(mergeCommit) - } - - p := &api.PushPayload{ - Ref: git.BranchPrefix + pr.BaseBranch, - Before: pr.MergeBase, - After: mergeCommit.ID.String(), - CompareURL: setting.AppURL + pr.BaseRepo.ComposeCompareURL(pr.MergeBase, pr.MergedCommitID), - Commits: ListToPushCommits(l).ToAPIPayloadCommits(pr.BaseRepo.HTMLURL()), - Repo: pr.BaseRepo.APIFormat(mode), - Pusher: pr.HeadRepo.MustOwner().APIFormat(), - Sender: doer.APIFormat(), - } - if err = PrepareWebhooks(pr.BaseRepo, HookEventPush, p); err != nil { - log.Error("PrepareWebhooks: %v", err) - } else { - go HookQueue.Add(pr.BaseRepo.ID) - } - return nil -} - -// setMerged sets a pull request to merged and closes the corresponding issue -func (pr *PullRequest) setMerged() (err error) { +// SetMerged sets a pull request to merged and closes the corresponding issue +func (pr *PullRequest) SetMerged() (err error) { if pr.HasMerged { return fmt.Errorf("PullRequest[%d] already merged", pr.Index) } @@ -705,7 +427,7 @@ func (pr *PullRequest) manuallyMerged() bool { pr.Merger = merger pr.MergerID = merger.ID - if err = pr.setMerged(); err != nil { + if err = pr.SetMerged(); err != nil { log.Error("PullRequest[%d].setMerged : %v", pr.ID, err) return false } diff --git a/modules/merge/merge.go b/modules/merge/merge.go new file mode 100644 index 0000000000000..eaeec4d924cd7 --- /dev/null +++ b/modules/merge/merge.go @@ -0,0 +1,327 @@ +// 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. + +package merge + +import ( + "bufio" + "bytes" + "fmt" + "io/ioutil" + "os" + "path" + "path/filepath" + "strings" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/cache" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" + api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" +) + +// Merge merges pull request to base repository. +// FIXME: add repoWorkingPull make sure two merges does not happen at same time. +func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repository, mergeStyle models.MergeStyle, message string) (err error) { + if err = pr.GetHeadRepo(); err != nil { + return fmt.Errorf("GetHeadRepo: %v", err) + } else if err = pr.GetBaseRepo(); err != nil { + return fmt.Errorf("GetBaseRepo: %v", err) + } + + prUnit, err := pr.BaseRepo.GetUnit(models.UnitTypePullRequests) + if err != nil { + return err + } + prConfig := prUnit.PullRequestsConfig() + + if err := pr.CheckUserAllowedToMerge(doer); err != nil { + return fmt.Errorf("CheckUserAllowedToMerge: %v", err) + } + + // Check if merge style is correct and allowed + if !prConfig.IsMergeStyleAllowed(mergeStyle) { + return models.ErrInvalidMergeStyle{ID: pr.BaseRepo.ID, Style: mergeStyle} + } + + defer func() { + go models.HookQueue.Add(pr.BaseRepo.ID) + go models.AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false) + }() + + // Clone base repo. + tmpBasePath, err := models.CreateTemporaryPath("merge") + if err != nil { + return err + } + defer models.RemoveTemporaryPath(tmpBasePath) + + 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) + } + + remoteRepoName := "head_repo" + + // 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) + if err != nil { + return err + } + defer f.Close() + data := filepath.Join(cache, "objects") + if _, err := fmt.Fprintln(f, data); err != nil { + return err + } + return nil + } + + if err := addCacheRepo(tmpBasePath, headRepoPath); err != nil { + return fmt.Errorf("addCacheRepo [%s -> %s]: %v", headRepoPath, tmpBasePath, err) + } + + var errbuf strings.Builder + 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()) + } + + // Fetch head branch + if err := git.NewCommand("fetch", remoteRepoName).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 + sparseCheckoutList, err := getDiffTree(tmpBasePath, pr.BaseBranch, trackingBranch) + if err != nil { + return fmt.Errorf("getDiffTree: %v", err) + } + + infoPath := filepath.Join(tmpBasePath, ".git", "info") + if err := os.MkdirAll(infoPath, 0700); err != nil { + return fmt.Errorf("creating directory failed [%s]: %v", infoPath, err) + } + sparseCheckoutListPath := filepath.Join(infoPath, "sparse-checkout") + if err := ioutil.WriteFile(sparseCheckoutListPath, []byte(sparseCheckoutList), 0600); err != nil { + return fmt.Errorf("Writing sparse-checkout file to %s: %v", sparseCheckoutListPath, err) + } + + if err := git.NewCommand("config", "--local", "core.sparseCheckout", "true").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git config [core.sparsecheckout -> true]: %v", errbuf.String()) + } + + originLFSURL := setting.LocalURL + pr.HeadRepo.FullName() + ".git/info/lfs" + if err := git.NewCommand("config", "--local", "remote.origin.lfsurl", originLFSURL).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git config [remote.origin.lfsurl]: %v", errbuf.String()) + } + + originToken, err := models.GenerateLFSAuthenticationToken(doer, pr.HeadRepo, "upload") + if err != nil { + return fmt.Errorf("Failed to generate authentication token for lfs") + } + if err := git.NewCommand("config", "--local", "http."+originLFSURL+".extraheader", "Authorization: "+originToken.Header["Authorization"]).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git config [http.origin.extraheader]: %v", errbuf.String()) + } + + remoteLFSURL := setting.LocalURL + pr.BaseRepo.FullName() + ".git/info/lfs" + if err := git.NewCommand("config", "--local", "remote."+remoteRepoName+".lfsurl", remoteLFSURL).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git config [remote.remote.lfsurl]: %v", errbuf.String()) + } + + remoteToken, err := models.GenerateLFSAuthenticationToken(doer, pr.BaseRepo, "upload") + if err != nil { + return fmt.Errorf("Failed to generate authentication token for lfs") + } + if err := git.NewCommand("config", "--local", "http."+remoteLFSURL+".extraheader", "Authorization: "+remoteToken.Header["Authorization"]).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git config [http.remote.extraheader]: %v", errbuf.String()) + } + + // Read base branch index + if err := git.NewCommand("read-tree", "HEAD").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git read-tree HEAD: %s", errbuf.String()) + } + + // Merge commits. + switch mergeStyle { + case models.MergeStyleMerge: + if err := git.NewCommand("merge", "--no-ff", "--no-commit", trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git merge --no-ff --no-commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + } + + sig := doer.NewGitSig() + if err := git.NewCommand("commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), "-m", message).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + } + case models.MergeStyleRebase: + // Checkout head branch + if err := git.NewCommand("checkout", "-b", stagingBranch, trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + 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 { + 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 { + return fmt.Errorf("git checkout: %s", errbuf.String()) + } + // Merge fast forward + if err := git.NewCommand("merge", "--ff-only", "-q", stagingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git merge --ff-only [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + } + case models.MergeStyleRebaseMerge: + // Checkout head branch + if err := git.NewCommand("checkout", "-b", stagingBranch, trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + 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 { + 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 { + return fmt.Errorf("git checkout: %s", errbuf.String()) + } + // Prepare merge with commit + if err := git.NewCommand("merge", "--no-ff", "--no-commit", "-q", stagingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git merge --no-ff [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + } + + // Set custom message and author and create merge commit + sig := doer.NewGitSig() + if err := git.NewCommand("commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), "-m", message).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + } + + case models.MergeStyleSquash: + // Merge with squash + if err := git.NewCommand("merge", "-q", "--squash", trackingBranch).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git merge --squash [%s -> %s]: %s", headRepoPath, tmpBasePath, errbuf.String()) + } + sig := pr.Issue.Poster.NewGitSig() + if err := git.NewCommand("commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), "-m", message).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + } + default: + return models.ErrInvalidMergeStyle{ID: pr.BaseRepo.ID, Style: mergeStyle} + } + + env := models.PushingEnvironment(doer, pr.BaseRepo) + + // Push back to upstream. + if err := git.NewCommand("push", "origin", pr.BaseBranch).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git push: %s", errbuf.String()) + } + + pr.MergedCommitID, err = baseGitRepo.GetBranchCommitID(pr.BaseBranch) + if err != nil { + return fmt.Errorf("GetBranchCommit: %v", err) + } + + pr.MergedUnix = util.TimeStampNow() + pr.Merger = doer + pr.MergerID = doer.ID + + if err = pr.SetMerged(); err != nil { + log.Error("setMerged [%d]: %v", pr.ID, err) + } + + if err = models.MergePullRequestAction(doer, pr.Issue.Repo, pr.Issue); err != nil { + log.Error("MergePullRequestAction [%d]: %v", pr.ID, err) + } + + // Reset cached commit count + cache.Remove(pr.Issue.Repo.GetCommitsCountCacheKey(pr.BaseBranch, true)) + + // Reload pull request information. + if err = pr.LoadAttributes(); err != nil { + log.Error("LoadAttributes: %v", err) + return nil + } + + mode, _ := models.AccessLevel(doer, pr.Issue.Repo) + if err = models.PrepareWebhooks(pr.Issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{ + Action: api.HookIssueClosed, + Index: pr.Index, + PullRequest: pr.APIFormat(), + Repository: pr.Issue.Repo.APIFormat(mode), + Sender: doer.APIFormat(), + }); err != nil { + log.Error("PrepareWebhooks: %v", err) + } else { + go models.HookQueue.Add(pr.Issue.Repo.ID) + } + + l, err := baseGitRepo.CommitsBetweenIDs(pr.MergedCommitID, pr.MergeBase) + if err != nil { + log.Error("CommitsBetweenIDs: %v", err) + return nil + } + + // It is possible that head branch is not fully sync with base branch for merge commits, + // so we need to get latest head commit and append merge commit manually + // to avoid strange diff commits produced. + mergeCommit, err := baseGitRepo.GetBranchCommit(pr.BaseBranch) + if err != nil { + log.Error("GetBranchCommit: %v", err) + return nil + } + if mergeStyle == models.MergeStyleMerge { + l.PushFront(mergeCommit) + } + + p := &api.PushPayload{ + Ref: git.BranchPrefix + pr.BaseBranch, + Before: pr.MergeBase, + After: mergeCommit.ID.String(), + CompareURL: setting.AppURL + pr.BaseRepo.ComposeCompareURL(pr.MergeBase, pr.MergedCommitID), + Commits: models.ListToPushCommits(l).ToAPIPayloadCommits(pr.BaseRepo.HTMLURL()), + Repo: pr.BaseRepo.APIFormat(mode), + Pusher: pr.HeadRepo.MustOwner().APIFormat(), + Sender: doer.APIFormat(), + } + if err = models.PrepareWebhooks(pr.BaseRepo, models.HookEventPush, p); err != nil { + log.Error("PrepareWebhooks: %v", err) + } else { + go models.HookQueue.Add(pr.BaseRepo.ID) + } + return nil +} + +func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { + getDiffTreeFromBranch := func(repoPath, baseBranch, headBranch string) (string, error) { + var outbuf, errbuf strings.Builder + // Compute the diff-tree for sparse-checkout + // The branch argument must be enclosed with double-quotes ("") in case it contains slashes (e.g "feature/test") + if err := git.NewCommand("diff-tree", "--no-commit-id", "--name-only", "-r", "--root", baseBranch, headBranch).RunInDirPipeline(repoPath, &outbuf, &errbuf); err != nil { + return "", fmt.Errorf("git diff-tree [%s base:%s head:%s]: %s", repoPath, baseBranch, headBranch, errbuf.String()) + } + return outbuf.String(), nil + } + + list, err := getDiffTreeFromBranch(repoPath, baseBranch, headBranch) + if err != nil { + return "", err + } + + // Prefixing '/' for each entry, otherwise all files with the same name in subdirectories would be matched. + out := bytes.Buffer{} + scanner := bufio.NewScanner(strings.NewReader(list)) + for scanner.Scan() { + fmt.Fprintf(&out, "/%s\n", scanner.Text()) + } + return out.String(), nil +} diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index f53ab4b8f3e54..a24f60a5387f9 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/merge" "code.gitea.io/gitea/modules/notification" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" @@ -587,7 +588,7 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) { message += "\n\n" + form.MergeMessageField } - if err := pr.Merge(ctx.User, ctx.Repo.GitRepo, models.MergeStyle(form.Do), message); err != nil { + if err := merge.Merge(pr, ctx.User, ctx.Repo.GitRepo, models.MergeStyle(form.Do), message); err != nil { if models.IsErrInvalidMergeStyle(err) { ctx.Status(405) return diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 412750dd55d28..7f0d8f3d6341e 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -19,6 +19,7 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/merge" "code.gitea.io/gitea/modules/notification" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -596,7 +597,7 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) { return } - if err = pr.Merge(ctx.User, ctx.Repo.GitRepo, models.MergeStyle(form.Do), message); err != nil { + if err = merge.Merge(pr, ctx.User, ctx.Repo.GitRepo, models.MergeStyle(form.Do), message); err != nil { if models.IsErrInvalidMergeStyle(err) { ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option")) ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) From ed58d7acad3236508cf9672cbe5c17a37c2429cf Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 31 May 2019 13:22:26 +0100 Subject: [PATCH 02/12] Replicate git lfs (pre-)merge hook This switches from relying on having git-lfs installed on the server, (and in fact .gitattributes being correctly installed.) Instead on merge we walk the merge history and ensure that all lfs objects pointed to in the history are added to the base repository. --- modules/merge/merge.go | 199 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 176 insertions(+), 23 deletions(-) diff --git a/modules/merge/merge.go b/modules/merge/merge.go index eaeec4d924cd7..706f629495ba5 100644 --- a/modules/merge/merge.go +++ b/modules/merge/merge.go @@ -9,11 +9,16 @@ import ( "bufio" "bytes" "fmt" + "io" "io/ioutil" "os" "path" "path/filepath" + "strconv" "strings" + "sync" + + "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/cache" @@ -119,34 +124,22 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("Writing sparse-checkout file to %s: %v", sparseCheckoutListPath, err) } - if err := git.NewCommand("config", "--local", "core.sparseCheckout", "true").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git config [core.sparsecheckout -> true]: %v", errbuf.String()) - } - - originLFSURL := setting.LocalURL + pr.HeadRepo.FullName() + ".git/info/lfs" - if err := git.NewCommand("config", "--local", "remote.origin.lfsurl", originLFSURL).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git config [remote.origin.lfsurl]: %v", errbuf.String()) + // 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 { + return fmt.Errorf("git config [filter.lfs.process -> <> ]: %v", errbuf.String()) } - - originToken, err := models.GenerateLFSAuthenticationToken(doer, pr.HeadRepo, "upload") - if err != nil { - return fmt.Errorf("Failed to generate authentication token for lfs") + if err := git.NewCommand("config", "--local", "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", "http."+originLFSURL+".extraheader", "Authorization: "+originToken.Header["Authorization"]).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git config [http.origin.extraheader]: %v", errbuf.String()) + if err := git.NewCommand("config", "--local", "filter.lfs.clean", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git config [filter.lfs.clean -> <> ]: %v", errbuf.String()) } - - remoteLFSURL := setting.LocalURL + pr.BaseRepo.FullName() + ".git/info/lfs" - if err := git.NewCommand("config", "--local", "remote."+remoteRepoName+".lfsurl", remoteLFSURL).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git config [remote.remote.lfsurl]: %v", errbuf.String()) + if err := git.NewCommand("config", "--local", "filter.lfs.smudge", "").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git config [filter.lfs.smudge -> <> ]: %v", errbuf.String()) } - remoteToken, err := models.GenerateLFSAuthenticationToken(doer, pr.BaseRepo, "upload") - if err != nil { - return fmt.Errorf("Failed to generate authentication token for lfs") - } - if err := git.NewCommand("config", "--local", "http."+remoteLFSURL+".extraheader", "Authorization: "+remoteToken.Header["Authorization"]).RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { - return fmt.Errorf("git config [http.remote.extraheader]: %v", errbuf.String()) + if err := git.NewCommand("config", "--local", "core.sparseCheckout", "true").RunInDirPipeline(tmpBasePath, nil, &errbuf); err != nil { + return fmt.Errorf("git config [core.sparsecheckout -> true]: %v", errbuf.String()) } // Read base branch index @@ -219,6 +212,166 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return models.ErrInvalidMergeStyle{ID: pr.BaseRepo.ID, Style: mergeStyle} } + if setting.LFS.StartServer { + // Now we have to implement git lfs pre-push + // git rev-list --objects --filter=blob:limit=1k HEAD --not base + // pass blob shas in to git cat-file --batch-check (possibly unnecessary) + // ensure only blobs and <=1k size then pass in to git cat-file --batch + // to read each sha and check each as a pointer + // Then if they are lfs -> add them to the baseRepo + revListReader, revListWriter := io.Pipe() + shasToCheckReader, shasToCheckWriter := io.Pipe() + catFileCheckReader, catFileCheckWriter := io.Pipe() + shasToBatchReader, shasToBatchWriter := io.Pipe() + catFileBatchReader, catFileBatchWriter := io.Pipe() + errChan := make(chan error, 1) + wg := sync.WaitGroup{} + wg.Add(6) + go func() { + defer wg.Done() + defer catFileBatchReader.Close() + + bufferedReader := bufio.NewReader(catFileBatchReader) + buf := make([]byte, 1025) + for { + // File descriptor line + _, err := bufferedReader.ReadString(' ') + if err != nil { + catFileBatchReader.CloseWithError(err) + break + } + // Throw away the blob + if _, err := bufferedReader.ReadString(' '); err != nil { + catFileBatchReader.CloseWithError(err) + break + } + sizeStr, err := bufferedReader.ReadString('\n') + if err != nil { + catFileBatchReader.CloseWithError(err) + break + } + size, err := strconv.Atoi(sizeStr[:len(sizeStr)-1]) + if err != nil { + catFileBatchReader.CloseWithError(err) + break + } + pointerBuf := buf[:size+1] + if _, err := io.ReadFull(bufferedReader, pointerBuf); err != nil { + catFileBatchReader.CloseWithError(err) + break + } + pointerBuf = pointerBuf[:size] + // now we need to check if the pointerBuf is an LFS pointer + pointer := lfs.IsPointerFile(&pointerBuf) + if pointer == nil { + continue + } + pointer.RepositoryID = pr.BaseRepoID + if _, err := models.NewLFSMetaObject(pointer); err != nil { + catFileBatchReader.CloseWithError(err) + break + } + + } + }() + go func() { + defer wg.Done() + defer shasToBatchReader.Close() + defer catFileBatchWriter.Close() + + stderr := new(bytes.Buffer) + if err := git.NewCommand("cat-file", "--batch").RunInDirFullPipeline(tmpBasePath, catFileBatchWriter, stderr, shasToBatchReader); err != nil { + shasToBatchReader.CloseWithError(fmt.Errorf("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String())) + } + }() + go func() { + defer wg.Done() + defer catFileCheckReader.Close() + + scanner := bufio.NewScanner(catFileCheckReader) + defer shasToBatchWriter.CloseWithError(scanner.Err()) + for scanner.Scan() { + line := scanner.Text() + if len(line) == 0 { + continue + } + fields := strings.Split(line, " ") + if len(fields) < 3 || fields[1] != "blob" { + continue + } + size, _ := strconv.Atoi(string(fields[2])) + if size > 1024 { + continue + } + toWrite := []byte(fields[0] + "\n") + for len(toWrite) > 0 { + n, err := shasToBatchWriter.Write(toWrite) + if err != nil { + catFileCheckReader.CloseWithError(err) + break + } + toWrite = toWrite[n:] + } + } + }() + go func() { + defer wg.Done() + defer shasToCheckReader.Close() + defer catFileCheckWriter.Close() + + stderr := new(bytes.Buffer) + cmd := git.NewCommand("cat-file", "--batch-check") + if err := cmd.RunInDirFullPipeline(tmpBasePath, catFileCheckWriter, stderr, shasToCheckReader); err != nil { + shasToCheckWriter.CloseWithError(fmt.Errorf("git cat-file --batch-check [%s]: %v - %s", tmpBasePath, err, errbuf.String())) + } + }() + go func() { + defer wg.Done() + defer revListReader.Close() + defer shasToCheckWriter.Close() + scanner := bufio.NewScanner(revListReader) + for scanner.Scan() { + line := scanner.Text() + if len(line) == 0 { + continue + } + fields := strings.Split(line, " ") + if len(fields) < 2 || len(fields[1]) == 0 { + continue + } + toWrite := []byte(fields[0] + "\n") + for len(toWrite) > 0 { + n, err := shasToCheckWriter.Write(toWrite) + if err != nil { + revListReader.CloseWithError(err) + break + } + toWrite = toWrite[n:] + } + } + shasToCheckWriter.CloseWithError(scanner.Err()) + }() + go func() { + defer wg.Done() + defer revListWriter.Close() + stderr := new(bytes.Buffer) + cmd := git.NewCommand("rev-list", "--objects", "--filter=blob:limit=1k", "HEAD", "--not", "origin/"+pr.BaseBranch) + if err := cmd.RunInDirPipeline(tmpBasePath, revListWriter, stderr); err != nil { + log.Error("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + errChan <- fmt.Errorf("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + } + }() + + wg.Wait() + select { + case err, has := <-errChan: + if has { + return err + } + default: + } + } + env := models.PushingEnvironment(doer, pr.BaseRepo) // Push back to upstream. From 02a0c26d44595d0767a28c19d84690f16b9279a0 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 31 May 2019 14:05:19 +0100 Subject: [PATCH 03/12] drop --filter as not available in 2.7 --- modules/merge/merge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/merge/merge.go b/modules/merge/merge.go index 706f629495ba5..3262e53c32774 100644 --- a/modules/merge/merge.go +++ b/modules/merge/merge.go @@ -355,7 +355,7 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor defer wg.Done() defer revListWriter.Close() stderr := new(bytes.Buffer) - cmd := git.NewCommand("rev-list", "--objects", "--filter=blob:limit=1k", "HEAD", "--not", "origin/"+pr.BaseBranch) + cmd := git.NewCommand("rev-list", "--objects", "HEAD", "--not", "origin/"+pr.BaseBranch) if err := cmd.RunInDirPipeline(tmpBasePath, revListWriter, stderr); err != nil { log.Error("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String()) errChan <- fmt.Errorf("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String()) From d6b24f0c73c4c72307ec770cccb2e11885966909 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 31 May 2019 16:21:58 +0100 Subject: [PATCH 04/12] Fix imports --- modules/merge/merge.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/merge/merge.go b/modules/merge/merge.go index 3262e53c32774..e2c9a467f6662 100644 --- a/modules/merge/merge.go +++ b/modules/merge/merge.go @@ -18,11 +18,10 @@ import ( "strings" "sync" - "code.gitea.io/gitea/modules/lfs" - "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" From a63414e7c62c11c25e34b040a0e036bd3193f557 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 1 Jun 2019 10:52:28 +0100 Subject: [PATCH 05/12] refactor making the lfs push clearer --- modules/merge/merge.go | 374 ++++++++++++++++++++++++----------------- 1 file changed, 218 insertions(+), 156 deletions(-) diff --git a/modules/merge/merge.go b/modules/merge/merge.go index e2c9a467f6662..f7d5f6198d101 100644 --- a/modules/merge/merge.go +++ b/modules/merge/merge.go @@ -211,163 +211,22 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return models.ErrInvalidMergeStyle{ID: pr.BaseRepo.ID, Style: mergeStyle} } - if setting.LFS.StartServer { - // Now we have to implement git lfs pre-push - // git rev-list --objects --filter=blob:limit=1k HEAD --not base - // pass blob shas in to git cat-file --batch-check (possibly unnecessary) - // ensure only blobs and <=1k size then pass in to git cat-file --batch - // to read each sha and check each as a pointer - // Then if they are lfs -> add them to the baseRepo - revListReader, revListWriter := io.Pipe() - shasToCheckReader, shasToCheckWriter := io.Pipe() - catFileCheckReader, catFileCheckWriter := io.Pipe() - shasToBatchReader, shasToBatchWriter := io.Pipe() - catFileBatchReader, catFileBatchWriter := io.Pipe() - errChan := make(chan error, 1) - wg := sync.WaitGroup{} - wg.Add(6) - go func() { - defer wg.Done() - defer catFileBatchReader.Close() - - bufferedReader := bufio.NewReader(catFileBatchReader) - buf := make([]byte, 1025) - for { - // File descriptor line - _, err := bufferedReader.ReadString(' ') - if err != nil { - catFileBatchReader.CloseWithError(err) - break - } - // Throw away the blob - if _, err := bufferedReader.ReadString(' '); err != nil { - catFileBatchReader.CloseWithError(err) - break - } - sizeStr, err := bufferedReader.ReadString('\n') - if err != nil { - catFileBatchReader.CloseWithError(err) - break - } - size, err := strconv.Atoi(sizeStr[:len(sizeStr)-1]) - if err != nil { - catFileBatchReader.CloseWithError(err) - break - } - pointerBuf := buf[:size+1] - if _, err := io.ReadFull(bufferedReader, pointerBuf); err != nil { - catFileBatchReader.CloseWithError(err) - break - } - pointerBuf = pointerBuf[:size] - // now we need to check if the pointerBuf is an LFS pointer - pointer := lfs.IsPointerFile(&pointerBuf) - if pointer == nil { - continue - } - pointer.RepositoryID = pr.BaseRepoID - if _, err := models.NewLFSMetaObject(pointer); err != nil { - catFileBatchReader.CloseWithError(err) - break - } - - } - }() - go func() { - defer wg.Done() - defer shasToBatchReader.Close() - defer catFileBatchWriter.Close() - - stderr := new(bytes.Buffer) - if err := git.NewCommand("cat-file", "--batch").RunInDirFullPipeline(tmpBasePath, catFileBatchWriter, stderr, shasToBatchReader); err != nil { - shasToBatchReader.CloseWithError(fmt.Errorf("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String())) - } - }() - go func() { - defer wg.Done() - defer catFileCheckReader.Close() - - scanner := bufio.NewScanner(catFileCheckReader) - defer shasToBatchWriter.CloseWithError(scanner.Err()) - for scanner.Scan() { - line := scanner.Text() - if len(line) == 0 { - continue - } - fields := strings.Split(line, " ") - if len(fields) < 3 || fields[1] != "blob" { - continue - } - size, _ := strconv.Atoi(string(fields[2])) - if size > 1024 { - continue - } - toWrite := []byte(fields[0] + "\n") - for len(toWrite) > 0 { - n, err := shasToBatchWriter.Write(toWrite) - if err != nil { - catFileCheckReader.CloseWithError(err) - break - } - toWrite = toWrite[n:] - } - } - }() - go func() { - defer wg.Done() - defer shasToCheckReader.Close() - defer catFileCheckWriter.Close() - - stderr := new(bytes.Buffer) - cmd := git.NewCommand("cat-file", "--batch-check") - if err := cmd.RunInDirFullPipeline(tmpBasePath, catFileCheckWriter, stderr, shasToCheckReader); err != nil { - shasToCheckWriter.CloseWithError(fmt.Errorf("git cat-file --batch-check [%s]: %v - %s", tmpBasePath, err, errbuf.String())) - } - }() - go func() { - defer wg.Done() - defer revListReader.Close() - defer shasToCheckWriter.Close() - scanner := bufio.NewScanner(revListReader) - for scanner.Scan() { - line := scanner.Text() - if len(line) == 0 { - continue - } - fields := strings.Split(line, " ") - if len(fields) < 2 || len(fields[1]) == 0 { - continue - } - toWrite := []byte(fields[0] + "\n") - for len(toWrite) > 0 { - n, err := shasToCheckWriter.Write(toWrite) - if err != nil { - revListReader.CloseWithError(err) - break - } - toWrite = toWrite[n:] - } - } - shasToCheckWriter.CloseWithError(scanner.Err()) - }() - go func() { - defer wg.Done() - defer revListWriter.Close() - stderr := new(bytes.Buffer) - cmd := git.NewCommand("rev-list", "--objects", "HEAD", "--not", "origin/"+pr.BaseBranch) - if err := cmd.RunInDirPipeline(tmpBasePath, revListWriter, stderr); err != nil { - log.Error("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String()) - errChan <- fmt.Errorf("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String()) - } - }() + // OK we should cache our current head and origin/headbranch + mergeHeadSHA, err := git.GetFullCommitID(tmpBasePath, "HEAD") + if err != nil { + return fmt.Errorf("Failed to get full commit id for HEAD: %v", err) + } + mergeBaseSHA, err := git.GetFullCommitID(tmpBasePath, "origin/"+pr.BaseBranch) + if err != nil { + return fmt.Errorf("Failed to get full commit id for origin/%s: %v", pr.BaseBranch, err) + } - wg.Wait() - select { - case err, has := <-errChan: - if has { - return err - } - default: + // Now it's questionable about where this should go - either after or before the push + // I think in the interests of data safety - failures to push to the lfs should prevent + // the merge as you can always remerge. + if setting.LFS.StartServer { + if err := LFSPush(tmpBasePath, mergeHeadSHA, mergeBaseSHA, pr); err != nil { + return err } } @@ -477,3 +336,206 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { } return out.String(), nil } + +// LFSPush pushes lfs objects referred to in new commits in the head repository from the base repository +func LFSPush(tmpBasePath, mergeHeadSHA, mergeBaseSHA string, pr *models.PullRequest) error { + // Now we have to implement git lfs push + // git rev-list --objects --filter=blob:limit=1k HEAD --not base + // pass blob shas in to git cat-file --batch-check (possibly unnecessary) + // ensure only blobs and <=1k size then pass in to git cat-file --batch + // to read each sha and check each as a pointer + // Then if they are lfs -> add them to the baseRepo + revListReader, revListWriter := io.Pipe() + shasToCheckReader, shasToCheckWriter := io.Pipe() + catFileCheckReader, catFileCheckWriter := io.Pipe() + shasToBatchReader, shasToBatchWriter := io.Pipe() + catFileBatchReader, catFileBatchWriter := io.Pipe() + errChan := make(chan error, 1) + wg := sync.WaitGroup{} + wg.Add(6) + // Create the go-routines in reverse order. + + // 6. Take the output of cat-file --batch and check if each file in turn + // to see if they're pointers to files in the LFS store associated with + // the head repo and add them to the base repo if so + go readCatFileBatch(catFileBatchReader, &wg, pr) + + // 5. Take the shas of the blobs and batch read them + go doCatFileBatch(shasToBatchReader, catFileBatchWriter, &wg, tmpBasePath) + + // 4. From the provided objects restrict to blobs <=1k + go readCatFileBatchCheck(catFileCheckReader, shasToBatchWriter, &wg) + + // 3. Run batch-check on the objects retrieved from rev-list + go doCatFileBatchCheck(shasToCheckReader, catFileCheckWriter, &wg, tmpBasePath) + + // 2. Check each object retrieved rejecting those without names as they will be commits or trees + go readRevListObjects(revListReader, shasToCheckWriter, &wg) + + // 1. Run rev-list objects from mergeHead to mergeBase + go doRevListObjects(revListWriter, &wg, tmpBasePath, mergeHeadSHA, mergeBaseSHA, errChan) + + wg.Wait() + select { + case err, has := <-errChan: + if has { + return err + } + default: + } + return nil +} + +func doRevListObjects(revListWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath, headSHA, baseSHA string, errChan chan<- error) { + defer wg.Done() + defer revListWriter.Close() + stderr := new(bytes.Buffer) + var errbuf strings.Builder + cmd := git.NewCommand("rev-list", "--objects", headSHA, "--not", baseSHA) + if err := cmd.RunInDirPipeline(tmpBasePath, revListWriter, stderr); err != nil { + log.Error("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + errChan <- fmt.Errorf("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + } +} + +func readRevListObjects(revListReader *io.PipeReader, shasToCheckWriter *io.PipeWriter, wg *sync.WaitGroup) { + defer wg.Done() + defer revListReader.Close() + defer shasToCheckWriter.Close() + scanner := bufio.NewScanner(revListReader) + for scanner.Scan() { + line := scanner.Text() + if len(line) == 0 { + continue + } + fields := strings.Split(line, " ") + if len(fields) < 2 || len(fields[1]) == 0 { + continue + } + toWrite := []byte(fields[0] + "\n") + for len(toWrite) > 0 { + n, err := shasToCheckWriter.Write(toWrite) + if err != nil { + revListReader.CloseWithError(err) + break + } + toWrite = toWrite[n:] + } + } + shasToCheckWriter.CloseWithError(scanner.Err()) +} + +func doCatFileBatchCheck(shasToCheckReader *io.PipeReader, catFileCheckWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) { + defer wg.Done() + defer shasToCheckReader.Close() + defer catFileCheckWriter.Close() + + stderr := new(bytes.Buffer) + var errbuf strings.Builder + cmd := git.NewCommand("cat-file", "--batch-check") + if err := cmd.RunInDirFullPipeline(tmpBasePath, catFileCheckWriter, stderr, shasToCheckReader); err != nil { + catFileCheckWriter.CloseWithError(fmt.Errorf("git cat-file --batch-check [%s]: %v - %s", tmpBasePath, err, errbuf.String())) + } +} + +func readCatFileBatchCheck(catFileCheckReader *io.PipeReader, shasToBatchWriter *io.PipeWriter, wg *sync.WaitGroup) { + defer wg.Done() + defer catFileCheckReader.Close() + + scanner := bufio.NewScanner(catFileCheckReader) + defer shasToBatchWriter.CloseWithError(scanner.Err()) + for scanner.Scan() { + line := scanner.Text() + if len(line) == 0 { + continue + } + fields := strings.Split(line, " ") + if len(fields) < 3 || fields[1] != "blob" { + continue + } + size, _ := strconv.Atoi(string(fields[2])) + if size > 1024 { + continue + } + toWrite := []byte(fields[0] + "\n") + for len(toWrite) > 0 { + n, err := shasToBatchWriter.Write(toWrite) + if err != nil { + catFileCheckReader.CloseWithError(err) + break + } + toWrite = toWrite[n:] + } + } +} + +func doCatFileBatch(shasToBatchReader *io.PipeReader, catFileBatchWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) { + defer wg.Done() + defer shasToBatchReader.Close() + defer catFileBatchWriter.Close() + + stderr := new(bytes.Buffer) + var errbuf strings.Builder + if err := git.NewCommand("cat-file", "--batch").RunInDirFullPipeline(tmpBasePath, catFileBatchWriter, stderr, shasToBatchReader); err != nil { + shasToBatchReader.CloseWithError(fmt.Errorf("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String())) + } +} + +func readCatFileBatch(catFileBatchReader *io.PipeReader, wg *sync.WaitGroup, pr *models.PullRequest) { + defer wg.Done() + defer catFileBatchReader.Close() + + bufferedReader := bufio.NewReader(catFileBatchReader) + buf := make([]byte, 1025) + for { + // File descriptor line: sha + _, err := bufferedReader.ReadString(' ') + if err != nil { + catFileBatchReader.CloseWithError(err) + break + } + // Throw away the blob + if _, err := bufferedReader.ReadString(' '); err != nil { + catFileBatchReader.CloseWithError(err) + break + } + sizeStr, err := bufferedReader.ReadString('\n') + if err != nil { + catFileBatchReader.CloseWithError(err) + break + } + size, err := strconv.Atoi(sizeStr[:len(sizeStr)-1]) + if err != nil { + catFileBatchReader.CloseWithError(err) + break + } + pointerBuf := buf[:size+1] + if _, err := io.ReadFull(bufferedReader, pointerBuf); err != nil { + catFileBatchReader.CloseWithError(err) + break + } + pointerBuf = pointerBuf[:size] + // Now we need to check if the pointerBuf is an LFS pointer + pointer := lfs.IsPointerFile(&pointerBuf) + if pointer == nil { + continue + } + // Then we need to check that this pointer is in the db + if _, err := pr.HeadRepo.GetLFSMetaObjectByOid(pointer.Oid); err != nil { + if err == models.ErrLFSObjectNotExist { + log.Warn("During merge of: %d in %-v, there is a pointer to LFS Oid: %s which although present in the LFS store is not associated with the head repo %-v", pr.Index, pr.BaseRepo, pointer.Oid, pr.HeadRepo) + continue + } + catFileBatchReader.CloseWithError(err) + break + } + // OK we have a pointer that is associated with the head repo + // and is actually a file in the LFS + // Therefore it should be associated with the base repo + pointer.RepositoryID = pr.BaseRepoID + if _, err := models.NewLFSMetaObject(pointer); err != nil { + catFileBatchReader.CloseWithError(err) + break + } + } +} From 6ab301a1873b0508a337b893ad7df32874f43905 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 1 Jun 2019 12:27:06 +0100 Subject: [PATCH 06/12] Expand git_test to check that on merging forks lfs objects are available --- .../api_helper_for_declarative_test.go | 38 +++++++++++++++++ integrations/git_test.go | 41 ++++++++++++++++--- 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/integrations/api_helper_for_declarative_test.go b/integrations/api_helper_for_declarative_test.go index 85f0ab621f846..42c271e3afeee 100644 --- a/integrations/api_helper_for_declarative_test.go +++ b/integrations/api_helper_for_declarative_test.go @@ -66,6 +66,44 @@ func doAPICreateRepository(ctx APITestContext, empty bool, callback ...func(*tes } } +func doAPIAddCollaborator(ctx APITestContext, username string, mode models.AccessMode) func(*testing.T) { + return func(t *testing.T) { + permission := "read" + + if mode == models.AccessModeAdmin { + permission = "admin" + } else if mode > models.AccessModeRead { + permission = "write" + } + addCollaboratorOption := &api.AddCollaboratorOption{ + Permission: &permission, + } + req := NewRequestWithJSON(t, "PUT", fmt.Sprintf("/api/v1/repos/%s/%s/collaborators/%s?token=%s", ctx.Username, ctx.Reponame, username, ctx.Token), addCollaboratorOption) + if ctx.ExpectedCode != 0 { + ctx.Session.MakeRequest(t, req, ctx.ExpectedCode) + return + } + ctx.Session.MakeRequest(t, req, http.StatusNoContent) + } +} + +func doAPIForkRepository(ctx APITestContext, username string, callback ...func(*testing.T, api.Repository)) func(*testing.T) { + return func(t *testing.T) { + createForkOption := &api.CreateForkOption{} + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/forks?token=%s", username, ctx.Reponame, ctx.Token), createForkOption) + if ctx.ExpectedCode != 0 { + ctx.Session.MakeRequest(t, req, ctx.ExpectedCode) + return + } + resp := ctx.Session.MakeRequest(t, req, http.StatusAccepted) + var repository api.Repository + DecodeJSON(t, resp, &repository) + if len(callback) > 0 { + callback[0](t, repository) + } + } +} + func doAPIGetRepository(ctx APITestContext, callback ...func(*testing.T, api.Repository)) func(*testing.T) { return func(t *testing.T) { urlStr := fmt.Sprintf("/api/v1/repos/%s/%s?token=%s", ctx.Username, ctx.Reponame, ctx.Token) diff --git a/integrations/git_test.go b/integrations/git_test.go index ce5aee493d835..9bc7aae980b0f 100644 --- a/integrations/git_test.go +++ b/integrations/git_test.go @@ -39,17 +39,23 @@ func testGit(t *testing.T, u *url.URL) { u.Path = baseAPITestContext.GitPath() + forkedUserCtx := NewAPITestContext(t, "user4", "repo1") + t.Run("HTTP", func(t *testing.T) { PrintCurrentTest(t) + ensureAnonymousClone(t, u) httpContext := baseAPITestContext httpContext.Reponame = "repo-tmp-17" + forkedUserCtx.Reponame = httpContext.Reponame dstPath, err := ioutil.TempDir("", httpContext.Reponame) assert.NoError(t, err) defer os.RemoveAll(dstPath) - t.Run("CreateRepo", doAPICreateRepository(httpContext, false)) - ensureAnonymousClone(t, u) + t.Run("CreateRepoInDifferentUser", doAPICreateRepository(forkedUserCtx, false)) + t.Run("AddUserAsCollaborator", doAPIAddCollaborator(forkedUserCtx, httpContext.Username, models.AccessModeRead)) + + t.Run("ForkFromDifferentUser", doAPIForkRepository(httpContext, forkedUserCtx.Username)) u.Path = httpContext.GitPath() u.User = url.UserPassword(username, userPassword) @@ -62,12 +68,22 @@ func testGit(t *testing.T, u *url.URL) { mediaTest(t, &httpContext, little, big, littleLFS, bigLFS) t.Run("BranchProtectMerge", doBranchProtectPRMerge(&httpContext, dstPath)) + t.Run("MergeFork", func(t *testing.T) { + t.Run("CreatePRAndMerge", doMergeFork(httpContext, forkedUserCtx, "master", httpContext.Username+":master")) + rawTest(t, &forkedUserCtx, little, big, littleLFS, bigLFS) + mediaTest(t, &forkedUserCtx, little, big, littleLFS, bigLFS) + }) }) t.Run("SSH", func(t *testing.T) { PrintCurrentTest(t) sshContext := baseAPITestContext sshContext.Reponame = "repo-tmp-18" keyname := "my-testing-key" + forkedUserCtx.Reponame = sshContext.Reponame + t.Run("CreateRepoInDifferentUser", doAPICreateRepository(forkedUserCtx, false)) + t.Run("AddUserAsCollaborator", doAPIAddCollaborator(forkedUserCtx, sshContext.Username, models.AccessModeRead)) + t.Run("ForkFromDifferentUser", doAPIForkRepository(sshContext, forkedUserCtx.Username)) + //Setup key the user ssh key withKeyFile(t, keyname, func(keyFile string) { t.Run("CreateUserKey", doAPICreateUserKey(sshContext, "test-key", keyFile)) @@ -81,8 +97,6 @@ func testGit(t *testing.T, u *url.URL) { assert.NoError(t, err) defer os.RemoveAll(dstPath) - t.Run("CreateRepo", doAPICreateRepository(sshContext, false)) - t.Run("Clone", doGitClone(dstPath, sshURL)) little, big := standardCommitAndPushTest(t, dstPath) @@ -91,8 +105,12 @@ func testGit(t *testing.T, u *url.URL) { mediaTest(t, &sshContext, little, big, littleLFS, bigLFS) t.Run("BranchProtectMerge", doBranchProtectPRMerge(&sshContext, dstPath)) + t.Run("MergeFork", func(t *testing.T) { + t.Run("CreatePRAndMerge", doMergeFork(sshContext, forkedUserCtx, "master", sshContext.Username+":master")) + rawTest(t, &forkedUserCtx, little, big, littleLFS, bigLFS) + mediaTest(t, &forkedUserCtx, little, big, littleLFS, bigLFS) + }) }) - }) } @@ -341,3 +359,16 @@ func doProtectBranch(ctx APITestContext, branch string, userToWhitelist string) assert.EqualValues(t, "success%3DBranch%2Bprotection%2Bfor%2Bbranch%2B%2527"+url.QueryEscape(branch)+"%2527%2Bhas%2Bbeen%2Bupdated.", flashCookie.Value) } } + +func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) func(t *testing.T) { + return func(t *testing.T) { + var pr api.PullRequest + var err error + t.Run("CreatePullRequest", func(t *testing.T) { + pr, err = doAPICreatePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, baseBranch, headBranch)(t) + assert.NoError(t, err) + }) + t.Run("MergePR", doAPIMergePullRequest(baseCtx, baseCtx.Username, baseCtx.Reponame, pr.Index)) + + } +} From 90bfcd83289d49daa38d18ac600fd39cf86971ee Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 1 Jun 2019 12:31:35 +0100 Subject: [PATCH 07/12] Delete the repository before checking that the lfs objects are present --- integrations/git_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/integrations/git_test.go b/integrations/git_test.go index 9bc7aae980b0f..8578fb86d5d76 100644 --- a/integrations/git_test.go +++ b/integrations/git_test.go @@ -70,6 +70,7 @@ func testGit(t *testing.T, u *url.URL) { t.Run("BranchProtectMerge", doBranchProtectPRMerge(&httpContext, dstPath)) t.Run("MergeFork", func(t *testing.T) { t.Run("CreatePRAndMerge", doMergeFork(httpContext, forkedUserCtx, "master", httpContext.Username+":master")) + t.Run("DeleteRepository", doAPIDeleteRepository(httpContext)) rawTest(t, &forkedUserCtx, little, big, littleLFS, bigLFS) mediaTest(t, &forkedUserCtx, little, big, littleLFS, bigLFS) }) @@ -107,6 +108,7 @@ func testGit(t *testing.T, u *url.URL) { t.Run("BranchProtectMerge", doBranchProtectPRMerge(&sshContext, dstPath)) t.Run("MergeFork", func(t *testing.T) { t.Run("CreatePRAndMerge", doMergeFork(sshContext, forkedUserCtx, "master", sshContext.Username+":master")) + t.Run("DeleteRepository", doAPIDeleteRepository(sshContext)) rawTest(t, &forkedUserCtx, little, big, littleLFS, bigLFS) mediaTest(t, &forkedUserCtx, little, big, littleLFS, bigLFS) }) From cfe1bac6b734c69efcaff9505c5e2b02a247521e Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 14 Jun 2019 17:45:31 +0100 Subject: [PATCH 08/12] move the LFS code out of the merge file and into a separate file --- modules/merge/lfs.go | 226 +++++++++++++++++++++++++++++++++++++++++ modules/merge/merge.go | 209 ------------------------------------- 2 files changed, 226 insertions(+), 209 deletions(-) create mode 100644 modules/merge/lfs.go diff --git a/modules/merge/lfs.go b/modules/merge/lfs.go new file mode 100644 index 0000000000000..459d2a4b4b5dc --- /dev/null +++ b/modules/merge/lfs.go @@ -0,0 +1,226 @@ +// 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. + +package merge + +import ( + "bufio" + "bytes" + "fmt" + "io" + "strconv" + "strings" + "sync" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/lfs" + "code.gitea.io/gitea/modules/log" +) + +// LFSPush pushes lfs objects referred to in new commits in the head repository from the base repository +func LFSPush(tmpBasePath, mergeHeadSHA, mergeBaseSHA string, pr *models.PullRequest) error { + // Now we have to implement git lfs push + // git rev-list --objects --filter=blob:limit=1k HEAD --not base + // pass blob shas in to git cat-file --batch-check (possibly unnecessary) + // ensure only blobs and <=1k size then pass in to git cat-file --batch + // to read each sha and check each as a pointer + // Then if they are lfs -> add them to the baseRepo + revListReader, revListWriter := io.Pipe() + shasToCheckReader, shasToCheckWriter := io.Pipe() + catFileCheckReader, catFileCheckWriter := io.Pipe() + shasToBatchReader, shasToBatchWriter := io.Pipe() + catFileBatchReader, catFileBatchWriter := io.Pipe() + errChan := make(chan error, 1) + wg := sync.WaitGroup{} + wg.Add(6) + // Create the go-routines in reverse order. + + // 6. Take the output of cat-file --batch and check if each file in turn + // to see if they're pointers to files in the LFS store associated with + // the head repo and add them to the base repo if so + go readCatFileBatch(catFileBatchReader, &wg, pr) + + // 5. Take the shas of the blobs and batch read them + go doCatFileBatch(shasToBatchReader, catFileBatchWriter, &wg, tmpBasePath) + + // 4. From the provided objects restrict to blobs <=1k + go readCatFileBatchCheck(catFileCheckReader, shasToBatchWriter, &wg) + + // 3. Run batch-check on the objects retrieved from rev-list + go doCatFileBatchCheck(shasToCheckReader, catFileCheckWriter, &wg, tmpBasePath) + + // 2. Check each object retrieved rejecting those without names as they will be commits or trees + go readRevListObjects(revListReader, shasToCheckWriter, &wg) + + // 1. Run rev-list objects from mergeHead to mergeBase + go doRevListObjects(revListWriter, &wg, tmpBasePath, mergeHeadSHA, mergeBaseSHA, errChan) + + wg.Wait() + select { + case err, has := <-errChan: + if has { + return err + } + default: + } + return nil +} + +func doRevListObjects(revListWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath, headSHA, baseSHA string, errChan chan<- error) { + defer wg.Done() + defer revListWriter.Close() + stderr := new(bytes.Buffer) + var errbuf strings.Builder + cmd := git.NewCommand("rev-list", "--objects", headSHA, "--not", baseSHA) + if err := cmd.RunInDirPipeline(tmpBasePath, revListWriter, stderr); err != nil { + log.Error("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + errChan <- fmt.Errorf("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String()) + } +} + +func readRevListObjects(revListReader *io.PipeReader, shasToCheckWriter *io.PipeWriter, wg *sync.WaitGroup) { + defer wg.Done() + defer revListReader.Close() + defer shasToCheckWriter.Close() + scanner := bufio.NewScanner(revListReader) + for scanner.Scan() { + line := scanner.Text() + if len(line) == 0 { + continue + } + fields := strings.Split(line, " ") + if len(fields) < 2 || len(fields[1]) == 0 { + continue + } + toWrite := []byte(fields[0] + "\n") + for len(toWrite) > 0 { + n, err := shasToCheckWriter.Write(toWrite) + if err != nil { + _ = revListReader.CloseWithError(err) + break + } + toWrite = toWrite[n:] + } + } + _ = shasToCheckWriter.CloseWithError(scanner.Err()) +} + +func doCatFileBatchCheck(shasToCheckReader *io.PipeReader, catFileCheckWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) { + defer wg.Done() + defer shasToCheckReader.Close() + defer catFileCheckWriter.Close() + + stderr := new(bytes.Buffer) + var errbuf strings.Builder + cmd := git.NewCommand("cat-file", "--batch-check") + if err := cmd.RunInDirFullPipeline(tmpBasePath, catFileCheckWriter, stderr, shasToCheckReader); err != nil { + _ = catFileCheckWriter.CloseWithError(fmt.Errorf("git cat-file --batch-check [%s]: %v - %s", tmpBasePath, err, errbuf.String())) + } +} + +func readCatFileBatchCheck(catFileCheckReader *io.PipeReader, shasToBatchWriter *io.PipeWriter, wg *sync.WaitGroup) { + defer wg.Done() + defer catFileCheckReader.Close() + + scanner := bufio.NewScanner(catFileCheckReader) + defer func() { + _ = shasToBatchWriter.CloseWithError(scanner.Err()) + }() + for scanner.Scan() { + line := scanner.Text() + if len(line) == 0 { + continue + } + fields := strings.Split(line, " ") + if len(fields) < 3 || fields[1] != "blob" { + continue + } + size, _ := strconv.Atoi(string(fields[2])) + if size > 1024 { + continue + } + toWrite := []byte(fields[0] + "\n") + for len(toWrite) > 0 { + n, err := shasToBatchWriter.Write(toWrite) + if err != nil { + _ = catFileCheckReader.CloseWithError(err) + break + } + toWrite = toWrite[n:] + } + } +} + +func doCatFileBatch(shasToBatchReader *io.PipeReader, catFileBatchWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) { + defer wg.Done() + defer shasToBatchReader.Close() + defer catFileBatchWriter.Close() + + stderr := new(bytes.Buffer) + var errbuf strings.Builder + if err := git.NewCommand("cat-file", "--batch").RunInDirFullPipeline(tmpBasePath, catFileBatchWriter, stderr, shasToBatchReader); err != nil { + _ = shasToBatchReader.CloseWithError(fmt.Errorf("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String())) + } +} + +func readCatFileBatch(catFileBatchReader *io.PipeReader, wg *sync.WaitGroup, pr *models.PullRequest) { + defer wg.Done() + defer catFileBatchReader.Close() + + bufferedReader := bufio.NewReader(catFileBatchReader) + buf := make([]byte, 1025) + for { + // File descriptor line: sha + _, err := bufferedReader.ReadString(' ') + if err != nil { + _ = catFileBatchReader.CloseWithError(err) + break + } + // Throw away the blob + if _, err := bufferedReader.ReadString(' '); err != nil { + _ = catFileBatchReader.CloseWithError(err) + break + } + sizeStr, err := bufferedReader.ReadString('\n') + if err != nil { + _ = catFileBatchReader.CloseWithError(err) + break + } + size, err := strconv.Atoi(sizeStr[:len(sizeStr)-1]) + if err != nil { + _ = catFileBatchReader.CloseWithError(err) + break + } + pointerBuf := buf[:size+1] + if _, err := io.ReadFull(bufferedReader, pointerBuf); err != nil { + _ = catFileBatchReader.CloseWithError(err) + break + } + pointerBuf = pointerBuf[:size] + // Now we need to check if the pointerBuf is an LFS pointer + pointer := lfs.IsPointerFile(&pointerBuf) + if pointer == nil { + continue + } + // Then we need to check that this pointer is in the db + if _, err := pr.HeadRepo.GetLFSMetaObjectByOid(pointer.Oid); err != nil { + if err == models.ErrLFSObjectNotExist { + log.Warn("During merge of: %d in %-v, there is a pointer to LFS Oid: %s which although present in the LFS store is not associated with the head repo %-v", pr.Index, pr.BaseRepo, pointer.Oid, pr.HeadRepo) + continue + } + _ = catFileBatchReader.CloseWithError(err) + break + } + // OK we have a pointer that is associated with the head repo + // and is actually a file in the LFS + // Therefore it should be associated with the base repo + pointer.RepositoryID = pr.BaseRepoID + if _, err := models.NewLFSMetaObject(pointer); err != nil { + _ = catFileBatchReader.CloseWithError(err) + break + } + } +} diff --git a/modules/merge/merge.go b/modules/merge/merge.go index bc4b8168f328d..0fe75d4c8bf31 100644 --- a/modules/merge/merge.go +++ b/modules/merge/merge.go @@ -9,19 +9,15 @@ import ( "bufio" "bytes" "fmt" - "io" "io/ioutil" "os" "path" "path/filepath" - "strconv" "strings" - "sync" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" - "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -341,208 +337,3 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { } return out.String(), nil } - -// LFSPush pushes lfs objects referred to in new commits in the head repository from the base repository -func LFSPush(tmpBasePath, mergeHeadSHA, mergeBaseSHA string, pr *models.PullRequest) error { - // Now we have to implement git lfs push - // git rev-list --objects --filter=blob:limit=1k HEAD --not base - // pass blob shas in to git cat-file --batch-check (possibly unnecessary) - // ensure only blobs and <=1k size then pass in to git cat-file --batch - // to read each sha and check each as a pointer - // Then if they are lfs -> add them to the baseRepo - revListReader, revListWriter := io.Pipe() - shasToCheckReader, shasToCheckWriter := io.Pipe() - catFileCheckReader, catFileCheckWriter := io.Pipe() - shasToBatchReader, shasToBatchWriter := io.Pipe() - catFileBatchReader, catFileBatchWriter := io.Pipe() - errChan := make(chan error, 1) - wg := sync.WaitGroup{} - wg.Add(6) - // Create the go-routines in reverse order. - - // 6. Take the output of cat-file --batch and check if each file in turn - // to see if they're pointers to files in the LFS store associated with - // the head repo and add them to the base repo if so - go readCatFileBatch(catFileBatchReader, &wg, pr) - - // 5. Take the shas of the blobs and batch read them - go doCatFileBatch(shasToBatchReader, catFileBatchWriter, &wg, tmpBasePath) - - // 4. From the provided objects restrict to blobs <=1k - go readCatFileBatchCheck(catFileCheckReader, shasToBatchWriter, &wg) - - // 3. Run batch-check on the objects retrieved from rev-list - go doCatFileBatchCheck(shasToCheckReader, catFileCheckWriter, &wg, tmpBasePath) - - // 2. Check each object retrieved rejecting those without names as they will be commits or trees - go readRevListObjects(revListReader, shasToCheckWriter, &wg) - - // 1. Run rev-list objects from mergeHead to mergeBase - go doRevListObjects(revListWriter, &wg, tmpBasePath, mergeHeadSHA, mergeBaseSHA, errChan) - - wg.Wait() - select { - case err, has := <-errChan: - if has { - return err - } - default: - } - return nil -} - -func doRevListObjects(revListWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath, headSHA, baseSHA string, errChan chan<- error) { - defer wg.Done() - defer revListWriter.Close() - stderr := new(bytes.Buffer) - var errbuf strings.Builder - cmd := git.NewCommand("rev-list", "--objects", headSHA, "--not", baseSHA) - if err := cmd.RunInDirPipeline(tmpBasePath, revListWriter, stderr); err != nil { - log.Error("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String()) - errChan <- fmt.Errorf("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String()) - } -} - -func readRevListObjects(revListReader *io.PipeReader, shasToCheckWriter *io.PipeWriter, wg *sync.WaitGroup) { - defer wg.Done() - defer revListReader.Close() - defer shasToCheckWriter.Close() - scanner := bufio.NewScanner(revListReader) - for scanner.Scan() { - line := scanner.Text() - if len(line) == 0 { - continue - } - fields := strings.Split(line, " ") - if len(fields) < 2 || len(fields[1]) == 0 { - continue - } - toWrite := []byte(fields[0] + "\n") - for len(toWrite) > 0 { - n, err := shasToCheckWriter.Write(toWrite) - if err != nil { - _ = revListReader.CloseWithError(err) - break - } - toWrite = toWrite[n:] - } - } - _ = shasToCheckWriter.CloseWithError(scanner.Err()) -} - -func doCatFileBatchCheck(shasToCheckReader *io.PipeReader, catFileCheckWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) { - defer wg.Done() - defer shasToCheckReader.Close() - defer catFileCheckWriter.Close() - - stderr := new(bytes.Buffer) - var errbuf strings.Builder - cmd := git.NewCommand("cat-file", "--batch-check") - if err := cmd.RunInDirFullPipeline(tmpBasePath, catFileCheckWriter, stderr, shasToCheckReader); err != nil { - _ = catFileCheckWriter.CloseWithError(fmt.Errorf("git cat-file --batch-check [%s]: %v - %s", tmpBasePath, err, errbuf.String())) - } -} - -func readCatFileBatchCheck(catFileCheckReader *io.PipeReader, shasToBatchWriter *io.PipeWriter, wg *sync.WaitGroup) { - defer wg.Done() - defer catFileCheckReader.Close() - - scanner := bufio.NewScanner(catFileCheckReader) - defer func() { - _ = shasToBatchWriter.CloseWithError(scanner.Err()) - }() - for scanner.Scan() { - line := scanner.Text() - if len(line) == 0 { - continue - } - fields := strings.Split(line, " ") - if len(fields) < 3 || fields[1] != "blob" { - continue - } - size, _ := strconv.Atoi(string(fields[2])) - if size > 1024 { - continue - } - toWrite := []byte(fields[0] + "\n") - for len(toWrite) > 0 { - n, err := shasToBatchWriter.Write(toWrite) - if err != nil { - _ = catFileCheckReader.CloseWithError(err) - break - } - toWrite = toWrite[n:] - } - } -} - -func doCatFileBatch(shasToBatchReader *io.PipeReader, catFileBatchWriter *io.PipeWriter, wg *sync.WaitGroup, tmpBasePath string) { - defer wg.Done() - defer shasToBatchReader.Close() - defer catFileBatchWriter.Close() - - stderr := new(bytes.Buffer) - var errbuf strings.Builder - if err := git.NewCommand("cat-file", "--batch").RunInDirFullPipeline(tmpBasePath, catFileBatchWriter, stderr, shasToBatchReader); err != nil { - _ = shasToBatchReader.CloseWithError(fmt.Errorf("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String())) - } -} - -func readCatFileBatch(catFileBatchReader *io.PipeReader, wg *sync.WaitGroup, pr *models.PullRequest) { - defer wg.Done() - defer catFileBatchReader.Close() - - bufferedReader := bufio.NewReader(catFileBatchReader) - buf := make([]byte, 1025) - for { - // File descriptor line: sha - _, err := bufferedReader.ReadString(' ') - if err != nil { - _ = catFileBatchReader.CloseWithError(err) - break - } - // Throw away the blob - if _, err := bufferedReader.ReadString(' '); err != nil { - _ = catFileBatchReader.CloseWithError(err) - break - } - sizeStr, err := bufferedReader.ReadString('\n') - if err != nil { - _ = catFileBatchReader.CloseWithError(err) - break - } - size, err := strconv.Atoi(sizeStr[:len(sizeStr)-1]) - if err != nil { - _ = catFileBatchReader.CloseWithError(err) - break - } - pointerBuf := buf[:size+1] - if _, err := io.ReadFull(bufferedReader, pointerBuf); err != nil { - _ = catFileBatchReader.CloseWithError(err) - break - } - pointerBuf = pointerBuf[:size] - // Now we need to check if the pointerBuf is an LFS pointer - pointer := lfs.IsPointerFile(&pointerBuf) - if pointer == nil { - continue - } - // Then we need to check that this pointer is in the db - if _, err := pr.HeadRepo.GetLFSMetaObjectByOid(pointer.Oid); err != nil { - if err == models.ErrLFSObjectNotExist { - log.Warn("During merge of: %d in %-v, there is a pointer to LFS Oid: %s which although present in the LFS store is not associated with the head repo %-v", pr.Index, pr.BaseRepo, pointer.Oid, pr.HeadRepo) - continue - } - _ = catFileBatchReader.CloseWithError(err) - break - } - // OK we have a pointer that is associated with the head repo - // and is actually a file in the LFS - // Therefore it should be associated with the base repo - pointer.RepositoryID = pr.BaseRepoID - if _, err := models.NewLFSMetaObject(pointer); err != nil { - _ = catFileBatchReader.CloseWithError(err) - break - } - } -} From a6dd916d8624533297bac83ea3d434999076bc4c Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 15 Jun 2019 17:48:27 +0100 Subject: [PATCH 09/12] Update modules/merge/lfs.go --- modules/merge/lfs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/merge/lfs.go b/modules/merge/lfs.go index 459d2a4b4b5dc..e73b187f80d35 100644 --- a/modules/merge/lfs.go +++ b/modules/merge/lfs.go @@ -171,7 +171,7 @@ func readCatFileBatch(catFileBatchReader *io.PipeReader, wg *sync.WaitGroup, pr defer catFileBatchReader.Close() bufferedReader := bufio.NewReader(catFileBatchReader) - buf := make([]byte, 1025) + buf := make([]byte, 1024) for { // File descriptor line: sha _, err := bufferedReader.ReadString(' ') From 4264c30044666ea6179c755de1045e74a5114137 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 15 Jun 2019 18:23:20 +0100 Subject: [PATCH 10/12] Revert buf length change We need to read the file plus the terminal newline that cat-file sends - hence 1024 + 1. --- modules/merge/lfs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/merge/lfs.go b/modules/merge/lfs.go index e73b187f80d35..459d2a4b4b5dc 100644 --- a/modules/merge/lfs.go +++ b/modules/merge/lfs.go @@ -171,7 +171,7 @@ func readCatFileBatch(catFileBatchReader *io.PipeReader, wg *sync.WaitGroup, pr defer catFileBatchReader.Close() bufferedReader := bufio.NewReader(catFileBatchReader) - buf := make([]byte, 1024) + buf := make([]byte, 1025) for { // File descriptor line: sha _, err := bufferedReader.ReadString(' ') From 06c601e1b67d1f4516d5a8c913c3cfa2f0231f1c Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 17 Jun 2019 08:40:46 +0100 Subject: [PATCH 11/12] move modules/merge to modules/pull as per @lunny --- modules/{merge => pull}/lfs.go | 2 +- modules/{merge => pull}/merge.go | 2 +- routers/api/v1/repo/pull.go | 2 +- routers/repo/pull.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename modules/{merge => pull}/lfs.go (99%) rename modules/{merge => pull}/merge.go (99%) diff --git a/modules/merge/lfs.go b/modules/pull/lfs.go similarity index 99% rename from modules/merge/lfs.go rename to modules/pull/lfs.go index 459d2a4b4b5dc..77890667d63d1 100644 --- a/modules/merge/lfs.go +++ b/modules/pull/lfs.go @@ -3,7 +3,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package merge +package pull import ( "bufio" diff --git a/modules/merge/merge.go b/modules/pull/merge.go similarity index 99% rename from modules/merge/merge.go rename to modules/pull/merge.go index 0fe75d4c8bf31..e12ede81d630d 100644 --- a/modules/merge/merge.go +++ b/modules/pull/merge.go @@ -3,7 +3,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package merge +package pull import ( "bufio" diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 24a8ebc84a173..25135edbc4044 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -596,7 +596,7 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) { message += "\n\n" + form.MergeMessageField } - if err := merge.Merge(pr, ctx.User, ctx.Repo.GitRepo, models.MergeStyle(form.Do), message); err != nil { + if err := pull.Merge(pr, ctx.User, ctx.Repo.GitRepo, models.MergeStyle(form.Do), message); err != nil { if models.IsErrInvalidMergeStyle(err) { ctx.Status(405) return diff --git a/routers/repo/pull.go b/routers/repo/pull.go index ebb56195a2fbe..0a3a3e8b0154a 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -597,7 +597,7 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) { return } - if err = merge.Merge(pr, ctx.User, ctx.Repo.GitRepo, models.MergeStyle(form.Do), message); err != nil { + if err = pull.Merge(pr, ctx.User, ctx.Repo.GitRepo, models.MergeStyle(form.Do), message); err != nil { if models.IsErrInvalidMergeStyle(err) { ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option")) ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index)) From f396505b23f7b7a5c7cee6d4b17980665f49828e Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 17 Jun 2019 08:46:02 +0100 Subject: [PATCH 12/12] missed updates to imports --- routers/api/v1/repo/pull.go | 2 +- routers/repo/pull.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 25135edbc4044..d99c9a00c99ae 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -14,8 +14,8 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/merge" "code.gitea.io/gitea/modules/notification" + "code.gitea.io/gitea/modules/pull" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" ) diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 0a3a3e8b0154a..36b0d047b1019 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -19,8 +19,8 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/merge" "code.gitea.io/gitea/modules/notification" + "code.gitea.io/gitea/modules/pull" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util"