-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor and tidy-up the merge/update branch code #22568
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
1217546
Refactor and tidy-up the merge/update branch code
zeripath 6874c95
oops
zeripath 8a7b323
trim the output from merge-base
zeripath 26079cc
Merge branch 'main' into tidy-up-merge.go
zeripath 1e7facd
Merge branch 'main' into tidy-up-merge.go
zeripath 01f0aeb
create a common simple way of converting a ColorFormat into a simple …
zeripath bd106eb
Merge remote-tracking branch 'zeripath/tidy-up-merge.go' into tidy-up…
zeripath 71c9b93
adjust pull request colorformat methods
zeripath 0301b3b
Merge remote-tracking branch 'origin/main' into tidy-up-merge.go
zeripath 06c42c3
Merge branch 'main' into tidy-up-merge.go
zeripath 2c56552
Merge branch 'main' into tidy-up-merge.go
zeripath c196577
few more minor changes
zeripath cfdb33d
more minor changes
zeripath 8fddc21
Merge branch 'main' into tidy-up-merge.go
techknowlogick 365b0a1
Merge branch 'main' into tidy-up-merge.go
techknowlogick 7130d24
Merge branch 'main' into tidy-up-merge.go
techknowlogick 5196fd2
Update services/pull/merge.go
techknowlogick ec3c85f
Merge branch 'main' into tidy-up-merge.go
techknowlogick File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
// Copyright 2023 The Gitea Authors. All rights reserved. | ||
// SPDX-License-Identifier: MIT | ||
|
||
package pull | ||
|
||
import ( | ||
repo_model "code.gitea.io/gitea/models/repo" | ||
"code.gitea.io/gitea/modules/git" | ||
"code.gitea.io/gitea/modules/log" | ||
) | ||
|
||
// doMergeStyleMerge merges the tracking into the current HEAD - which is assumed to tbe staging branch (equal to the pr.BaseBranch) | ||
func doMergeStyleMerge(ctx *mergeContext, message string) error { | ||
cmd := git.NewCommand(ctx, "merge", "--no-ff", "--no-commit").AddDynamicArguments(trackingBranch) | ||
if err := runMergeCommand(ctx, repo_model.MergeStyleMerge, cmd); err != nil { | ||
log.Error("%-v Unable to merge tracking into base: %v", ctx.pr, err) | ||
return err | ||
} | ||
|
||
if err := commitAndSignNoAuthor(ctx, message); err != nil { | ||
log.Error("%-v Unable to make final commit: %v", ctx.pr, err) | ||
return err | ||
} | ||
return nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,297 @@ | ||
// Copyright 2023 The Gitea Authors. All rights reserved. | ||
// SPDX-License-Identifier: MIT | ||
|
||
package pull | ||
|
||
import ( | ||
"bufio" | ||
"bytes" | ||
"context" | ||
"fmt" | ||
"io" | ||
"os" | ||
"path/filepath" | ||
"strings" | ||
"time" | ||
|
||
"code.gitea.io/gitea/models" | ||
issues_model "code.gitea.io/gitea/models/issues" | ||
repo_model "code.gitea.io/gitea/models/repo" | ||
user_model "code.gitea.io/gitea/models/user" | ||
"code.gitea.io/gitea/modules/git" | ||
"code.gitea.io/gitea/modules/log" | ||
asymkey_service "code.gitea.io/gitea/services/asymkey" | ||
) | ||
|
||
type mergeContext struct { | ||
*prContext | ||
doer *user_model.User | ||
sig *git.Signature | ||
committer *git.Signature | ||
signArg git.TrustedCmdArgs | ||
env []string | ||
} | ||
|
||
func (ctx *mergeContext) RunOpts() *git.RunOpts { | ||
ctx.outbuf.Reset() | ||
ctx.errbuf.Reset() | ||
return &git.RunOpts{ | ||
Env: ctx.env, | ||
Dir: ctx.tmpBasePath, | ||
Stdout: ctx.outbuf, | ||
Stderr: ctx.errbuf, | ||
} | ||
} | ||
|
||
func createTemporaryRepoForMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, expectedHeadCommitID string) (mergeCtx *mergeContext, cancel context.CancelFunc, err error) { | ||
// Clone base repo. | ||
prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr) | ||
if err != nil { | ||
log.Error("createTemporaryRepoForPR: %v", err) | ||
return nil, cancel, err | ||
} | ||
|
||
mergeCtx = &mergeContext{ | ||
prContext: prCtx, | ||
doer: doer, | ||
} | ||
|
||
if expectedHeadCommitID != "" { | ||
trackingCommitID, _, err := git.NewCommand(ctx, "show-ref", "--hash").AddDynamicArguments(git.BranchPrefix + trackingBranch).RunStdString(&git.RunOpts{Dir: mergeCtx.tmpBasePath}) | ||
if err != nil { | ||
defer cancel() | ||
log.Error("failed to get sha of head branch in %-v: show-ref[%s] --hash refs/heads/tracking: %v", mergeCtx.pr, mergeCtx.tmpBasePath, err) | ||
return nil, nil, fmt.Errorf("unable to get sha of head branch in %v %w", pr, err) | ||
} | ||
if strings.TrimSpace(trackingCommitID) != expectedHeadCommitID { | ||
defer cancel() | ||
return nil, nil, models.ErrSHADoesNotMatch{ | ||
GivenSHA: expectedHeadCommitID, | ||
CurrentSHA: trackingCommitID, | ||
} | ||
} | ||
} | ||
|
||
mergeCtx.outbuf.Reset() | ||
mergeCtx.errbuf.Reset() | ||
if err := prepareTemporaryRepoForMerge(mergeCtx); err != nil { | ||
defer cancel() | ||
return nil, nil, err | ||
} | ||
|
||
mergeCtx.sig = doer.NewGitSig() | ||
mergeCtx.committer = mergeCtx.sig | ||
|
||
// Determine if we should sign | ||
sign, keyID, signer, _ := asymkey_service.SignMerge(ctx, mergeCtx.pr, mergeCtx.doer, mergeCtx.tmpBasePath, "HEAD", trackingBranch) | ||
if sign { | ||
mergeCtx.signArg = git.ToTrustedCmdArgs([]string{"-S" + keyID}) | ||
if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { | ||
mergeCtx.committer = signer | ||
} | ||
} else { | ||
mergeCtx.signArg = git.ToTrustedCmdArgs([]string{"--no-gpg-sign"}) | ||
} | ||
|
||
commitTimeStr := time.Now().Format(time.RFC3339) | ||
|
||
// Because this may call hooks we should pass in the environment | ||
mergeCtx.env = append(os.Environ(), | ||
zeripath marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"GIT_AUTHOR_NAME="+mergeCtx.sig.Name, | ||
"GIT_AUTHOR_EMAIL="+mergeCtx.sig.Email, | ||
"GIT_AUTHOR_DATE="+commitTimeStr, | ||
"GIT_COMMITTER_NAME="+mergeCtx.committer.Name, | ||
"GIT_COMMITTER_EMAIL="+mergeCtx.committer.Email, | ||
"GIT_COMMITTER_DATE="+commitTimeStr, | ||
) | ||
|
||
return mergeCtx, cancel, nil | ||
} | ||
|
||
// prepareTemporaryRepoForMerge takes a repository that has been created using createTemporaryRepo | ||
// it then sets up the sparse-checkout and other things | ||
func prepareTemporaryRepoForMerge(ctx *mergeContext) error { | ||
infoPath := filepath.Join(ctx.tmpBasePath, ".git", "info") | ||
if err := os.MkdirAll(infoPath, 0o700); err != nil { | ||
log.Error("%-v Unable to create .git/info in %s: %v", ctx.pr, ctx.tmpBasePath, err) | ||
return fmt.Errorf("Unable to create .git/info in tmpBasePath: %w", err) | ||
} | ||
|
||
// Enable sparse-checkout | ||
// Here we use the .git/info/sparse-checkout file as described in the git documentation | ||
sparseCheckoutListFile, err := os.OpenFile(filepath.Join(infoPath, "sparse-checkout"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600) | ||
if err != nil { | ||
log.Error("%-v Unable to write .git/info/sparse-checkout file in %s: %v", ctx.pr, ctx.tmpBasePath, err) | ||
return fmt.Errorf("Unable to write .git/info/sparse-checkout file in tmpBasePath: %w", err) | ||
} | ||
defer sparseCheckoutListFile.Close() // we will close it earlier but we need to ensure it is closed if there is an error | ||
|
||
if err := getDiffTree(ctx, ctx.tmpBasePath, baseBranch, trackingBranch, sparseCheckoutListFile); err != nil { | ||
log.Error("%-v getDiffTree(%s, %s, %s): %v", ctx.pr, ctx.tmpBasePath, baseBranch, trackingBranch, err) | ||
return fmt.Errorf("getDiffTree: %w", err) | ||
} | ||
|
||
if err := sparseCheckoutListFile.Close(); err != nil { | ||
log.Error("%-v Unable to close .git/info/sparse-checkout file in %s: %v", ctx.pr, ctx.tmpBasePath, err) | ||
return fmt.Errorf("Unable to close .git/info/sparse-checkout file in tmpBasePath: %w", err) | ||
} | ||
|
||
gitConfigCommand := func() *git.Command { | ||
return git.NewCommand(ctx, "config", "--local") | ||
} | ||
|
||
setConfig := func(key, value string) error { | ||
if err := gitConfigCommand().AddArguments(git.ToTrustedCmdArgs([]string{key, value})...). | ||
Run(ctx.RunOpts()); err != nil { | ||
if value == "" { | ||
value = "<>" | ||
} | ||
log.Error("git config [%s -> %s ]: %v\n%s\n%s", key, value, err, ctx.outbuf.String(), ctx.errbuf.String()) | ||
return fmt.Errorf("git config [%s -> %s ]: %w\n%s\n%s", key, value, err, ctx.outbuf.String(), ctx.errbuf.String()) | ||
} | ||
ctx.outbuf.Reset() | ||
ctx.errbuf.Reset() | ||
|
||
return nil | ||
} | ||
|
||
// Switch off LFS process (set required, clean and smudge here also) | ||
if err := setConfig("filter.lfs.process", ""); err != nil { | ||
return err | ||
} | ||
|
||
if err := setConfig("filter.lfs.required", "false"); err != nil { | ||
return err | ||
} | ||
|
||
if err := setConfig("filter.lfs.clean", ""); err != nil { | ||
return err | ||
} | ||
|
||
if err := setConfig("filter.lfs.smudge", ""); err != nil { | ||
return err | ||
} | ||
|
||
if err := setConfig("core.sparseCheckout", "true"); err != nil { | ||
return err | ||
} | ||
delvh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Read base branch index | ||
if err := git.NewCommand(ctx, "read-tree", "HEAD"). | ||
Run(ctx.RunOpts()); err != nil { | ||
log.Error("git read-tree HEAD: %v\n%s\n%s", err, ctx.outbuf.String(), ctx.errbuf.String()) | ||
return fmt.Errorf("Unable to read base branch in to the index: %w\n%s\n%s", err, ctx.outbuf.String(), ctx.errbuf.String()) | ||
} | ||
ctx.outbuf.Reset() | ||
ctx.errbuf.Reset() | ||
|
||
return nil | ||
} | ||
|
||
// getDiffTree returns a string containing all the files that were changed between headBranch and baseBranch | ||
// the filenames are escaped so as to fit the format required for .git/info/sparse-checkout | ||
func getDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string, out io.Writer) error { | ||
diffOutReader, diffOutWriter, err := os.Pipe() | ||
if err != nil { | ||
log.Error("Unable to create os.Pipe for %s", repoPath) | ||
return err | ||
} | ||
defer func() { | ||
_ = diffOutReader.Close() | ||
_ = diffOutWriter.Close() | ||
}() | ||
|
||
scanNullTerminatedStrings := func(data []byte, atEOF bool) (advance int, token []byte, err error) { | ||
if atEOF && len(data) == 0 { | ||
return 0, nil, nil | ||
} | ||
if i := bytes.IndexByte(data, '\x00'); i >= 0 { | ||
return i + 1, data[0:i], nil | ||
} | ||
if atEOF { | ||
return len(data), data, nil | ||
} | ||
return 0, nil, nil | ||
} | ||
|
||
err = git.NewCommand(ctx, "diff-tree", "--no-commit-id", "--name-only", "-r", "-r", "-z", "--root").AddDynamicArguments(baseBranch, headBranch). | ||
Run(&git.RunOpts{ | ||
Dir: repoPath, | ||
Stdout: diffOutWriter, | ||
PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error { | ||
// Close the writer end of the pipe to begin processing | ||
_ = diffOutWriter.Close() | ||
defer func() { | ||
// Close the reader on return to terminate the git command if necessary | ||
_ = diffOutReader.Close() | ||
}() | ||
|
||
// Now scan the output from the command | ||
scanner := bufio.NewScanner(diffOutReader) | ||
scanner.Split(scanNullTerminatedStrings) | ||
for scanner.Scan() { | ||
filepath := scanner.Text() | ||
// escape '*', '?', '[', spaces and '!' prefix | ||
filepath = escapedSymbols.ReplaceAllString(filepath, `\$1`) | ||
// no necessary to escape the first '#' symbol because the first symbol is '/' | ||
fmt.Fprintf(out, "/%s\n", filepath) | ||
} | ||
return scanner.Err() | ||
}, | ||
}) | ||
return err | ||
} | ||
|
||
// rebaseTrackingOnToBase checks out the tracking branch as staging and rebases it on to the base branch | ||
// if there is a conflict it will return a models.ErrRebaseConflicts | ||
func rebaseTrackingOnToBase(ctx *mergeContext, mergeStyle repo_model.MergeStyle) error { | ||
// Checkout head branch | ||
if err := git.NewCommand(ctx, "checkout", "-b").AddDynamicArguments(stagingBranch, trackingBranch). | ||
Run(ctx.RunOpts()); err != nil { | ||
return fmt.Errorf("unable to git checkout tracking as staging in temp repo for %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) | ||
} | ||
ctx.outbuf.Reset() | ||
ctx.errbuf.Reset() | ||
|
||
// Rebase before merging | ||
if err := git.NewCommand(ctx, "rebase").AddDynamicArguments(baseBranch). | ||
Run(ctx.RunOpts()); err != nil { | ||
// Rebase will leave a REBASE_HEAD file in .git if there is a conflict | ||
if _, statErr := os.Stat(filepath.Join(ctx.tmpBasePath, ".git", "REBASE_HEAD")); statErr == nil { | ||
var commitSha string | ||
ok := false | ||
failingCommitPaths := []string{ | ||
filepath.Join(ctx.tmpBasePath, ".git", "rebase-apply", "original-commit"), // Git < 2.26 | ||
filepath.Join(ctx.tmpBasePath, ".git", "rebase-merge", "stopped-sha"), // Git >= 2.26 | ||
} | ||
for _, failingCommitPath := range failingCommitPaths { | ||
if _, statErr := os.Stat(failingCommitPath); statErr == nil { | ||
commitShaBytes, readErr := os.ReadFile(failingCommitPath) | ||
if readErr != nil { | ||
// Abandon this attempt to handle the error | ||
return fmt.Errorf("unable to git rebase staging on to base in temp repo for %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) | ||
} | ||
commitSha = strings.TrimSpace(string(commitShaBytes)) | ||
ok = true | ||
break | ||
} | ||
} | ||
if !ok { | ||
log.Error("Unable to determine failing commit sha for failing rebase in temp repo for %-v. Cannot cast as models.ErrRebaseConflicts.", ctx.pr) | ||
return fmt.Errorf("unable to git rebase staging on to base in temp repo for %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) | ||
} | ||
log.Debug("Conflict when rebasing staging on to base in %-v at %s: %v\n%s\n%s", ctx.pr, commitSha, err, ctx.outbuf.String(), ctx.errbuf.String()) | ||
return models.ErrRebaseConflicts{ | ||
CommitSHA: commitSha, | ||
Style: mergeStyle, | ||
StdOut: ctx.outbuf.String(), | ||
StdErr: ctx.errbuf.String(), | ||
Err: err, | ||
} | ||
} | ||
return fmt.Errorf("unable to git rebase staging on to base in temp repo for %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) | ||
} | ||
ctx.outbuf.Reset() | ||
ctx.errbuf.Reset() | ||
return nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
// Copyright 2023 The Gitea Authors. All rights reserved. | ||
// SPDX-License-Identifier: MIT | ||
|
||
package pull | ||
|
||
import ( | ||
"fmt" | ||
|
||
repo_model "code.gitea.io/gitea/models/repo" | ||
"code.gitea.io/gitea/modules/git" | ||
"code.gitea.io/gitea/modules/log" | ||
) | ||
|
||
// doMergeStyleRebase rebaases the tracking branch on the base branch as the current HEAD with or with a merge commit to the original pr branch | ||
func doMergeStyleRebase(ctx *mergeContext, mergeStyle repo_model.MergeStyle, message string) error { | ||
if err := rebaseTrackingOnToBase(ctx, mergeStyle); err != nil { | ||
return err | ||
} | ||
|
||
// Checkout base branch again | ||
if err := git.NewCommand(ctx, "checkout").AddDynamicArguments(baseBranch). | ||
Run(ctx.RunOpts()); err != nil { | ||
log.Error("git checkout base prior to merge post staging rebase %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) | ||
return fmt.Errorf("git checkout base prior to merge post staging rebase %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) | ||
} | ||
ctx.outbuf.Reset() | ||
ctx.errbuf.Reset() | ||
|
||
cmd := git.NewCommand(ctx, "merge") | ||
if mergeStyle == repo_model.MergeStyleRebase { | ||
cmd.AddArguments("--ff-only") | ||
} else { | ||
cmd.AddArguments("--no-ff", "--no-commit") | ||
} | ||
cmd.AddDynamicArguments(stagingBranch) | ||
|
||
// Prepare merge with commit | ||
if err := runMergeCommand(ctx, mergeStyle, cmd); err != nil { | ||
log.Error("Unable to merge staging into base: %v", err) | ||
return err | ||
} | ||
if mergeStyle == repo_model.MergeStyleRebaseMerge { | ||
if err := commitAndSignNoAuthor(ctx, message); err != nil { | ||
log.Error("Unable to make final commit: %v", err) | ||
return err | ||
} | ||
} | ||
|
||
return nil | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could think about extracting this check into its own method.