From f0ed6de89da27ec4d908a5a807fd5da4fbe62b7f Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 23 Feb 2024 21:42:15 +0100 Subject: [PATCH] [FEAT] Check if commit is already present in target branch - Check if someone is (accidentally) trying to create a pull request via AGit with changes already in the target branch and fail if that is the case. - Added integration test. --- services/agit/agit.go | 15 +++++++++++++++ tests/integration/git_test.go | 23 +++++++++++++++++++---- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/services/agit/agit.go b/services/agit/agit.go index 35bfb3340..4c970ee78 100644 --- a/services/agit/agit.go +++ b/services/agit/agit.go @@ -101,6 +101,21 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git. return nil, fmt.Errorf("failed to get unmerged AGit flow pull request in repository %q: %w", repo.FullName(), err) } + // Check if the changes are already in the target branch. + stdout, _, gitErr := git.NewCommand(ctx, "branch", "--contains").AddDynamicArguments(opts.NewCommitIDs[i], baseBranchName).RunStdString(&git.RunOpts{Dir: repo.RepoPath()}) + if gitErr != nil { + return nil, fmt.Errorf("failed to check if the target branch already contains the new commit in repository %q: %w", repo.FullName(), err) + } + if len(stdout) > 0 { + results = append(results, private.HookProcReceiveRefResult{ + OriginalRef: opts.RefFullNames[i], + OldOID: opts.OldCommitIDs[i], + NewOID: opts.NewCommitIDs[i], + Err: "The target branch already contains this commit", + }) + continue + } + // Automatically fill out the title and the description from the first commit. shouldGetCommit := len(title) == 0 || len(description) == 0 diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index d02427ffc..125950a3a 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -937,13 +937,13 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headB }) }) + upstreamGitRepo, err := git.OpenRepository(git.DefaultContext, filepath.Join(setting.RepoRootPath, ctx.Username, ctx.Reponame+".git")) + require.NoError(t, err) + defer upstreamGitRepo.Close() + t.Run("Force push", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - upstreamGitRepo, err := git.OpenRepository(git.DefaultContext, filepath.Join(setting.RepoRootPath, ctx.Username, ctx.Reponame+".git")) - require.NoError(t, err) - defer upstreamGitRepo.Close() - _, _, gitErr := git.NewCommand(git.DefaultContext, "push", "origin").AddDynamicArguments("HEAD:refs/for/master/" + headBranch + "-force-push").RunStdString(&git.RunOpts{Dir: dstPath}) require.NoError(t, gitErr) @@ -984,6 +984,21 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headB }) }) + t.Run("Branch already contains commit", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + branchCommit, err := upstreamGitRepo.GetBranchCommit("master") + require.NoError(t, err) + + _, _, gitErr := git.NewCommand(git.DefaultContext, "reset", "--hard").AddDynamicArguments(branchCommit.ID.String() + "~1").RunStdString(&git.RunOpts{Dir: dstPath}) + require.NoError(t, gitErr) + + _, stdErr, gitErr := git.NewCommand(git.DefaultContext, "push", "origin").AddDynamicArguments("HEAD:refs/for/master/" + headBranch + "-already-contains").RunStdString(&git.RunOpts{Dir: dstPath}) + assert.Error(t, gitErr) + + assert.Contains(t, stdErr, "already contains this commit") + }) + t.Run("Merge", doAPIMergePullRequest(*ctx, ctx.Username, ctx.Reponame, pr1.Index)) t.Run("CheckoutMasterAgain", doGitCheckoutBranch(dstPath, "master")) }