From 0729c535f0c9e8b7f6f3455238a341b2a0803f09 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 11 Aug 2019 01:27:08 +0200 Subject: [PATCH 1/9] remove useless continue --- models/webhook.go | 1 - 1 file changed, 1 deletion(-) diff --git a/models/webhook.go b/models/webhook.go index e3e11e59633f4..ac39501ca1872 100644 --- a/models/webhook.go +++ b/models/webhook.go @@ -890,7 +890,6 @@ func DeliverHooks() { for _, t := range tasks { if err = t.deliver(); err != nil { log.Error("deliver: %v", err) - continue } } From b70cd19921d2f5e387647892651c28160a32c93b Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 11 Aug 2019 03:23:47 +0200 Subject: [PATCH 2/9] remove duplicate call of PushUpdate with post-receive during delete --- modules/repofiles/delete.go | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/modules/repofiles/delete.go b/modules/repofiles/delete.go index a8ab277b28c02..b8a0af4a2cb39 100644 --- a/modules/repofiles/delete.go +++ b/modules/repofiles/delete.go @@ -178,31 +178,9 @@ func DeleteRepoFile(repo *models.Repository, doer *models.User, opts *DeleteRepo return nil, err } - // Simulate push event. - oldCommitID := opts.LastCommitID - if opts.NewBranch != opts.OldBranch { - oldCommitID = git.EmptySHA - } - - if err = repo.GetOwner(); err != nil { + if err = repo.GetOwner(); err != nil { //TODO debug if needed anymore return nil, fmt.Errorf("GetOwner: %v", err) } - err = PushUpdate( - repo, - opts.NewBranch, - models.PushUpdateOptions{ - PusherID: doer.ID, - PusherName: doer.Name, - RepoUserName: repo.Owner.Name, - RepoName: repo.Name, - RefFullName: git.BranchPrefix + opts.NewBranch, - OldCommitID: oldCommitID, - NewCommitID: commitHash, - }, - ) - if err != nil { - return nil, fmt.Errorf("PushUpdate: %v", err) - } commit, err = t.GetCommit(commitHash) if err != nil { From fcbdf3cc8ddd9bd825b81d1d6940bfc1470e8fd8 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 11 Aug 2019 03:25:38 +0200 Subject: [PATCH 3/9] remove duplicate call of PushUpdate with post-receive during create or update --- modules/repofiles/delete.go | 4 ---- modules/repofiles/update.go | 26 -------------------------- 2 files changed, 30 deletions(-) diff --git a/modules/repofiles/delete.go b/modules/repofiles/delete.go index b8a0af4a2cb39..2210faae6bb60 100644 --- a/modules/repofiles/delete.go +++ b/modules/repofiles/delete.go @@ -178,10 +178,6 @@ func DeleteRepoFile(repo *models.Repository, doer *models.User, opts *DeleteRepo return nil, err } - if err = repo.GetOwner(); err != nil { //TODO debug if needed anymore - return nil, fmt.Errorf("GetOwner: %v", err) - } - commit, err = t.GetCommit(commitHash) if err != nil { return nil, err diff --git a/modules/repofiles/update.go b/modules/repofiles/update.go index 26b5113f15ead..62e2afbc960b2 100644 --- a/modules/repofiles/update.go +++ b/modules/repofiles/update.go @@ -396,32 +396,6 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up return nil, err } - // Simulate push event. - oldCommitID := opts.LastCommitID - if opts.NewBranch != opts.OldBranch || oldCommitID == "" { - oldCommitID = git.EmptySHA - } - - if err = repo.GetOwner(); err != nil { - return nil, fmt.Errorf("GetOwner: %v", err) - } - err = PushUpdate( - repo, - opts.NewBranch, - models.PushUpdateOptions{ - PusherID: doer.ID, - PusherName: doer.Name, - RepoUserName: repo.Owner.Name, - RepoName: repo.Name, - RefFullName: git.BranchPrefix + opts.NewBranch, - OldCommitID: oldCommitID, - NewCommitID: commitHash, - }, - ) - if err != nil { - return nil, fmt.Errorf("PushUpdate: %v", err) - } - commit, err = t.GetCommit(commitHash) if err != nil { return nil, err From 15e20e81315eb7c388120ec9899912e585fecb9e Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 11 Aug 2019 03:26:54 +0200 Subject: [PATCH 4/9] remove duplicate call of PushUpdate with post-receive during upload --- modules/repofiles/upload.go | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/modules/repofiles/upload.go b/modules/repofiles/upload.go index 2da101c64dad6..228cd025efda4 100644 --- a/modules/repofiles/upload.go +++ b/modules/repofiles/upload.go @@ -177,31 +177,5 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep return err } - // Simulate push event. - oldCommitID := opts.LastCommitID - if opts.NewBranch != opts.OldBranch { - oldCommitID = git.EmptySHA - } - - if err = repo.GetOwner(); err != nil { - return fmt.Errorf("GetOwner: %v", err) - } - err = PushUpdate( - repo, - opts.NewBranch, - models.PushUpdateOptions{ - PusherID: doer.ID, - PusherName: doer.Name, - RepoUserName: repo.Owner.Name, - RepoName: repo.Name, - RefFullName: git.BranchPrefix + opts.NewBranch, - OldCommitID: oldCommitID, - NewCommitID: commitHash, - }, - ) - if err != nil { - return fmt.Errorf("PushUpdate: %v", err) - } - return models.DeleteUploads(uploads...) } From 24ebadc9a764e5381adb9fdb9b9b77d92c3915df Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 11 Aug 2019 03:36:18 +0200 Subject: [PATCH 5/9] fix imports --- modules/repofiles/upload.go | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/repofiles/upload.go b/modules/repofiles/upload.go index 228cd025efda4..f2ffec7ebcddb 100644 --- a/modules/repofiles/upload.go +++ b/modules/repofiles/upload.go @@ -11,7 +11,6 @@ import ( "strings" "code.gitea.io/gitea/models" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/setting" ) From 7eb03a58dfe1003eff7aff4621ef15b64ced19b4 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 11 Aug 2019 14:13:17 +0200 Subject: [PATCH 6/9] remove duplicate call of PushUpdate with post-receive during merge --- modules/pull/merge.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/modules/pull/merge.go b/modules/pull/merge.go index ed2fe48379c3b..8615622392b00 100644 --- a/modules/pull/merge.go +++ b/modules/pull/merge.go @@ -310,21 +310,6 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor 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 } From 6f9ae4384c4102e7823995532e13af1fd2609098 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 11 Aug 2019 14:42:35 +0200 Subject: [PATCH 7/9] remove un-needed code --- modules/pull/merge.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/modules/pull/merge.go b/modules/pull/merge.go index 8615622392b00..cf2fb7fc4f3de 100644 --- a/modules/pull/merge.go +++ b/modules/pull/merge.go @@ -292,24 +292,6 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor 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) - } - return nil } From 76e3f8c428803b74f0c44918aa3706314e0f51a5 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 11 Aug 2019 18:26:18 +0200 Subject: [PATCH 8/9] add test for merge --- integrations/pull_merge_test.go | 34 +++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/integrations/pull_merge_test.go b/integrations/pull_merge_test.go index f3efa63b077af..412b2bb97aaf8 100644 --- a/integrations/pull_merge_test.go +++ b/integrations/pull_merge_test.go @@ -54,6 +54,10 @@ func testPullCleanUp(t *testing.T, session *TestSession, user, repo, pullnum str func TestPullMerge(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + hookTasks, err := models.HookTasks(1, 1) //Retrieve previous hook number + assert.NoError(t, err) + hookTasksLenBefore := len(hookTasks) + session := loginUser(t, "user1") testRepoFork(t, session, "user2", "repo1", "user1", "repo1") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") @@ -63,11 +67,19 @@ func TestPullMerge(t *testing.T) { elem := strings.Split(test.RedirectURL(resp), "/") assert.EqualValues(t, "pulls", elem[3]) testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleMerge) + + hookTasks, err = models.HookTasks(1, 1) + assert.NoError(t, err) + assert.Len(t, hookTasks, hookTasksLenBefore + 1) }) } func TestPullRebase(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + hookTasks, err := models.HookTasks(1, 1) //Retrieve previous hook number + assert.NoError(t, err) + hookTasksLenBefore := len(hookTasks) + session := loginUser(t, "user1") testRepoFork(t, session, "user2", "repo1", "user1", "repo1") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") @@ -77,12 +89,21 @@ func TestPullRebase(t *testing.T) { elem := strings.Split(test.RedirectURL(resp), "/") assert.EqualValues(t, "pulls", elem[3]) testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleRebase) + + hookTasks, err = models.HookTasks(1, 1) + assert.NoError(t, err) + assert.Len(t, hookTasks, hookTasksLenBefore + 1) }) } func TestPullRebaseMerge(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { prepareTestEnv(t) + + hookTasks, err := models.HookTasks(1, 1) //Retrieve previous hook number + assert.NoError(t, err) + hookTasksLenBefore := len(hookTasks) + session := loginUser(t, "user1") testRepoFork(t, session, "user2", "repo1", "user1", "repo1") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") @@ -92,12 +113,21 @@ func TestPullRebaseMerge(t *testing.T) { elem := strings.Split(test.RedirectURL(resp), "/") assert.EqualValues(t, "pulls", elem[3]) testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleRebaseMerge) + + hookTasks, err = models.HookTasks(1, 1) + assert.NoError(t, err) + assert.Len(t, hookTasks, hookTasksLenBefore + 1) }) } func TestPullSquash(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { prepareTestEnv(t) + + hookTasks, err := models.HookTasks(1, 1) //Retrieve previous hook number + assert.NoError(t, err) + hookTasksLenBefore := len(hookTasks) + session := loginUser(t, "user1") testRepoFork(t, session, "user2", "repo1", "user1", "repo1") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") @@ -108,6 +138,10 @@ func TestPullSquash(t *testing.T) { elem := strings.Split(test.RedirectURL(resp), "/") assert.EqualValues(t, "pulls", elem[3]) testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleSquash) + + hookTasks, err = models.HookTasks(1, 1) + assert.NoError(t, err) + assert.Len(t, hookTasks, hookTasksLenBefore + 1) }) } From d8d8668c6a5984f72926a00c538d406118b1e31c Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Sun, 11 Aug 2019 18:28:12 +0200 Subject: [PATCH 9/9] fmt --- integrations/pull_merge_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/integrations/pull_merge_test.go b/integrations/pull_merge_test.go index 412b2bb97aaf8..2f4e48f2938a8 100644 --- a/integrations/pull_merge_test.go +++ b/integrations/pull_merge_test.go @@ -70,7 +70,7 @@ func TestPullMerge(t *testing.T) { hookTasks, err = models.HookTasks(1, 1) assert.NoError(t, err) - assert.Len(t, hookTasks, hookTasksLenBefore + 1) + assert.Len(t, hookTasks, hookTasksLenBefore+1) }) } @@ -79,7 +79,7 @@ func TestPullRebase(t *testing.T) { hookTasks, err := models.HookTasks(1, 1) //Retrieve previous hook number assert.NoError(t, err) hookTasksLenBefore := len(hookTasks) - + session := loginUser(t, "user1") testRepoFork(t, session, "user2", "repo1", "user1", "repo1") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") @@ -92,7 +92,7 @@ func TestPullRebase(t *testing.T) { hookTasks, err = models.HookTasks(1, 1) assert.NoError(t, err) - assert.Len(t, hookTasks, hookTasksLenBefore + 1) + assert.Len(t, hookTasks, hookTasksLenBefore+1) }) } @@ -116,7 +116,7 @@ func TestPullRebaseMerge(t *testing.T) { hookTasks, err = models.HookTasks(1, 1) assert.NoError(t, err) - assert.Len(t, hookTasks, hookTasksLenBefore + 1) + assert.Len(t, hookTasks, hookTasksLenBefore+1) }) } @@ -141,7 +141,7 @@ func TestPullSquash(t *testing.T) { hookTasks, err = models.HookTasks(1, 1) assert.NoError(t, err) - assert.Len(t, hookTasks, hookTasksLenBefore + 1) + assert.Len(t, hookTasks, hookTasksLenBefore+1) }) }