From 1d4cf964605496bc4a6d33691d5f96ca634967d24c20b27310ae0c76e908dfa6 Mon Sep 17 00:00:00 2001 From: Adam Majer Date: Thu, 19 Feb 2026 15:30:10 +0100 Subject: [PATCH 1/3] pr: add merge modes documentation and config parsing --- common/config.go | 24 +++++++++++++--- common/config_test.go | 64 +++++++++++++++++++++++++++++++++++++++++++ workflow-pr/README.md | 13 +++++++++ 3 files changed, 97 insertions(+), 4 deletions(-) diff --git a/common/config.go b/common/config.go index 602d916..7a4a53f 100644 --- a/common/config.go +++ b/common/config.go @@ -39,6 +39,10 @@ const ( Permission_ForceMerge = "force-merge" Permission_Group = "release-engineering" + + MergeModeFF = "ff-only" + MergeModeReplace = "replace" + MergeModeDevel = "devel" ) type ConfigFile struct { @@ -52,9 +56,9 @@ type ReviewGroup struct { } type QAConfig struct { - Name string - Origin string - Label string // requires this gitea lable to be set or skipped + Name string + Origin string + Label string // requires this gitea lable to be set or skipped BuildDisableRepos []string // which repos to build disable in the new project } @@ -89,7 +93,8 @@ type AutogitConfig struct { Committers []string // group in addition to Reviewers and Maintainers that can order the bot around, mostly as helper for factory-maintainers Subdirs []string // list of directories to sort submodules into. Needed b/c _manifest cannot list non-existent directories - Labels map[string]string // list of tags, if not default, to apply + Labels map[string]string // list of tags, if not default, to apply + MergeMode string // project merge mode NoProjectGitPR bool // do not automatically create project git PRs, just assign reviewers and assume somethign else creates the ProjectGit PR ManualMergeOnly bool // only merge with "Merge OK" comment by Project Maintainers and/or Package Maintainers and/or reviewers @@ -184,6 +189,17 @@ func ReadWorkflowConfig(gitea GiteaFileContentAndRepoFetcher, git_project string } } config.GitProjectName = config.GitProjectName + "#" + branch + + // verify merge modes + switch config.MergeMode { + case MergeModeFF, MergeModeDevel, MergeModeReplace: + break // good results + case "": + config.MergeMode = MergeModeFF + default: + return nil, fmt.Errorf("Unsupported merge mode in %s: %s", git_project, config.MergeMode) + } + return config, nil } diff --git a/common/config_test.go b/common/config_test.go index 2398cda..e223f8c 100644 --- a/common/config_test.go +++ b/common/config_test.go @@ -341,3 +341,67 @@ func TestConfigPermissions(t *testing.T) { }) } } + +func TestConfigMergeModeParser(t *testing.T) { + tests := []struct { + name string + json string + mergeMode string + wantErr bool + }{ + { + name: "empty", + json: "{}", + mergeMode: common.MergeModeFF, + }, + { + name: "ff-only", + json: `{"MergeMode": "ff-only"}`, + mergeMode: common.MergeModeFF, + }, + { + name: "replace", + json: `{"MergeMode": "replace"}`, + mergeMode: common.MergeModeReplace, + }, + { + name: "devel", + json: `{"MergeMode": "devel"}`, + mergeMode: common.MergeModeDevel, + }, + { + name: "unsupported", + json: `{"MergeMode": "invalid"}`, + wantErr: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + repo := models.Repository{ + DefaultBranch: "master", + } + + ctl := gomock.NewController(t) + gitea := mock_common.NewMockGiteaFileContentAndRepoFetcher(ctl) + gitea.EXPECT().GetRepositoryFileContent("foo", "bar", "", "workflow.config").Return([]byte(test.json), "abc", nil) + gitea.EXPECT().GetRepository("foo", "bar").Return(&repo, nil) + + config, err := common.ReadWorkflowConfig(gitea, "foo/bar") + if test.wantErr { + if err == nil { + t.Fatal("Expected error, got nil") + } + return + } + + if err != nil { + t.Fatal(err) + } + + if config.MergeMode != test.mergeMode { + t.Errorf("Expected MergeMode %s, got %s", test.mergeMode, config.MergeMode) + } + }) + } +} diff --git a/workflow-pr/README.md b/workflow-pr/README.md index 62bb463..3854a36 100644 --- a/workflow-pr/README.md +++ b/workflow-pr/README.md @@ -96,6 +96,19 @@ Package Deletion Requests If you wish to re-add a package, create a new PrjGit PR which adds again the submodule on the branch that has the "-removed" suffix. The bot will automatically remove this suffix from the project branch in the pool. +Merge Modes +----------- + +| Merge Mode | Description +|------------|-------------------------------------------------------------------------------- +| ff-only | Only allow --ff-only merges in the package branch. This is best suited for +| | devel projects and openSUSE Tumbleweed development, where history should be linear +| replace | Merge is done via `-X theirs` strategy and old files are removed in the merge. +| | This works well for downstream codestreams, like Leap, that would update their branch +| | using latest version. +| devel | No merge, just set the project branch to PR HEAD. This is suitable for downstream +| | projects like Leap during development cycle, where keeping maintenance history is not important + Labels ------ -- 2.51.1 From 6d8c0b941497acdff45e8de2f2ec87d082b231bb2687b05c66d5e01cd204d3e2 Mon Sep 17 00:00:00 2001 From: Adam Majer Date: Thu, 19 Feb 2026 16:18:55 +0100 Subject: [PATCH 2/3] pr: add function that checks and prepares PRs --- common/pr.go | 158 +++++++++++++++- common/pr_test.go | 348 +++++++++++++++++++++++++++++++++++- workflow-pr/pr_processor.go | 6 + 3 files changed, 502 insertions(+), 10 deletions(-) diff --git a/common/pr.go b/common/pr.go index 013f421..aab2472 100644 --- a/common/pr.go +++ b/common/pr.go @@ -554,6 +554,145 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData return is_manually_reviewed_ok } +func (rs *PRSet) AddMergeCommit(git Git, remote string, pr int) bool { + prinfo := rs.PRs[pr] + + LogDebug("Adding merge commit for %s", PRtoString(prinfo.PR)) + if !prinfo.PR.AllowMaintainerEdit { + LogError(" PR is not editable by maintainer") + return false + } + + repo := prinfo.PR.Base.Repo + head := prinfo.PR.Head + br := rs.Config.Branch + if len(br) == 0 { + br = prinfo.PR.Base.Name + } + + msg := fmt.Sprintf("Merge branch '%s' into %s", br, head.Name) + if err := git.GitExec(repo.Name, "merge", "--no-ff", "--no-commit", "-X", "theirs", head.Sha); err != nil { + if err := git.GitExec(repo.Name, "merge", "--no-ff", "--no-commit", "--allow-unrelated-histories", "-X", "theirs", head.Sha); err != nil { + return false + } + LogError("WARNING: Merging unrelated histories") + } + + // ensure only files that are in head.Sha are kept + git.GitExecOrPanic(repo.Name, "read-tree", "-m", head.Sha) + git.GitExecOrPanic(repo.Name, "commit", "-m", msg) + git.GitExecOrPanic(repo.Name, "clean", "-fxd") + + if !IsDryRun { + git.GitExecOrPanic(repo.Name, "push", remote, "HEAD:"+head.Name) + prinfo.PR.Head.Sha = strings.TrimSpace(git.GitExecWithOutputOrPanic(repo.Name, "rev-list", "-1", "HEAD")) // need to update as it's pushed but pr not refetched + } + + return true +} + +func (rs *PRSet) HasMerge(git Git, pr int) bool { + prinfo := rs.PRs[pr] + + repo := prinfo.PR.Base.Repo + head := prinfo.PR.Head + br := rs.Config.Branch + if len(br) == 0 { + br = prinfo.PR.Base.Name + } + + parents, err := git.GitExecWithOutput(repo.Name, "show", "-s", "--format=%P", head.Sha) + if err == nil { + p := strings.Fields(strings.TrimSpace(parents)) + if len(p) == 2 { + targetHead, _ := git.GitExecWithOutput(repo.Name, "rev-parse", "HEAD") + targetHead = strings.TrimSpace(targetHead) + if p[0] == targetHead || p[1] == targetHead { + return true + } + } + } + return false +} + +func (rs *PRSet) PrepareForMerge(git Git) bool { + // verify that package can merge here. Checkout current target branch of each PRSet, make a temporary branch + // PR_#_mergetest and perform the merge based + + if rs.Config.MergeMode == MergeModeDevel { + return true // always can merge as we set branch here, not merge anything + } else { + // make sure that all the package PRs are in mergeable state + for idx, prinfo := range rs.PRs { + if rs.IsPrjGitPR(prinfo.PR) { + continue + } + + repo := prinfo.PR.Base.Repo + head := prinfo.PR.Head + br := rs.Config.Branch + if len(br) == 0 { + br = prinfo.PR.Base.Name + } + + remote, err := git.GitClone(repo.Name, br, repo.SSHURL) + if err != nil { + return false + } + git.GitExecOrPanic(repo.Name, "fetch", remote, head.Sha) + switch rs.Config.MergeMode { + case MergeModeFF: + if err := git.GitExec(repo.Name, "merge-base", "--is-ancestor", "HEAD", head.Sha); err != nil { + return false + } + case MergeModeReplace: + Verify: + if err := git.GitExec(repo.Name, "merge-base", "--is-ancestor", "HEAD", head.Sha); err != nil { + if !rs.HasMerge(git, idx) { + forkRemote, err := git.GitClone(repo.Name, head.Name, head.Repo.SSHURL) + if err != nil { + LogError("Failed to clone head repo:", head.Name, head.Repo.SSHURL) + return false + } + LogDebug("Merge commit is missing and this is not FF merge possibility") + git.GitExecOrPanic(repo.Name, "checkout", remote+"/"+br) + if !rs.AddMergeCommit(git, forkRemote, idx) { + return false + } + if !IsDryRun { + goto Verify + } + } + } + } + } + } + + // now we check project git if mergeable + prjgit_info, err := rs.GetPrjGitPR() + if err != nil { + return false + } + prjgit := prjgit_info.PR + + _, _, prjgitBranch := rs.Config.GetPrjGit() + remote, err := git.GitClone(DefaultGitPrj, prjgitBranch, prjgit.Base.Repo.SSHURL) + if err != nil { + return false + } + + testBranch := fmt.Sprintf("PR_%d_mergetest", prjgit.Index) + git.GitExecOrPanic(DefaultGitPrj, "fetch", remote, prjgit.Head.Sha) + if err := git.GitExec(DefaultGitPrj, "checkout", "-B", testBranch, prjgit.Base.Sha); err != nil { + return false + } + if err := git.GitExec(DefaultGitPrj, "merge", "--no-ff", "--no-commit", prjgit.Head.Sha); err != nil { + return false + } + + return true +} + func (rs *PRSet) Merge(gitea GiteaReviewUnrequester, git Git) error { prjgit_info, err := rs.GetPrjGitPR() if err != nil { @@ -718,11 +857,10 @@ func (rs *PRSet) Merge(gitea GiteaReviewUnrequester, git Git) error { prinfo.RemoteName, err = git.GitClone(repo.Name, br, repo.SSHURL) PanicOnError(err) git.GitExecOrPanic(repo.Name, "fetch", prinfo.RemoteName, head.Sha) - - if isNewRepo { - LogInfo("Force-pushing new repository branch", br, "to", head.Sha) - // we don't merge, we just set the branch to this commit + if rs.Config.MergeMode == MergeModeDevel || isNewRepo { + git.GitExecOrPanic(repo.Name, "checkout", "-B", br, head.Sha) } else { + git.GitExecOrPanic(repo.Name, "fetch", prinfo.RemoteName, head.Sha) git.GitExecOrPanic(repo.Name, "merge", "--ff", head.Sha) } } @@ -748,11 +886,15 @@ func (rs *PRSet) Merge(gitea GiteaReviewUnrequester, git Git) error { } if !IsDryRun { - if isNewRepo { - git.GitExecOrPanic(repo.Name, "push", "-f", prinfo.RemoteName, prinfo.PR.Head.Sha+":"+prinfo.PR.Base.Name) - } else { - git.GitExecOrPanic(repo.Name, "push", prinfo.RemoteName) + params := []string{"push"} + if rs.Config.MergeMode == MergeModeDevel || isNewRepo { + params = append(params, "-f") } + params = append(params, prinfo.RemoteName) + if isNewRepo { + params = append(params, prinfo.PR.Head.Sha+":"+prinfo.PR.Base.Name) + } + git.GitExecOrPanic(repo.Name, params...) } else { LogInfo("*** WOULD push", repo.Name, "to", prinfo.RemoteName) } diff --git a/common/pr_test.go b/common/pr_test.go index 4b9fe63..1617a7f 100644 --- a/common/pr_test.go +++ b/common/pr_test.go @@ -2,6 +2,7 @@ package common_test import ( "errors" + "fmt" "os" "os/exec" "path" @@ -1273,7 +1274,7 @@ func TestPRMerge(t *testing.T) { Owner: &models.User{ UserName: "org", }, - SSHURL: "file://" + path.Join(repoDir, "prjgit"), + SSHURL: "ssh://git@src.opensuse.org/org/prj.git", }, }, Head: &models.PRBranchInfo{ @@ -1295,7 +1296,7 @@ func TestPRMerge(t *testing.T) { Owner: &models.User{ UserName: "org", }, - SSHURL: "file://" + path.Join(cmd.Dir, "prjgit"), + SSHURL: "ssh://git@src.opensuse.org/org/prj.git", }, }, Head: &models.PRBranchInfo{ @@ -1405,3 +1406,346 @@ func TestPRChanges(t *testing.T) { }) } } + +func TestPRPrepareForMerge(t *testing.T) { + tests := []struct { + name string + setup func(*mock_common.MockGit, *models.PullRequest, *models.PullRequest) + config *common.AutogitConfig + expected bool + editable bool + }{ + { + name: "Success Devel", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeDevel, + }, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) {}, + expected: true, + }, + { + name: "Success FF", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeFF, + }, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) { + m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha) + m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(nil) + + m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha) + m.EXPECT().GitExec("_ObsPrj", "checkout", "-B", "PR_1_mergetest", prjPR.Base.Sha).Return(nil) + m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "--no-commit", prjPR.Head.Sha).Return(nil) + }, + expected: true, + }, + { + name: "Success Replace MergeCommit", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeReplace, + }, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) { + m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha) + // merge-base fails initially + m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(fmt.Errorf("not ancestor")) + // HasMerge returns true + m.EXPECT().GitExecWithOutput("pkg", "show", "-s", "--format=%P", pkgPR.Head.Sha).Return("parent1 target_head", nil) + m.EXPECT().GitExecWithOutput("pkg", "rev-parse", "HEAD").Return("target_head", nil) + + m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha) + m.EXPECT().GitExec("_ObsPrj", "checkout", "-B", "PR_1_mergetest", prjPR.Base.Sha).Return(nil) + m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "--no-commit", prjPR.Head.Sha).Return(nil) + }, + expected: true, + }, + { + name: "Merge Conflict in PrjGit", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeFF, + }, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) { + m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha) + m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(nil) + + m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha) + m.EXPECT().GitExec("_ObsPrj", "checkout", "-B", "PR_1_mergetest", prjPR.Base.Sha).Return(nil) + m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "--no-commit", prjPR.Head.Sha).Return(fmt.Errorf("conflict")) + }, + expected: false, + }, + { + name: "Not FF in PkgGit", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeFF, + }, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) { + m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha) + m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(fmt.Errorf("not ancestor")) + }, + expected: false, + }, + { + name: "Success Replace with AddMergeCommit", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeReplace, + }, + editable: true, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) { + m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin", pkgPR.Head.Sha) + // First merge-base fails + m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", pkgPR.Head.Sha).Return(fmt.Errorf("not ancestor")) + // HasMerge returns false + m.EXPECT().GitExecWithOutput("pkg", "show", "-s", "--format=%P", pkgPR.Head.Sha).Return("parent1", nil) + m.EXPECT().GitClone("pkg", pkgPR.Head.Name, pkgPR.Base.Repo.SSHURL).Return("origin_fork", nil) + // AddMergeCommit is called + m.EXPECT().GitExecOrPanic("pkg", "checkout", "origin/master") + m.EXPECT().GitExec("pkg", "merge", "--no-ff", "--no-commit", "-X", "theirs", pkgPR.Head.Sha).Return(nil) + m.EXPECT().GitExecOrPanic("pkg", "read-tree", "-m", pkgPR.Head.Sha) + m.EXPECT().GitExecOrPanic("pkg", "commit", "-m", gomock.Any()) + m.EXPECT().GitExecOrPanic("pkg", "clean", "-fxd") + m.EXPECT().GitExecOrPanic("pkg", "push", "origin_fork", "HEAD:"+pkgPR.Head.Name) + m.EXPECT().GitExecWithOutputOrPanic("pkg", "rev-list", "-1", "HEAD").Return("new_pkg_head_sha") + // Second merge-base succeeds (after goto Verify) + m.EXPECT().GitExec("pkg", "merge-base", "--is-ancestor", "HEAD", "new_pkg_head_sha").Return(nil) + + m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha) + m.EXPECT().GitExec("_ObsPrj", "checkout", "-B", "PR_1_mergetest", prjPR.Base.Sha).Return(nil) + m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "--no-commit", prjPR.Head.Sha).Return(nil) + }, + expected: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + prjPR := &models.PullRequest{ + Index: 1, + Base: &models.PRBranchInfo{ + Name: "master", + Sha: "base_sha", + Repo: &models.Repository{ + Owner: &models.User{UserName: "org"}, + Name: "_ObsPrj", + SSHURL: "ssh://git@src.opensuse.org/org/_ObsPrj.git", + }, + }, + Head: &models.PRBranchInfo{ + Sha: "head_sha", + Repo: &models.Repository{ + Owner: &models.User{UserName: "org"}, + Name: "_ObsPrj", + SSHURL: "ssh://git@src.opensuse.org/org/_ObsPrj.git", + }, + }, + } + + pkgPR := &models.PullRequest{ + Index: 2, + Base: &models.PRBranchInfo{ + Name: "master", + Sha: "pkg_base_sha", + Repo: &models.Repository{ + Owner: &models.User{UserName: "org"}, + Name: "pkg", + SSHURL: "ssh://git@src.opensuse.org/org/pkg.git", + }, + }, + Head: &models.PRBranchInfo{ + Name: "branch_name", + Sha: "pkg_head_sha", + Repo: &models.Repository{ + Owner: &models.User{UserName: "org"}, + Name: "pkg", + SSHURL: "ssh://git@src.opensuse.org/org/pkg.git", + }, + }, + AllowMaintainerEdit: test.editable, + } + + ctl := gomock.NewController(t) + git := mock_common.NewMockGit(ctl) + test.setup(git, prjPR, pkgPR) + + prset := &common.PRSet{ + Config: test.config, + PRs: []*common.PRInfo{ + {PR: prjPR}, + {PR: pkgPR}, + }, + } + + if res := prset.PrepareForMerge(git); res != test.expected { + t.Errorf("Expected %v, got %v", test.expected, res) + } + }) + } +} + +func TestPRMergeMock(t *testing.T) { + tests := []struct { + name string + setup func(*mock_common.MockGit, *models.PullRequest, *models.PullRequest) + config *common.AutogitConfig + }{ + { + name: "Success FF", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeFF, + }, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) { + m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha) + m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "-m", gomock.Any(), prjPR.Head.Sha).Return(nil) + + m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin_pkg", nil) + m.EXPECT().GitExecOrPanic("pkg", "fetch", "origin_pkg", pkgPR.Head.Sha) + m.EXPECT().GitExecOrPanic("pkg", "merge", "--ff", pkgPR.Head.Sha) + m.EXPECT().GitExecOrPanic("pkg", "push", "origin_pkg") + m.EXPECT().GitExecOrPanic("_ObsPrj", "push", "origin") + }, + }, + { + name: "Success Devel", + config: &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeDevel, + }, + setup: func(m *mock_common.MockGit, prjPR, pkgPR *models.PullRequest) { + m.EXPECT().GitClone("_ObsPrj", "master", prjPR.Base.Repo.SSHURL).Return("origin", nil) + m.EXPECT().GitExecOrPanic("_ObsPrj", "fetch", "origin", prjPR.Head.Sha) + m.EXPECT().GitExec("_ObsPrj", "merge", "--no-ff", "-m", gomock.Any(), prjPR.Head.Sha).Return(nil) + + m.EXPECT().GitClone("pkg", "master", pkgPR.Base.Repo.SSHURL).Return("origin_pkg", nil) + m.EXPECT().GitExecOrPanic("pkg", "checkout", "-B", "master", pkgPR.Head.Sha) + m.EXPECT().GitExecOrPanic("pkg", "push", "-f", "origin_pkg") + m.EXPECT().GitExecOrPanic("_ObsPrj", "push", "origin") + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + prjPR := &models.PullRequest{ + Index: 1, + Base: &models.PRBranchInfo{ + Name: "master", + Sha: "prj_base_sha", + Repo: &models.Repository{ + Owner: &models.User{UserName: "org"}, + Name: "_ObsPrj", + SSHURL: "ssh://git@src.opensuse.org/org/_ObsPrj.git", + }, + }, + Head: &models.PRBranchInfo{ + Sha: "prj_head_sha", + }, + } + pkgPR := &models.PullRequest{ + Index: 2, + Base: &models.PRBranchInfo{ + Name: "master", + Sha: "pkg_base_sha", + Repo: &models.Repository{ + Owner: &models.User{UserName: "org"}, + Name: "pkg", + SSHURL: "ssh://git@src.opensuse.org/org/pkg.git", + }, + }, + Head: &models.PRBranchInfo{ + Sha: "pkg_head_sha", + }, + } + + ctl := gomock.NewController(t) + git := mock_common.NewMockGit(ctl) + reviewUnrequestMock := mock_common.NewMockGiteaReviewUnrequester(ctl) + reviewUnrequestMock.EXPECT().UnrequestReview(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil) + + test.setup(git, prjPR, pkgPR) + + prset := &common.PRSet{ + Config: test.config, + PRs: []*common.PRInfo{ + {PR: prjPR}, + {PR: pkgPR}, + }, + } + + if err := prset.Merge(reviewUnrequestMock, git); err != nil { + t.Errorf("Unexpected error: %v", err) + } + }) + } +} + +func TestPRAddMergeCommit(t *testing.T) { + pkgPR := &models.PullRequest{ + Index: 2, + Base: &models.PRBranchInfo{ + Name: "master", + Sha: "pkg_base_sha", + Repo: &models.Repository{ + Owner: &models.User{UserName: "org"}, + Name: "pkg", + SSHURL: "ssh://git@src.opensuse.org/org/pkg.git", + }, + }, + Head: &models.PRBranchInfo{ + Name: "branch_name", + Sha: "pkg_head_sha", + }, + AllowMaintainerEdit: true, + } + + config := &common.AutogitConfig{ + Organization: "org", + GitProjectName: "org/_ObsPrj#master", + MergeMode: common.MergeModeReplace, + } + + ctl := gomock.NewController(t) + git := mock_common.NewMockGit(ctl) + + git.EXPECT().GitExec("pkg", "merge", "--no-ff", "--no-commit", "-X", "theirs", pkgPR.Head.Sha).Return(nil) + git.EXPECT().GitExecOrPanic("pkg", "read-tree", "-m", pkgPR.Head.Sha) + git.EXPECT().GitExecOrPanic("pkg", "commit", "-m", gomock.Any()) + git.EXPECT().GitExecOrPanic("pkg", "clean", "-fxd") + git.EXPECT().GitExecOrPanic("pkg", "push", "origin", "HEAD:branch_name") + git.EXPECT().GitExecWithOutputOrPanic("pkg", "rev-list", "-1", "HEAD").Return("new_head_sha") + + prset := &common.PRSet{ + Config: config, + PRs: []*common.PRInfo{ + {PR: &models.PullRequest{}}, // prjgit at index 0 + {PR: pkgPR}, // pkg at index 1 + }, + } + + if res := prset.AddMergeCommit(git, "origin", 1); !res { + t.Errorf("Expected true, got %v", res) + } +} + diff --git a/workflow-pr/pr_processor.go b/workflow-pr/pr_processor.go index a460a5f..88e38f1 100644 --- a/workflow-pr/pr_processor.go +++ b/workflow-pr/pr_processor.go @@ -413,6 +413,12 @@ func (pr *PRProcessor) Process(req *models.PullRequest) error { } common.LogInfo("fetched PRSet of size:", len(prset.PRs)) + if !prset.PrepareForMerge(git) { + common.LogError("PRs are NOT mergeable.") + } else { + common.LogInfo("PRs are in mergeable state.") + } + prjGitPRbranch := prGitBranchNameForPR(prRepo, prNo) prjGitPR, err := prset.GetPrjGitPR() if err == common.PRSet_PrjGitMissing { -- 2.51.1 From 95632d3d7d341c779204cec99dc2ec2a15af0c6297be3a229b37dd357a288092 Mon Sep 17 00:00:00 2001 From: Adam Majer Date: Tue, 24 Feb 2026 17:48:58 +0100 Subject: [PATCH 3/3] pr: No need to try to merge changes We can reset current worktree and clobber it with the merged changes. We want to emulate `git merge -s theirs` strategy while `git merge -Xtheirs` only picks `theirs` in case of conflicts. So, resetting the changes and reading exact is sufficient `git read-tree -u` updates the current work tree so we do not have unstaged changes. --- common/pr.go | 3 +-- common/pr_test.go | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/common/pr.go b/common/pr.go index aab2472..fd52bc0 100644 --- a/common/pr.go +++ b/common/pr.go @@ -579,9 +579,8 @@ func (rs *PRSet) AddMergeCommit(git Git, remote string, pr int) bool { } // ensure only files that are in head.Sha are kept - git.GitExecOrPanic(repo.Name, "read-tree", "-m", head.Sha) + git.GitExecOrPanic(repo.Name, "read-tree", "--reset", "-u", head.Sha) git.GitExecOrPanic(repo.Name, "commit", "-m", msg) - git.GitExecOrPanic(repo.Name, "clean", "-fxd") if !IsDryRun { git.GitExecOrPanic(repo.Name, "push", remote, "HEAD:"+head.Name) diff --git a/common/pr_test.go b/common/pr_test.go index 1617a7f..0a6f8f0 100644 --- a/common/pr_test.go +++ b/common/pr_test.go @@ -1519,9 +1519,8 @@ func TestPRPrepareForMerge(t *testing.T) { // AddMergeCommit is called m.EXPECT().GitExecOrPanic("pkg", "checkout", "origin/master") m.EXPECT().GitExec("pkg", "merge", "--no-ff", "--no-commit", "-X", "theirs", pkgPR.Head.Sha).Return(nil) - m.EXPECT().GitExecOrPanic("pkg", "read-tree", "-m", pkgPR.Head.Sha) + m.EXPECT().GitExecOrPanic("pkg", "read-tree", "--reset", "-u", pkgPR.Head.Sha) m.EXPECT().GitExecOrPanic("pkg", "commit", "-m", gomock.Any()) - m.EXPECT().GitExecOrPanic("pkg", "clean", "-fxd") m.EXPECT().GitExecOrPanic("pkg", "push", "origin_fork", "HEAD:"+pkgPR.Head.Name) m.EXPECT().GitExecWithOutputOrPanic("pkg", "rev-list", "-1", "HEAD").Return("new_pkg_head_sha") // Second merge-base succeeds (after goto Verify) @@ -1730,9 +1729,8 @@ func TestPRAddMergeCommit(t *testing.T) { git := mock_common.NewMockGit(ctl) git.EXPECT().GitExec("pkg", "merge", "--no-ff", "--no-commit", "-X", "theirs", pkgPR.Head.Sha).Return(nil) - git.EXPECT().GitExecOrPanic("pkg", "read-tree", "-m", pkgPR.Head.Sha) + git.EXPECT().GitExecOrPanic("pkg", "read-tree", "--reset", "-u", pkgPR.Head.Sha) git.EXPECT().GitExecOrPanic("pkg", "commit", "-m", gomock.Any()) - git.EXPECT().GitExecOrPanic("pkg", "clean", "-fxd") git.EXPECT().GitExecOrPanic("pkg", "push", "origin", "HEAD:branch_name") git.EXPECT().GitExecWithOutputOrPanic("pkg", "rev-list", "-1", "HEAD").Return("new_head_sha") -- 2.51.1