From d4fe971f4639671d3f1af360bbe19fd0735fc295 Mon Sep 17 00:00:00 2001 From: Mura Li Date: Tue, 11 Sep 2018 15:02:15 +0800 Subject: [PATCH 01/10] Optimize pulls merging By utilizing `git clone -s --no-checkout` rather than cloning the whole repo. --- models/pull.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/models/pull.go b/models/pull.go index 8b4961d8b67c..259f4e44ec25 100644 --- a/models/pull.go +++ b/models/pull.go @@ -371,17 +371,10 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle var stderr string if _, stderr, err = process.GetManager().ExecTimeout(5*time.Minute, fmt.Sprintf("PullRequest.Merge (git clone): %s", tmpBasePath), - "git", "clone", baseGitRepo.Path, tmpBasePath); err != nil { + "git", "clone", "-s", "--no-checkout", "-b", pr.BaseBranch, baseGitRepo.Path, tmpBasePath); err != nil { return fmt.Errorf("git clone: %s", stderr) } - // Check out base branch. - if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, - fmt.Sprintf("PullRequest.Merge (git checkout): %s", tmpBasePath), - "git", "checkout", pr.BaseBranch); err != nil { - return fmt.Errorf("git checkout: %s", stderr) - } - // Add head repo remote. if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, fmt.Sprintf("PullRequest.Merge (git remote add): %s", tmpBasePath), @@ -389,13 +382,21 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle return fmt.Errorf("git remote add [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr) } - // Merge commits. + // Read base branch index + if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, + fmt.Sprintf("PullRequest.Merge (git read-tree): %s", tmpBasePath), + "git", "read-tree", "HEAD"); err != nil { + return fmt.Errorf("git read-tree HEAD: %s", stderr) + } + + // Fetch head branch if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, fmt.Sprintf("PullRequest.Merge (git fetch): %s", tmpBasePath), "git", "fetch", "head_repo"); err != nil { return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr) } + // Merge commits. switch mergeStyle { case MergeStyleMerge: if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, From c2f7a287291ec1d420e7a9ff27e0e494b106cd69 Mon Sep 17 00:00:00 2001 From: Mura Li Date: Mon, 22 Oct 2018 14:11:50 +0800 Subject: [PATCH 02/10] Use sparse-checkout to speedup pulls merge --- models/pull.go | 88 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 81 insertions(+), 7 deletions(-) diff --git a/models/pull.go b/models/pull.go index 259f4e44ec25..b25d75f19659 100644 --- a/models/pull.go +++ b/models/pull.go @@ -5,11 +5,14 @@ package models import ( + "bufio" + "bytes" "fmt" "io/ioutil" "os" "path" "path/filepath" + "sort" "strconv" "strings" "time" @@ -328,6 +331,59 @@ func (pr *PullRequest) CheckUserAllowedToMerge(doer *User) (err error) { return nil } +func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { + var buf bytes.Buffer + + getDiffTreeFromBranch := func(repoPath, branch string) (string, error) { + var stdout, stderr string + // Compute the diff-tree for sparse-checkout + stdout, stderr, err := process.GetManager().ExecDir(-1, repoPath, + fmt.Sprintf("PullRequest.Merge (git diff-tree): %s", repoPath), + "git", "diff-tree", "--no-commit-id", "--name-only", "-r", branch) + if err != nil { + return "", fmt.Errorf("git diff-tree [%s/%s]: %s", repoPath, branch, stderr) + } + return stdout, nil + } + + listBase, err := getDiffTreeFromBranch(repoPath, baseBranch) + if err != nil { + return "", err + } + if _, err := buf.WriteString(listBase); err != nil { + return "", err + } + listHead, err := getDiffTreeFromBranch(repoPath, baseBranch) + if err != nil { + return "", err + } + if _, err := buf.WriteString(listHead); err != nil { + return "", err + } + + //.Deduplicate + seen := make(map[string]struct{}) + scanner := bufio.NewScanner(&buf) + for scanner.Scan() { + seen[scanner.Text()] = struct{}{} + } + + // Sorting for eaiser debugging + sorted := []string{} + for k := range seen { + sorted = append(sorted, k) + } + sort.Strings(sorted) + + var result strings.Builder + for _, v := range sorted { + if _, err := result.WriteString(v); err != nil { + return "", err + } + } + return result.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) { @@ -382,13 +438,6 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle return fmt.Errorf("git remote add [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr) } - // Read base branch index - if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, - fmt.Sprintf("PullRequest.Merge (git read-tree): %s", tmpBasePath), - "git", "read-tree", "HEAD"); err != nil { - return fmt.Errorf("git read-tree HEAD: %s", stderr) - } - // Fetch head branch if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, fmt.Sprintf("PullRequest.Merge (git fetch): %s", tmpBasePath), @@ -396,6 +445,31 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr) } + // Enable sparse-checkout + sparseCheckoutList, err := getDiffTree(tmpBasePath, pr.BaseBranch, pr.HeadBranch) + if err != nil { + return fmt.Errorf("getDiffTree: %v", err) + } + log.Debug("sparseCheckoutList:\n", sparseCheckoutList) + + sparseCheckoutListPath := filepath.Join(tmpBasePath, ".git", "info", "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 _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, + fmt.Sprintf("PullRequest.Merge (git config): %s", tmpBasePath), + "git", "config", "--local", "core.sparseCheckout", "true"); err != nil { + return fmt.Errorf("git config [core.sparsecheckout -> true]: %v", stderr) + } + + // Read base branch index + if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, + fmt.Sprintf("PullRequest.Merge (git read-tree): %s", tmpBasePath), + "git", "read-tree", "HEAD"); err != nil { + return fmt.Errorf("git read-tree HEAD: %s", stderr) + } + // Merge commits. switch mergeStyle { case MergeStyleMerge: From 03c126d72bbef989c449bb1eb0d145170e5b8179 Mon Sep 17 00:00:00 2001 From: Mura Li Date: Fri, 21 Dec 2018 11:02:06 +0800 Subject: [PATCH 03/10] Use bytes.Buffer instead of strings.Builder for backward compatibility --- models/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/pull.go b/models/pull.go index b25d75f19659..5e44b54dc394 100644 --- a/models/pull.go +++ b/models/pull.go @@ -375,7 +375,7 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { } sort.Strings(sorted) - var result strings.Builder + var result bytes.Buffer for _, v := range sorted { if _, err := result.WriteString(v); err != nil { return "", err From c7a509fd9d7a62d39a77a6f92bd39c20c60ac6d3 Mon Sep 17 00:00:00 2001 From: Mura Li Date: Fri, 21 Dec 2018 13:56:36 +0800 Subject: [PATCH 04/10] Fix empty diff-tree output for repos with only the initial commit --- models/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/pull.go b/models/pull.go index 5e44b54dc394..a1f8680741ec 100644 --- a/models/pull.go +++ b/models/pull.go @@ -339,7 +339,7 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { // Compute the diff-tree for sparse-checkout stdout, stderr, err := process.GetManager().ExecDir(-1, repoPath, fmt.Sprintf("PullRequest.Merge (git diff-tree): %s", repoPath), - "git", "diff-tree", "--no-commit-id", "--name-only", "-r", branch) + "git", "diff-tree", "--no-commit-id", "--name-only", "-r", "--root", branch) if err != nil { return "", fmt.Errorf("git diff-tree [%s/%s]: %s", repoPath, branch, stderr) } From 5a3c10b3c720656b26fcb8d9956ad84075517da6 Mon Sep 17 00:00:00 2001 From: Mura Li Date: Fri, 21 Dec 2018 13:57:34 +0800 Subject: [PATCH 05/10] Fix missing argument for the format string --- models/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/pull.go b/models/pull.go index a1f8680741ec..e7ab9dbef550 100644 --- a/models/pull.go +++ b/models/pull.go @@ -450,7 +450,7 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle if err != nil { return fmt.Errorf("getDiffTree: %v", err) } - log.Debug("sparseCheckoutList:\n", sparseCheckoutList) + log.Debug("sparseCheckoutList: %s\n", sparseCheckoutList) sparseCheckoutListPath := filepath.Join(tmpBasePath, ".git", "info", "sparse-checkout") if err := ioutil.WriteFile(sparseCheckoutListPath, []byte(sparseCheckoutList), 0600); err != nil { From 23d1f557633ebdd20197e3c107c43771ce656d7b Mon Sep 17 00:00:00 2001 From: Mura Li Date: Fri, 21 Dec 2018 15:13:29 +0800 Subject: [PATCH 06/10] Rework diff-tree-list generation --- models/pull.go | 66 ++++++++++++++------------------------------------ 1 file changed, 18 insertions(+), 48 deletions(-) diff --git a/models/pull.go b/models/pull.go index e7ab9dbef550..755c24b62882 100644 --- a/models/pull.go +++ b/models/pull.go @@ -5,14 +5,11 @@ package models import ( - "bufio" - "bytes" "fmt" "io/ioutil" "os" "path" "path/filepath" - "sort" "strconv" "strings" "time" @@ -332,56 +329,24 @@ func (pr *PullRequest) CheckUserAllowedToMerge(doer *User) (err error) { } func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { - var buf bytes.Buffer - - getDiffTreeFromBranch := func(repoPath, branch string) (string, error) { + getDiffTreeFromBranch := func(repoPath, baseBranch, headBranch string) (string, error) { var stdout, stderr string // 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") stdout, stderr, err := process.GetManager().ExecDir(-1, repoPath, fmt.Sprintf("PullRequest.Merge (git diff-tree): %s", repoPath), - "git", "diff-tree", "--no-commit-id", "--name-only", "-r", "--root", branch) + "git", "diff-tree", "--no-commit-id", "--name-only", "-r", "--root", baseBranch, headBranch) if err != nil { - return "", fmt.Errorf("git diff-tree [%s/%s]: %s", repoPath, branch, stderr) + return "", fmt.Errorf("git diff-tree [%s base:%s head:%s]: %s", repoPath, baseBranch, headBranch, stderr) } return stdout, nil } - listBase, err := getDiffTreeFromBranch(repoPath, baseBranch) + list, err := getDiffTreeFromBranch(repoPath, baseBranch, headBranch) if err != nil { return "", err } - if _, err := buf.WriteString(listBase); err != nil { - return "", err - } - listHead, err := getDiffTreeFromBranch(repoPath, baseBranch) - if err != nil { - return "", err - } - if _, err := buf.WriteString(listHead); err != nil { - return "", err - } - - //.Deduplicate - seen := make(map[string]struct{}) - scanner := bufio.NewScanner(&buf) - for scanner.Scan() { - seen[scanner.Text()] = struct{}{} - } - - // Sorting for eaiser debugging - sorted := []string{} - for k := range seen { - sorted = append(sorted, k) - } - sort.Strings(sorted) - - var result bytes.Buffer - for _, v := range sorted { - if _, err := result.WriteString(v); err != nil { - return "", err - } - } - return result.String(), nil + return list, nil } // Merge merges pull request to base repository. @@ -431,22 +396,27 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle return fmt.Errorf("git clone: %s", stderr) } + remoteRepoName := "head_repo" + // Add head repo remote. if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, fmt.Sprintf("PullRequest.Merge (git remote add): %s", tmpBasePath), - "git", "remote", "add", "head_repo", headRepoPath); err != nil { + "git", "remote", "add", remoteRepoName, headRepoPath); err != nil { return fmt.Errorf("git remote add [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr) } // Fetch head branch if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, fmt.Sprintf("PullRequest.Merge (git fetch): %s", tmpBasePath), - "git", "fetch", "head_repo"); err != nil { + "git", "fetch", remoteRepoName); err != nil { return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr) } + trackingBranch := path.Join(remoteRepoName, pr.HeadBranch) + stagingBranch := fmt.Sprintf("%s_%s", remoteRepoName, pr.HeadBranch) + // Enable sparse-checkout - sparseCheckoutList, err := getDiffTree(tmpBasePath, pr.BaseBranch, pr.HeadBranch) + sparseCheckoutList, err := getDiffTree(tmpBasePath, pr.BaseBranch, trackingBranch) if err != nil { return fmt.Errorf("getDiffTree: %v", err) } @@ -475,7 +445,7 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle case MergeStyleMerge: if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, fmt.Sprintf("PullRequest.Merge (git merge --no-ff --no-commit): %s", tmpBasePath), - "git", "merge", "--no-ff", "--no-commit", "head_repo/"+pr.HeadBranch); err != nil { + "git", "merge", "--no-ff", "--no-commit", trackingBranch); err != nil { return fmt.Errorf("git merge --no-ff --no-commit [%s]: %v - %s", tmpBasePath, err, stderr) } @@ -490,7 +460,7 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle // Checkout head branch if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, fmt.Sprintf("PullRequest.Merge (git checkout): %s", tmpBasePath), - "git", "checkout", "-b", "head_repo_"+pr.HeadBranch, "head_repo/"+pr.HeadBranch); err != nil { + "git", "checkout", "-b", stagingBranch, trackingBranch); err != nil { return fmt.Errorf("git checkout: %s", stderr) } // Rebase before merging @@ -508,7 +478,7 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle // Merge fast forward if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, fmt.Sprintf("PullRequest.Merge (git rebase): %s", tmpBasePath), - "git", "merge", "--ff-only", "-q", "head_repo_"+pr.HeadBranch); err != nil { + "git", "merge", "--ff-only", "-q", stagingBranch); err != nil { return fmt.Errorf("git merge --ff-only [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr) } case MergeStyleRebaseMerge: @@ -550,7 +520,7 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle // Merge with squash if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, fmt.Sprintf("PullRequest.Merge (git squash): %s", tmpBasePath), - "git", "merge", "-q", "--squash", "head_repo/"+pr.HeadBranch); err != nil { + "git", "merge", "-q", "--squash", trackingBranch); err != nil { return fmt.Errorf("git merge --squash [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr) } sig := pr.Issue.Poster.NewGitSig() From f67dd1b2960cd380f2c50b7fef175383b860b5cc Mon Sep 17 00:00:00 2001 From: Mura Li Date: Sat, 22 Dec 2018 09:53:32 +0800 Subject: [PATCH 07/10] Remove logging code --- models/pull.go | 1 - 1 file changed, 1 deletion(-) diff --git a/models/pull.go b/models/pull.go index 755c24b62882..6e53bd0878d9 100644 --- a/models/pull.go +++ b/models/pull.go @@ -420,7 +420,6 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle if err != nil { return fmt.Errorf("getDiffTree: %v", err) } - log.Debug("sparseCheckoutList: %s\n", sparseCheckoutList) sparseCheckoutListPath := filepath.Join(tmpBasePath, ".git", "info", "sparse-checkout") if err := ioutil.WriteFile(sparseCheckoutListPath, []byte(sparseCheckoutList), 0600); err != nil { From 1b9d6998bf1fb0f3f6f0a15e9b71bbf975008b06 Mon Sep 17 00:00:00 2001 From: Mura Li Date: Tue, 25 Dec 2018 11:40:35 +0800 Subject: [PATCH 08/10] File list for sparse-checkout must be prefix with / Otherwise, they would match all files with the same name under subdirectories. --- models/pull.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/models/pull.go b/models/pull.go index 6e53bd0878d9..f9024eadce8e 100644 --- a/models/pull.go +++ b/models/pull.go @@ -5,6 +5,8 @@ package models import ( + "bufio" + "bytes" "fmt" "io/ioutil" "os" @@ -346,7 +348,14 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) { if err != nil { return "", err } - return list, nil + + // 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. From 3c6b43c9c99883bb484291908c12430d0575a1ed Mon Sep 17 00:00:00 2001 From: Mura Li Date: Fri, 28 Dec 2018 20:02:15 +0800 Subject: [PATCH 09/10] Update onto the rebased head --- models/pull.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/pull.go b/models/pull.go index f9024eadce8e..39905ed1fe39 100644 --- a/models/pull.go +++ b/models/pull.go @@ -493,7 +493,7 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle // Checkout head branch if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, fmt.Sprintf("PullRequest.Merge (git checkout): %s", tmpBasePath), - "git", "checkout", "-b", "head_repo_"+pr.HeadBranch, "head_repo/"+pr.HeadBranch); err != nil { + "git", "checkout", "-b", stagingBranch, trackingBranch); err != nil { return fmt.Errorf("git checkout: %s", stderr) } // Rebase before merging @@ -511,7 +511,7 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle // Prepare merge with commit if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, fmt.Sprintf("PullRequest.Merge (git merge): %s", tmpBasePath), - "git", "merge", "--no-ff", "--no-commit", "-q", "head_repo_"+pr.HeadBranch); err != nil { + "git", "merge", "--no-ff", "--no-commit", "-q", stagingBranch); err != nil { return fmt.Errorf("git merge --no-ff [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr) } From 70dc42c5d5bbacb65c31f8d79e846565a98ce05e Mon Sep 17 00:00:00 2001 From: Mura Li Date: Sat, 29 Dec 2018 11:53:27 +0800 Subject: [PATCH 10/10] Use referecen repo to avoid fetching objects --- models/pull.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/models/pull.go b/models/pull.go index 39905ed1fe39..e1a69ad2fcdc 100644 --- a/models/pull.go +++ b/models/pull.go @@ -408,6 +408,23 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle 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) + } if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath, fmt.Sprintf("PullRequest.Merge (git remote add): %s", tmpBasePath), "git", "remote", "add", remoteRepoName, headRepoPath); err != nil {