From 444959540a32f944100144d280acdcf12ed3f3f4b1ce8ec658f3bee3abd7ba4a Mon Sep 17 00:00:00 2001 From: Adam Majer Date: Mon, 1 Sep 2025 16:39:13 +0200 Subject: [PATCH 1/3] submodule conflict resolution If merging a ProjectGit with submodules, * removed submodules are to be removed always * modified/added submodules are to be present * submodules modified in base project and PR should conflict --- common/git_parser.go | 296 +++++++++++++++++++++++++++++++++++ common/git_utils.go | 320 ++++++++++++++++---------------------- common/git_utils_test.go | 149 +++++++++++++++++- common/pr.go | 78 +--------- common/pr_test.go | 34 ++-- common/test_repo_setup.sh | 20 ++- 6 files changed, 610 insertions(+), 287 deletions(-) create mode 100644 common/git_parser.go diff --git a/common/git_parser.go b/common/git_parser.go new file mode 100644 index 0000000..91f3658 --- /dev/null +++ b/common/git_parser.go @@ -0,0 +1,296 @@ +package common + +import ( + "errors" + "fmt" + "io" +) + +const ( + GitStatus_Untracked = 0 + GitStatus_Modified = 1 + GitStatus_Ignored = 2 + GitStatus_Unmerged = 3 // States[0..3] -- Stage1, Stage2, Stage3 of merge objects + GitStatus_Renamed = 4 // orig name in States[0] +) + +type GitStatusData struct { + Path string + Status int + States [3]string + + /* + A 4 character field describing the submodule state. + "N..." when the entry is not a submodule. + "S" when the entry is a submodule. + is "C" if the commit changed; otherwise ".". + is "M" if it has tracked changes; otherwise ".". + is "U" if there are untracked changes; otherwise ".". + */ + SubmoduleChanges string +} + +func parseGit_HexString(data io.ByteReader) (string, error) { + str := make([]byte, 0, 32) + for { + c, err := data.ReadByte() + if err != nil { + return "", err + } + switch { + case c == 0 || c == ' ': + return string(str), nil + case c >= 'a' && c <= 'f': + case c >= 'A' && c <= 'F': + case c >= '0' && c <= '9': + default: + return "", errors.New("Invalid character in hex string:" + string(c)) + } + str = append(str, c) + } +} +func parseGit_String(data io.ByteReader) (string, error) { + str := make([]byte, 0, 100) + for { + c, err := data.ReadByte() + if err != nil { + return "", errors.New("Unexpected EOF. Expected NUL string term") + } + if c == 0 || c == ' ' { + return string(str), nil + } + str = append(str, c) + } +} + +func parseGit_StringWithSpace(data io.ByteReader) (string, error) { + str := make([]byte, 0, 100) + for { + c, err := data.ReadByte() + if err != nil { + return "", errors.New("Unexpected EOF. Expected NUL string term") + } + if c == 0 { + return string(str), nil + } + str = append(str, c) + } +} + +func skipGitStatusEntry(data io.ByteReader, skipSpaceLen int) error { + for skipSpaceLen > 0 { + c, err := data.ReadByte() + if err != nil { + return err + } + if c == ' ' { + skipSpaceLen-- + } + } + + return nil +} + +func parseSingleStatusEntry(data io.ByteReader) (*GitStatusData, error) { + ret := GitStatusData{} + statusType, err := data.ReadByte() + if err != nil { + return nil, nil + } + switch statusType { + case '1': + var err error + if err = skipGitStatusEntry(data, 8); err != nil { + return nil, err + } + ret.Status = GitStatus_Modified + ret.Path, err = parseGit_StringWithSpace(data) + if err != nil { + return nil, err + } + case '2': + var err error + if err = skipGitStatusEntry(data, 9); err != nil { + return nil, err + } + ret.Status = GitStatus_Renamed + ret.Path, err = parseGit_StringWithSpace(data) + if err != nil { + return nil, err + } + ret.States[0], err = parseGit_StringWithSpace(data) + if err != nil { + return nil, err + } + case '?': + var err error + if err = skipGitStatusEntry(data, 1); err != nil { + return nil, err + } + ret.Status = GitStatus_Untracked + ret.Path, err = parseGit_StringWithSpace(data) + if err != nil { + return nil, err + } + case '!': + var err error + if err = skipGitStatusEntry(data, 1); err != nil { + return nil, err + } + ret.Status = GitStatus_Ignored + ret.Path, err = parseGit_StringWithSpace(data) + if err != nil { + return nil, err + } + case 'u': + var err error + if err = skipGitStatusEntry(data, 2); err != nil { + return nil, err + } + if ret.SubmoduleChanges, err = parseGit_String(data); err != nil { + return nil, err + } + if err = skipGitStatusEntry(data, 4); err != nil { + return nil, err + } + + if ret.States[0], err = parseGit_HexString(data); err != nil { + return nil, err + } + if ret.States[1], err = parseGit_HexString(data); err != nil { + return nil, err + } + if ret.States[2], err = parseGit_HexString(data); err != nil { + return nil, err + } + ret.Status = GitStatus_Unmerged + ret.Path, err = parseGit_StringWithSpace(data) + if err != nil { + return nil, err + } + default: + return nil, errors.New("Invalid status type" + string(statusType)) + } + return &ret, nil +} + +func parseGitStatusData(data io.ByteReader) (Data, error) { + ret := make([]GitStatusData, 0, 10) + for { + data, err := parseSingleStatusEntry(data) + if err != nil { + return nil, err + } else if data == nil { + break + } + + ret = append(ret, *data) + } + return ret, nil +} + +type Data interface{} + +type CommitStatus int + +const ( + Add CommitStatus = iota + Rm + Copy + Modify + Rename + TypeChange + Unmerged + Unknown +) + +type GitDiffRawData struct { + SrcMode, DstMode string + SrcCommit, DstCommit string + Status CommitStatus + Src, Dst string +} + +func parseGit_DiffIndexStatus(data io.ByteReader, d *GitDiffRawData) error { + b, err := data.ReadByte() + if err != nil { + return err + } + + switch b { + case 'A': + d.Status = Add + case 'C': + d.Status = Copy + case 'D': + d.Status = Rm + case 'M': + d.Status = Modify + case 'R': + d.Status = Rename + case 'T': + d.Status = TypeChange + case 'U': + d.Status = Unmerged + case 'X': + return fmt.Errorf("Unexpected unknown change type. This is a git bug") + } + _, err = parseGit_StringWithSpace(data) + if err != nil { + return err + } + + return nil +} + +func parseSingleGitDiffIndexRawData(data io.ByteReader) (*GitDiffRawData, error) { + var ret GitDiffRawData + + b, err := data.ReadByte() + if err != nil { + return nil, err + } + if b != ':' { + return nil, fmt.Errorf("Expected ':' but got '%s'", string(b)) + } + + if ret.SrcMode, err = parseGit_String(data); err != nil { + return nil, err + } + if ret.DstMode, err = parseGit_String(data); err != nil { + return nil, err + } + if ret.Src, err = parseGit_String(data); err != nil { + return nil, err + } + if ret.Dst, err = parseGit_String(data); err != nil { + return nil, err + } + if err = parseGit_DiffIndexStatus(data, &ret); err != nil { + return nil, err + } + ret.Dst = ret.Src + switch ret.Status { + case Copy, Rename: + if ret.Src, err = parseGit_StringWithSpace(data); err != nil { + return nil, err + } + } + + return &ret, nil +} + +func parseGitDiffIndexRawData(data io.ByteReader) (Data, error) { + ret := make([]GitDiffRawData, 0, 10) + for { + data, err := parseSingleGitDiffIndexRawData(data) + if err != nil { + return nil, err + } else if data == nil { + break + } + + ret = append(ret, *data) + } + return ret, nil +} diff --git a/common/git_utils.go b/common/git_utils.go index bd5342a..47c64c3 100644 --- a/common/git_utils.go +++ b/common/git_utils.go @@ -19,9 +19,7 @@ package common */ import ( - "bufio" "bytes" - "errors" "fmt" "io" "os" @@ -44,6 +42,10 @@ type GitDirectoryLister interface { GitDirectoryList(gitPath, commitId string) (dirlist map[string]string, err error) } +type GitSubmoduleFileConflictResolver interface { + GitResolveSubmoduleFileConflict(cwd, MergeBase, Head, MergeHead string) error +} + type GitStatusLister interface { GitStatus(cwd string) ([]GitStatusData, error) } @@ -75,6 +77,7 @@ type Git interface { GitExecQuietOrPanic(cwd string, params ...string) GitDiffLister + GitSubmoduleFileConflictResolver } type GitHandlerImpl struct { @@ -1002,193 +1005,10 @@ func (e *GitHandlerImpl) GitSubmoduleCommitId(cwd, packageName, commitId string) return subCommitId, len(subCommitId) > 0 } -const ( - GitStatus_Untracked = 0 - GitStatus_Modified = 1 - GitStatus_Ignored = 2 - GitStatus_Unmerged = 3 // States[0..3] -- Stage1, Stage2, Stage3 of merge objects - GitStatus_Renamed = 4 // orig name in States[0] -) - -type GitStatusData struct { - Path string - Status int - States [3]string - - /* - A 4 character field describing the submodule state. - "N..." when the entry is not a submodule. - "S" when the entry is a submodule. - is "C" if the commit changed; otherwise ".". - is "M" if it has tracked changes; otherwise ".". - is "U" if there are untracked changes; otherwise ".". - */ - SubmoduleChanges string -} - -func parseGitStatusHexString(data io.ByteReader) (string, error) { - str := make([]byte, 0, 32) - for { - c, err := data.ReadByte() - if err != nil { - return "", err - } - switch { - case c == 0 || c == ' ': - return string(str), nil - case c >= 'a' && c <= 'f': - case c >= 'A' && c <= 'F': - case c >= '0' && c <= '9': - default: - return "", errors.New("Invalid character in hex string:" + string(c)) - } - str = append(str, c) - } -} -func parseGitStatusString(data io.ByteReader) (string, error) { - str := make([]byte, 0, 100) - for { - c, err := data.ReadByte() - if err != nil { - return "", errors.New("Unexpected EOF. Expected NUL string term") - } - if c == 0 || c == ' ' { - return string(str), nil - } - str = append(str, c) - } -} - -func parseGitStatusStringWithSpace(data io.ByteReader) (string, error) { - str := make([]byte, 0, 100) - for { - c, err := data.ReadByte() - if err != nil { - return "", errors.New("Unexpected EOF. Expected NUL string term") - } - if c == 0 { - return string(str), nil - } - str = append(str, c) - } -} - -func skipGitStatusEntry(data io.ByteReader, skipSpaceLen int) error { - for skipSpaceLen > 0 { - c, err := data.ReadByte() - if err != nil { - return err - } - if c == ' ' { - skipSpaceLen-- - } - } - - return nil -} - -func parseSingleStatusEntry(data io.ByteReader) (*GitStatusData, error) { - ret := GitStatusData{} - statusType, err := data.ReadByte() - if err != nil { - return nil, nil - } - switch statusType { - case '1': - var err error - if err = skipGitStatusEntry(data, 8); err != nil { - return nil, err - } - ret.Status = GitStatus_Modified - ret.Path, err = parseGitStatusStringWithSpace(data) - if err != nil { - return nil, err - } - case '2': - var err error - if err = skipGitStatusEntry(data, 9); err != nil { - return nil, err - } - ret.Status = GitStatus_Renamed - ret.Path, err = parseGitStatusStringWithSpace(data) - if err != nil { - return nil, err - } - ret.States[0], err = parseGitStatusStringWithSpace(data) - if err != nil { - return nil, err - } - case '?': - var err error - if err = skipGitStatusEntry(data, 1); err != nil { - return nil, err - } - ret.Status = GitStatus_Untracked - ret.Path, err = parseGitStatusStringWithSpace(data) - if err != nil { - return nil, err - } - case '!': - var err error - if err = skipGitStatusEntry(data, 1); err != nil { - return nil, err - } - ret.Status = GitStatus_Ignored - ret.Path, err = parseGitStatusStringWithSpace(data) - if err != nil { - return nil, err - } - case 'u': - var err error - if err = skipGitStatusEntry(data, 2); err != nil { - return nil, err - } - if ret.SubmoduleChanges, err = parseGitStatusString(data); err != nil { - return nil, err - } - if err = skipGitStatusEntry(data, 4); err != nil { - return nil, err - } - - if ret.States[0], err = parseGitStatusHexString(data); err != nil { - return nil, err - } - if ret.States[1], err = parseGitStatusHexString(data); err != nil { - return nil, err - } - if ret.States[2], err = parseGitStatusHexString(data); err != nil { - return nil, err - } - ret.Status = GitStatus_Unmerged - ret.Path, err = parseGitStatusStringWithSpace(data) - if err != nil { - return nil, err - } - default: - return nil, errors.New("Invalid status type" + string(statusType)) - } - return &ret, nil -} - -func parseGitStatusData(data io.ByteReader) ([]GitStatusData, error) { - ret := make([]GitStatusData, 0, 10) - for { - data, err := parseSingleStatusEntry(data) - if err != nil { - return nil, err - } else if data == nil { - break - } - - ret = append(ret, *data) - } - return ret, nil -} - -func (e *GitHandlerImpl) GitStatus(cwd string) (ret []GitStatusData, err error) { - LogDebug("getting git-status()") - - cmd := exec.Command("/usr/bin/git", "status", "--porcelain=2", "-z") +func (e *GitHandlerImpl) GitExecWithDataParse(cwd string, dataprocessor func(io.ByteReader) (Data, error), gitcmd string, args ...string) (Data, error) { + LogDebug("getting", gitcmd) + args = append([]string{gitcmd}, args...) + cmd := exec.Command("/usr/bin/git", args...) cmd.Env = []string{ "GIT_CEILING_DIRECTORIES=" + e.GitPath, "GIT_LFS_SKIP_SMUDGE=1", @@ -1205,7 +1025,12 @@ func (e *GitHandlerImpl) GitStatus(cwd string) (ret []GitStatusData, err error) LogError("Error running command", cmd.Args, err) } - return parseGitStatusData(bufio.NewReader(bytes.NewReader(out))) + return dataprocessor(bytes.NewReader(out)) +} + +func (e *GitHandlerImpl) GitStatus(cwd string) (ret []GitStatusData, err error) { + data, err := e.GitExecWithDataParse(cwd, parseGitStatusData, "status", "--porcelain=2", "-z") + return data.([]GitStatusData), err } func (e *GitHandlerImpl) GitDiff(cwd, base, head string) (string, error) { @@ -1230,3 +1055,118 @@ func (e *GitHandlerImpl) GitDiff(cwd, base, head string) (string, error) { return string(out), nil } + +func (e *GitHandlerImpl) GitDiffIndex(cwd, commit string) ([]GitDiffRawData, error) { + data, err := e.GitExecWithDataParse("diff-index", parseGitDiffIndexRawData, cwd, "diff-index", "-z", "--raw", "--full-index", "--submodule=short", "HEAD") + return data.([]GitDiffRawData), err +} + +func (git *GitHandlerImpl) GitResolveSubmoduleFileConflict(cwd, mergeBase, head, mergeHead string) error { + status, err := git.GitStatus(cwd) + if err != nil { + return fmt.Errorf("Status failed: %w", err) + } + + // we can only resolve conflicts with .gitmodules + for _, s := range status { + if s.Status == GitStatus_Unmerged { + if s.Path != ".gitmodules" { + return fmt.Errorf("Conflict elsewhere than submodules: %s", s.Path) + } + + submodules1, err := git.GitSubmoduleList(cwd, mergeBase) + if err != nil { + return fmt.Errorf("Failed to fetch submodules during merge resolution: %w", err) + } +/* + submodules2, err := git.GitSubmoduleList(cwd, head) + if err != nil { + return fmt.Errorf("Failed to fetch submodules during merge resolution: %w", err) + } +*/ + submodules3, err := git.GitSubmoduleList(cwd, mergeHead) + if err != nil { + return fmt.Errorf("Failed to fetch submodules during merge resolution: %w", err) + } + + // find modified submodules in the mergeHead + modifiedSubmodules := make([]string, 0, 10) + removedSubmodules := make([]string, 0, 10) + addedSubmodules := make([]string, 0, 10) + + for submodulePath, oldHash := range submodules1 { + if newHash, found := submodules3[submodulePath]; found && newHash != oldHash { + modifiedSubmodules = append(modifiedSubmodules, submodulePath) + } else if !found { + removedSubmodules = append(removedSubmodules, submodulePath) + } + } + for submodulePath, _ := range submodules3 { + if _, found := submodules1[submodulePath]; !found { + addedSubmodules = append(addedSubmodules, submodulePath) + } + } + + // We need to adjust the `submodules` list by the pending changes to the index + s1, err := git.GitExecWithOutput(cwd, "cat-file", "blob", s.States[0]) + if err != nil { + return fmt.Errorf("Failed fetching data during .gitmodules merge resoulution: %w", err) + } + s2, err := git.GitExecWithOutput(cwd, "cat-file", "blob", s.States[1]) + if err != nil { + return fmt.Errorf("Failed fetching data during .gitmodules merge resoulution: %w", err) + } + s3, err := git.GitExecWithOutput(cwd, "cat-file", "blob", s.States[2]) + if err != nil { + return fmt.Errorf("Failed fetching data during .gitmodules merge resoulution: %w", err) + } + + _, err = ParseSubmodulesFile(strings.NewReader(s1)) + if err != nil { + return fmt.Errorf("Failed parsing submodule file [%s] in merge: %w", s.States[0], err) + } + subs2, err := ParseSubmodulesFile(strings.NewReader(s2)) + if err != nil { + return fmt.Errorf("Failed parsing submodule file [%s] in merge: %w", s.States[0], err) + } + subs3, err := ParseSubmodulesFile(strings.NewReader(s3)) + if err != nil { + return fmt.Errorf("Failed parsing submodule file [%s] in merge: %w", s.States[0], err) + } + + overrideSet := make([]Submodule, 0, len(addedSubmodules)+len(modifiedSubmodules)) + for i := range subs3 { + if slices.Contains(addedSubmodules, subs3[i].Path) || slices.Contains(modifiedSubmodules, subs3[i].Path) { + overrideSet = append(overrideSet, subs3[i]) + } + } + // merge from subs1 (merge-base), subs2 (changes in base since merge-base HEAD), subs3 (merge_source MERGE_HEAD) + // this will update submodules + SubmoduleCompare := func(a, b Submodule) int { return strings.Compare(a.Path, b.Path) } + CompactCompare := func(a, b Submodule) bool { return a.Path == b.Path } + + // remove submodules that are removed in the PR + subs2 = slices.DeleteFunc(subs2, func(a Submodule) bool { return slices.Contains(removedSubmodules, a.Path) }) + + mergedSubs := slices.Concat(overrideSet, subs2) + slices.SortStableFunc(mergedSubs, SubmoduleCompare) + filteredSubs := slices.CompactFunc(mergedSubs, CompactCompare) + + out, err := os.Create(path.Join(git.GetPath(), cwd, ".gitmodules")) + if err != nil { + return fmt.Errorf("Can't open .gitmodules for writing: %w", err) + } + if err = WriteSubmodules(filteredSubs, out); err != nil { + return fmt.Errorf("Can't write .gitmodules: %w", err) + } + if out.Close(); err != nil { + return fmt.Errorf("Can't close .gitmodules: %w", err) + } + + git.GitExecOrPanic(cwd, "add", ".gitmodules") + git.GitExecOrPanic(cwd, "-c", "core.editor=true", "merge", "--continue") + } + } + + return nil +} diff --git a/common/git_utils_test.go b/common/git_utils_test.go index 342b3b4..d3befdf 100644 --- a/common/git_utils_test.go +++ b/common/git_utils_test.go @@ -24,6 +24,7 @@ import ( "os" "os/exec" "path" + "runtime/debug" "slices" "strings" "testing" @@ -93,6 +94,145 @@ func TestGitClone(t *testing.T) { } } +func TestSubmoduleConflictResolution(t *testing.T) { + tests := []struct { + name string + checkout, merge string + result string + merge_fail bool + }{ + { + name: "adding two submodules", + checkout: "base_add_b1", + merge: "base_add_b2", + result: `[submodule "pkgA"] + path = pkgA + url = ../pkgA +[submodule "pkgB"] + path = pkgB + url = ../pkgB +[submodule "pkgB1"] + path = pkgB1 + url = ../pkgB1 +[submodule "pkgB2"] + path = pkgB2 + url = ../pkgB2 +[submodule "pkgC"] + path = pkgC + url = ../pkgC +`, + }, + { + name: "remove one module and add another", + checkout: "base_rm_c", + merge: "base_add_b2", + result: `[submodule "pkgA"] + path = pkgA + url = ../pkgA +[submodule "pkgB"] + path = pkgB + url = ../pkgB +[submodule "pkgB2"] + path = pkgB2 + url = ../pkgB2 +`, + }, + { + name: "add one and remove another", + checkout: "base_add_b2", + merge: "base_rm_c", + result: `[submodule "pkgA"] + path = pkgA + url = ../pkgA +[submodule "pkgB"] + path = pkgB + url = ../pkgB +[submodule "pkgB2"] + path = pkgB2 + url = ../pkgB2 +`, + }, + { + name: "rm modified submodule", + checkout: "base_modify_c", + merge: "base_rm_c", + merge_fail: true, + }, + { + name: "modified removed submodule", + checkout: "base_rm_c", + merge: "base_modify_c", + merge_fail: true, + }, + } + + d, err := os.MkdirTemp(os.TempDir(), "submoduletests") + if err != nil { + t.Fatal(err) + } + cwd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + cmd := exec.Command(cwd + "/test_repo_setup.sh") + cmd.Dir = d + _, err = cmd.Output() + if err != nil { + t.Fatal(err) + } + + gh, err := AllocateGitWorkTree(d, "test", "foo@example.com") + if err != nil { + t.Fatal(err) + } + + success := true + noErrorOrFail := func(t *testing.T, err error) { + if err != nil { + t.Fatal(string(debug.Stack()), err) + } + } + for _, test := range tests { + success = t.Run(test.name, func(t *testing.T) { + git, err := gh.ReadExistingPath("prjgit") + defer git.Close() + + if err != nil { + t.Fatal(err) + } + noErrorOrFail(t, git.GitExec("", "reset", "--hard")) + noErrorOrFail(t, git.GitExec("", "checkout", "-B", "test", test.checkout)) + // noErrorOrFail(t, git.GitExec("", "merge", test.checkout)) + err = git.GitExec("", "merge", test.merge) + if err == nil { + t.Fatal("expected a conflict") + } + err = git.GitResolveSubmoduleFileConflict("", "main", test.checkout, test.merge) + if err != nil { + if test.merge_fail { + return // success + } + t.Fatal(err) + } + if test.merge_fail { + t.Fatal("Expected fail but succeeded?") + } + data, err := os.ReadFile(git.GetPath() + "/.gitmodules") + if err != nil { + t.Fatal("Cannot read .gitmodules.", err) + } + + if string(data) != test.result { + t.Error("Expected", len(test.result), test.result, "but have", len(data), string(data)) + } + }) && success + } + + if success { + os.RemoveAll(d) + } +} + func TestGitMsgParsing(t *testing.T) { t.Run("tree message with size 56", func(t *testing.T) { const hdr = "f40888ea4515fe2e8eea617a16f5f50a45f652d894de3ad181d58de3aafb8f98 tree 56\x00" @@ -584,12 +724,15 @@ func TestGitStatusParse(t *testing.T) { if err != nil { t.Fatal(err) } - if len(r) != len(test.res) { - t.Fatal("len(r):", len(r), "is not expected", len(test.res)) + + res := r.([]GitStatusData) + + if len(res) != len(test.res) { + t.Fatal("len(r):", len(res), "is not expected", len(test.res)) } for _, expected := range test.res { - if !slices.Contains(r, expected) { + if !slices.Contains(res, expected) { t.Fatal("result", r, "doesn't contains expected", expected) } } diff --git a/common/pr.go b/common/pr.go index 5cd11f9..e5a7aa3 100644 --- a/common/pr.go +++ b/common/pr.go @@ -4,8 +4,6 @@ import ( "bufio" "errors" "fmt" - "os" - "path" "slices" "strings" @@ -433,80 +431,8 @@ func (rs *PRSet) Merge(gitea GiteaReviewUnrequester, git Git) error { err = git.GitExec(DefaultGitPrj, "merge", "--no-ff", "-m", msg, prjgit.Head.Sha) if err != nil { - status, statusErr := git.GitStatus(DefaultGitPrj) - if statusErr != nil { - return fmt.Errorf("Failed to merge: %w . Status also failed: %w", err, statusErr) - } - - // we can only resolve conflicts with .gitmodules - for _, s := range status { - if s.Status == GitStatus_Unmerged { - panic("Can't handle conflicts yet") - if s.Path != ".gitmodules" { - return err - } - - submodules, err := git.GitSubmoduleList(DefaultGitPrj, "MERGE_HEAD") - if err != nil { - return fmt.Errorf("Failed to fetch submodules during merge resolution: %w", err) - } - s1, err := git.GitExecWithOutput(DefaultGitPrj, "cat-file", "blob", s.States[0]) - if err != nil { - return fmt.Errorf("Failed fetching data during .gitmodules merge resoulution: %w", err) - } - s2, err := git.GitExecWithOutput(DefaultGitPrj, "cat-file", "blob", s.States[1]) - if err != nil { - return fmt.Errorf("Failed fetching data during .gitmodules merge resoulution: %w", err) - } - s3, err := git.GitExecWithOutput(DefaultGitPrj, "cat-file", "blob", s.States[2]) - if err != nil { - return fmt.Errorf("Failed fetching data during .gitmodules merge resoulution: %w", err) - } - - subs1, err := ParseSubmodulesFile(strings.NewReader(s1)) - if err != nil { - return fmt.Errorf("Failed parsing submodule file [%s] in merge: %w", s.States[0], err) - } - subs2, err := ParseSubmodulesFile(strings.NewReader(s2)) - if err != nil { - return fmt.Errorf("Failed parsing submodule file [%s] in merge: %w", s.States[0], err) - } - subs3, err := ParseSubmodulesFile(strings.NewReader(s3)) - if err != nil { - return fmt.Errorf("Failed parsing submodule file [%s] in merge: %w", s.States[0], err) - } - - // merge from subs3 (target), subs1 (orig), subs2 (2-nd base that is missing from target base) - // this will update submodules - mergedSubs := slices.Concat(subs1, subs2, subs3) - - var filteredSubs []Submodule = make([]Submodule, 0, max(len(subs1), len(subs2), len(subs3))) - nextSub: - for subName := range submodules { - - for i := range mergedSubs { - if path.Base(mergedSubs[i].Path) == subName { - filteredSubs = append(filteredSubs, mergedSubs[i]) - continue nextSub - } - } - return fmt.Errorf("Cannot find submodule for path: %s", subName) - } - - out, err := os.Create(path.Join(git.GetPath(), DefaultGitPrj, ".gitmodules")) - if err != nil { - return fmt.Errorf("Can't open .gitmodules for writing: %w", err) - } - if err = WriteSubmodules(filteredSubs, out); err != nil { - return fmt.Errorf("Can't write .gitmodules: %w", err) - } - if out.Close(); err != nil { - return fmt.Errorf("Can't close .gitmodules: %w", err) - } - - git.GitExecOrPanic(DefaultGitPrj, "add", ".gitmodules") - git.GitExecOrPanic(DefaultGitPrj, "-c", "core.editor=true", "merge", "--continue") - } + if resolveError := git.GitResolveSubmoduleFileConflict(DefaultGitPrj, prjgit.MergeBase, prjgit.Base.Sha, prjgit.Head.Sha); resolveError != nil { + return fmt.Errorf("Merge failed. (%w): %w", err, resolveError) } } diff --git a/common/pr_test.go b/common/pr_test.go index c59dde3..3fadb2e 100644 --- a/common/pr_test.go +++ b/common/pr_test.go @@ -15,22 +15,23 @@ import ( "src.opensuse.org/autogits/common/gitea-generated/models" mock_common "src.opensuse.org/autogits/common/mock" ) -/* -func TestCockpit(t *testing.T) { - common.SetLoggingLevel(common.LogLevelDebug) - gitea := common.AllocateGiteaTransport("https://src.opensuse.org") - tl, err := gitea.GetTimeline("cockpit", "cockpit", 29) - if err != nil { - t.Fatal("Fail to timeline", err) - } - t.Log(tl) - r, err := common.FetchGiteaReviews(gitea, []string{}, "cockpit", "cockpit", 29) - if err != nil { - t.Fatal("Error:", err) - } - t.Error(r) -} +/* + func TestCockpit(t *testing.T) { + common.SetLoggingLevel(common.LogLevelDebug) + gitea := common.AllocateGiteaTransport("https://src.opensuse.org") + tl, err := gitea.GetTimeline("cockpit", "cockpit", 29) + if err != nil { + t.Fatal("Fail to timeline", err) + } + t.Log(tl) + r, err := common.FetchGiteaReviews(gitea, []string{}, "cockpit", "cockpit", 29) + if err != nil { + t.Fatal("Error:", err) + } + + t.Error(r) + } */ func reviewsToTimeline(reviews []*models.PullReview) []*models.TimelineComment { timeline := make([]*models.TimelineComment, len(reviews)) @@ -940,6 +941,7 @@ func TestPRMerge(t *testing.T) { pr: &models.PullRequest{ Base: &models.PRBranchInfo{ Sha: "e8b0de43d757c96a9d2c7101f4bff404e322f53a1fa4041fb85d646110c38ad4", // "base_add_b1" + Name: "master", Repo: &models.Repository{ Name: "prj", Owner: &models.User{ @@ -960,6 +962,7 @@ func TestPRMerge(t *testing.T) { pr: &models.PullRequest{ Base: &models.PRBranchInfo{ Sha: "4fbd1026b2d7462ebe9229a49100c11f1ad6555520a21ba515122d8bc41328a8", + Name: "master", Repo: &models.Repository{ Name: "prj", Owner: &models.User{ @@ -979,6 +982,7 @@ func TestPRMerge(t *testing.T) { t.Run(test.name, func(t *testing.T) { ctl := gomock.NewController(t) mock := mock_common.NewMockGiteaPRTimelineFetcher(ctl) + reviewUnrequestMock := mock_common.NewMockGiteaReviewUnrequester(ctl) reviewUnrequestMock.EXPECT().UnrequestReview(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) diff --git a/common/test_repo_setup.sh b/common/test_repo_setup.sh index be58c22..0835ef5 100755 --- a/common/test_repo_setup.sh +++ b/common/test_repo_setup.sh @@ -40,10 +40,24 @@ create_prjgit_sample() { git submodule -q add ../pkgB2 pkgB2 git commit -q -m "pkgB2 added" - git checkout main + git checkout -b base_rm_c main git clean -ffxd - git submodule -q add -f ../pkgB1 pkgB1 - git commit -q -m "main adding pkgB1" + git rm pkgC + git commit -q -m 'pkgC removed' + + git checkout -b base_modify_c main + git submodule update --init pkgC + pushd pkgC + echo "mofieid" >> README.md + git commit -q -m "modified" README.md + popd + git commit pkgC -m "modifiedC" + git submodule deinit -f pkgC + +# git checkout main +# git clean -ffxd +# git submodule -q add -f ../pkgB1 pkgB1 +# git commit -q -m "main adding pkgB1" popd } -- 2.51.1 From d5d69109061c4afec174bf7defb8ab072a861b6bd03b1d7c5b20c59f13c2ab6b Mon Sep 17 00:00:00 2001 From: Adam Majer Date: Wed, 29 Oct 2025 13:01:25 +0100 Subject: [PATCH 2/3] Add command line utility to automerge conflicts It assumes CWD is the root of the git project --- .gitignore | 2 + autogits.spec | 6 + common/git_utils.go | 212 ++++++++++++++------------- common/git_utils_test.go | 2 +- common/pr.go | 2 +- utils/gitmodules-automerge/README.md | 9 ++ utils/gitmodules-automerge/main.go | 42 ++++++ 7 files changed, 173 insertions(+), 102 deletions(-) create mode 100644 utils/gitmodules-automerge/README.md create mode 100644 utils/gitmodules-automerge/main.go diff --git a/.gitignore b/.gitignore index 86763d8..805b4be 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ *.osc *.conf +utils/gitmodules-automerge/gitmodules-automerge +utils/hujson/hujson diff --git a/autogits.spec b/autogits.spec index 56264f4..a1a1c64 100644 --- a/autogits.spec +++ b/autogits.spec @@ -96,6 +96,7 @@ Provides: /usr/bin/hujson %description utils HuJSON to JSON parser, using stdin -> stdout pipe +gitmodules-automerge fixes conflicts in .gitmodules conflicts during merge %package workflow-direct @@ -127,6 +128,9 @@ go build \ go build \ -C utils/hujson \ -buildmode=pie +go build \ + -C utils/gitmodules-automerge \ + -buildmode=pie go build \ -C gitea-events-rabbitmq-publisher \ -buildmode=pie @@ -175,6 +179,7 @@ install -D -m0644 systemd/obs-status-service.service install -D -m0755 workflow-direct/workflow-direct %{buildroot}%{_bindir}/workflow-direct install -D -m0755 workflow-pr/workflow-pr %{buildroot}%{_bindir}/workflow-pr install -D -m0755 utils/hujson/hujson %{buildroot}%{_bindir}/hujson +install -D -m0755 utils/hujson/gitmodules-automerge %{buildroot}%{_bindir}/gitmodules-automerge %pre gitea-events-rabbitmq-publisher %service_add_pre gitea-events-rabbitmq-publisher.service @@ -256,6 +261,7 @@ install -D -m0755 utils/hujson/hujson %files utils %license COPYING %{_bindir}/hujson +%{_bindir}/gitmodules-automerge %files workflow-direct %license COPYING diff --git a/common/git_utils.go b/common/git_utils.go index 47c64c3..47fd52e 100644 --- a/common/git_utils.go +++ b/common/git_utils.go @@ -43,7 +43,8 @@ type GitDirectoryLister interface { } type GitSubmoduleFileConflictResolver interface { - GitResolveSubmoduleFileConflict(cwd, MergeBase, Head, MergeHead string) error + GitResolveConflicts(cwd, MergeBase, Head, MergeHead string) error + GitResolveSubmoduleFileConflict(s GitStatusData, cwd, mergeBase, head, mergeHead string) error } type GitStatusLister interface { @@ -356,13 +357,20 @@ func (e *GitHandlerImpl) GitExecWithOutput(cwd string, params ...string) (string cmd.Env = []string{ "GIT_CEILING_DIRECTORIES=" + e.GitPath, "GIT_CONFIG_GLOBAL=/dev/null", - "GIT_AUTHOR_NAME=" + e.GitCommiter, - "GIT_COMMITTER_NAME=" + e.GitCommiter, - "EMAIL=not@exist@src.opensuse.org", "GIT_LFS_SKIP_SMUDGE=1", "GIT_LFS_SKIP_PUSH=1", "GIT_SSH_COMMAND=/usr/bin/ssh -o StrictHostKeyChecking=yes", } + if len(e.GitEmail) > 0 { + cmd.Env = append(cmd.Env, "EMAIL="+e.GitEmail) + } + if len(e.GitCommiter) > 0 { + cmd.Env = append(cmd.Env, + "GIT_AUTHOR_NAME="+e.GitCommiter, + "GIT_COMMITTER_NAME="+e.GitCommiter, + ) + } + if len(ExtraGitParams) > 0 { cmd.Env = append(cmd.Env, ExtraGitParams...) } @@ -1061,7 +1069,7 @@ func (e *GitHandlerImpl) GitDiffIndex(cwd, commit string) ([]GitDiffRawData, err return data.([]GitDiffRawData), err } -func (git *GitHandlerImpl) GitResolveSubmoduleFileConflict(cwd, mergeBase, head, mergeHead string) error { +func (git *GitHandlerImpl) GitResolveConflicts(cwd, mergeBase, head, mergeHead string) error { status, err := git.GitStatus(cwd) if err != nil { return fmt.Errorf("Status failed: %w", err) @@ -1069,104 +1077,108 @@ func (git *GitHandlerImpl) GitResolveSubmoduleFileConflict(cwd, mergeBase, head, // we can only resolve conflicts with .gitmodules for _, s := range status { - if s.Status == GitStatus_Unmerged { - if s.Path != ".gitmodules" { - return fmt.Errorf("Conflict elsewhere than submodules: %s", s.Path) + if s.Status == GitStatus_Unmerged && s.Path == ".gitmodules" { + if err := git.GitResolveSubmoduleFileConflict(s, cwd, mergeBase, head, mergeHead); err != nil { + return err } - - submodules1, err := git.GitSubmoduleList(cwd, mergeBase) - if err != nil { - return fmt.Errorf("Failed to fetch submodules during merge resolution: %w", err) - } -/* - submodules2, err := git.GitSubmoduleList(cwd, head) - if err != nil { - return fmt.Errorf("Failed to fetch submodules during merge resolution: %w", err) - } -*/ - submodules3, err := git.GitSubmoduleList(cwd, mergeHead) - if err != nil { - return fmt.Errorf("Failed to fetch submodules during merge resolution: %w", err) - } - - // find modified submodules in the mergeHead - modifiedSubmodules := make([]string, 0, 10) - removedSubmodules := make([]string, 0, 10) - addedSubmodules := make([]string, 0, 10) - - for submodulePath, oldHash := range submodules1 { - if newHash, found := submodules3[submodulePath]; found && newHash != oldHash { - modifiedSubmodules = append(modifiedSubmodules, submodulePath) - } else if !found { - removedSubmodules = append(removedSubmodules, submodulePath) - } - } - for submodulePath, _ := range submodules3 { - if _, found := submodules1[submodulePath]; !found { - addedSubmodules = append(addedSubmodules, submodulePath) - } - } - - // We need to adjust the `submodules` list by the pending changes to the index - s1, err := git.GitExecWithOutput(cwd, "cat-file", "blob", s.States[0]) - if err != nil { - return fmt.Errorf("Failed fetching data during .gitmodules merge resoulution: %w", err) - } - s2, err := git.GitExecWithOutput(cwd, "cat-file", "blob", s.States[1]) - if err != nil { - return fmt.Errorf("Failed fetching data during .gitmodules merge resoulution: %w", err) - } - s3, err := git.GitExecWithOutput(cwd, "cat-file", "blob", s.States[2]) - if err != nil { - return fmt.Errorf("Failed fetching data during .gitmodules merge resoulution: %w", err) - } - - _, err = ParseSubmodulesFile(strings.NewReader(s1)) - if err != nil { - return fmt.Errorf("Failed parsing submodule file [%s] in merge: %w", s.States[0], err) - } - subs2, err := ParseSubmodulesFile(strings.NewReader(s2)) - if err != nil { - return fmt.Errorf("Failed parsing submodule file [%s] in merge: %w", s.States[0], err) - } - subs3, err := ParseSubmodulesFile(strings.NewReader(s3)) - if err != nil { - return fmt.Errorf("Failed parsing submodule file [%s] in merge: %w", s.States[0], err) - } - - overrideSet := make([]Submodule, 0, len(addedSubmodules)+len(modifiedSubmodules)) - for i := range subs3 { - if slices.Contains(addedSubmodules, subs3[i].Path) || slices.Contains(modifiedSubmodules, subs3[i].Path) { - overrideSet = append(overrideSet, subs3[i]) - } - } - // merge from subs1 (merge-base), subs2 (changes in base since merge-base HEAD), subs3 (merge_source MERGE_HEAD) - // this will update submodules - SubmoduleCompare := func(a, b Submodule) int { return strings.Compare(a.Path, b.Path) } - CompactCompare := func(a, b Submodule) bool { return a.Path == b.Path } - - // remove submodules that are removed in the PR - subs2 = slices.DeleteFunc(subs2, func(a Submodule) bool { return slices.Contains(removedSubmodules, a.Path) }) - - mergedSubs := slices.Concat(overrideSet, subs2) - slices.SortStableFunc(mergedSubs, SubmoduleCompare) - filteredSubs := slices.CompactFunc(mergedSubs, CompactCompare) - - out, err := os.Create(path.Join(git.GetPath(), cwd, ".gitmodules")) - if err != nil { - return fmt.Errorf("Can't open .gitmodules for writing: %w", err) - } - if err = WriteSubmodules(filteredSubs, out); err != nil { - return fmt.Errorf("Can't write .gitmodules: %w", err) - } - if out.Close(); err != nil { - return fmt.Errorf("Can't close .gitmodules: %w", err) - } - - git.GitExecOrPanic(cwd, "add", ".gitmodules") - git.GitExecOrPanic(cwd, "-c", "core.editor=true", "merge", "--continue") + } else if s.Status == GitStatus_Unmerged { + return fmt.Errorf("Cannot automatically resolve conflict: %s", s.Path) } } + return git.GitExec(cwd, "-c", "core.editor=true", "merge", "--continue") +} + +func (git *GitHandlerImpl) GitResolveSubmoduleFileConflict(s GitStatusData, cwd, mergeBase, head, mergeHead string) error { + submodules1, err := git.GitSubmoduleList(cwd, mergeBase) + if err != nil { + return fmt.Errorf("Failed to fetch submodules during merge resolution: %w", err) + } + /* + submodules2, err := git.GitSubmoduleList(cwd, head) + if err != nil { + return fmt.Errorf("Failed to fetch submodules during merge resolution: %w", err) + } + */ + submodules3, err := git.GitSubmoduleList(cwd, mergeHead) + if err != nil { + return fmt.Errorf("Failed to fetch submodules during merge resolution: %w", err) + } + + // find modified submodules in the mergeHead + modifiedSubmodules := make([]string, 0, 10) + removedSubmodules := make([]string, 0, 10) + addedSubmodules := make([]string, 0, 10) + + for submodulePath, oldHash := range submodules1 { + if newHash, found := submodules3[submodulePath]; found && newHash != oldHash { + modifiedSubmodules = append(modifiedSubmodules, submodulePath) + } else if !found { + removedSubmodules = append(removedSubmodules, submodulePath) + } + } + for submodulePath, _ := range submodules3 { + if _, found := submodules1[submodulePath]; !found { + addedSubmodules = append(addedSubmodules, submodulePath) + } + } + + // We need to adjust the `submodules` list by the pending changes to the index + s1, err := git.GitExecWithOutput(cwd, "cat-file", "blob", s.States[0]) + if err != nil { + return fmt.Errorf("Failed fetching data during .gitmodules merge resoulution: %w", err) + } + s2, err := git.GitExecWithOutput(cwd, "cat-file", "blob", s.States[1]) + if err != nil { + return fmt.Errorf("Failed fetching data during .gitmodules merge resoulution: %w", err) + } + s3, err := git.GitExecWithOutput(cwd, "cat-file", "blob", s.States[2]) + if err != nil { + return fmt.Errorf("Failed fetching data during .gitmodules merge resoulution: %w", err) + } + + _, err = ParseSubmodulesFile(strings.NewReader(s1)) + if err != nil { + return fmt.Errorf("Failed parsing submodule file [%s] in merge: %w", s.States[0], err) + } + subs2, err := ParseSubmodulesFile(strings.NewReader(s2)) + if err != nil { + return fmt.Errorf("Failed parsing submodule file [%s] in merge: %w", s.States[0], err) + } + subs3, err := ParseSubmodulesFile(strings.NewReader(s3)) + if err != nil { + return fmt.Errorf("Failed parsing submodule file [%s] in merge: %w", s.States[0], err) + } + + overrideSet := make([]Submodule, 0, len(addedSubmodules)+len(modifiedSubmodules)) + for i := range subs3 { + if slices.Contains(addedSubmodules, subs3[i].Path) || slices.Contains(modifiedSubmodules, subs3[i].Path) { + overrideSet = append(overrideSet, subs3[i]) + } + } + // merge from subs1 (merge-base), subs2 (changes in base since merge-base HEAD), subs3 (merge_source MERGE_HEAD) + // this will update submodules + SubmoduleCompare := func(a, b Submodule) int { return strings.Compare(a.Path, b.Path) } + CompactCompare := func(a, b Submodule) bool { return a.Path == b.Path } + + // remove submodules that are removed in the PR + subs2 = slices.DeleteFunc(subs2, func(a Submodule) bool { return slices.Contains(removedSubmodules, a.Path) }) + + mergedSubs := slices.Concat(overrideSet, subs2) + slices.SortStableFunc(mergedSubs, SubmoduleCompare) + filteredSubs := slices.CompactFunc(mergedSubs, CompactCompare) + + out, err := os.Create(path.Join(git.GetPath(), cwd, ".gitmodules")) + if err != nil { + return fmt.Errorf("Can't open .gitmodules for writing: %w", err) + } + if err = WriteSubmodules(filteredSubs, out); err != nil { + return fmt.Errorf("Can't write .gitmodules: %w", err) + } + if out.Close(); err != nil { + return fmt.Errorf("Can't close .gitmodules: %w", err) + } + + git.GitExecOrPanic(cwd, "add", ".gitmodules") return nil } diff --git a/common/git_utils_test.go b/common/git_utils_test.go index d3befdf..b13a540 100644 --- a/common/git_utils_test.go +++ b/common/git_utils_test.go @@ -207,7 +207,7 @@ func TestSubmoduleConflictResolution(t *testing.T) { if err == nil { t.Fatal("expected a conflict") } - err = git.GitResolveSubmoduleFileConflict("", "main", test.checkout, test.merge) + err = git.GitResolveConflicts("", "main", test.checkout, test.merge) if err != nil { if test.merge_fail { return // success diff --git a/common/pr.go b/common/pr.go index e5a7aa3..3391a2e 100644 --- a/common/pr.go +++ b/common/pr.go @@ -431,7 +431,7 @@ func (rs *PRSet) Merge(gitea GiteaReviewUnrequester, git Git) error { err = git.GitExec(DefaultGitPrj, "merge", "--no-ff", "-m", msg, prjgit.Head.Sha) if err != nil { - if resolveError := git.GitResolveSubmoduleFileConflict(DefaultGitPrj, prjgit.MergeBase, prjgit.Base.Sha, prjgit.Head.Sha); resolveError != nil { + if resolveError := git.GitResolveConflicts(DefaultGitPrj, prjgit.MergeBase, prjgit.Base.Sha, prjgit.Head.Sha); resolveError != nil { return fmt.Errorf("Merge failed. (%w): %w", err, resolveError) } } diff --git a/utils/gitmodules-automerge/README.md b/utils/gitmodules-automerge/README.md new file mode 100644 index 0000000..c31057f --- /dev/null +++ b/utils/gitmodules-automerge/README.md @@ -0,0 +1,9 @@ +Purpose +------- + +Automatically resolve git configlicts in .gitmodules if there's a conflict +due to a merge. + +It uses HEAD and MERGE_HEAD to calculate merge base and pass it to the +conflict resolution + diff --git a/utils/gitmodules-automerge/main.go b/utils/gitmodules-automerge/main.go new file mode 100644 index 0000000..c202efd --- /dev/null +++ b/utils/gitmodules-automerge/main.go @@ -0,0 +1,42 @@ +package main + +import ( + "os" + "strings" + + "src.opensuse.org/autogits/common" +) + +func main() { + cwd, err := os.Getwd() + if err != nil { + common.LogError(err) + return + } + gh, err := common.AllocateGitWorkTree(cwd, "", "") + if err != nil { + common.LogError(err) + return + } + + git, err := gh.ReadExistingPath("") + if err != nil { + common.LogError(err) + return + } + + MergeBase := strings.TrimSpace(git.GitExecWithOutputOrPanic("", "merge-base", "HEAD", "MERGE_HEAD")) + status, err := git.GitStatus("") + if err != nil { + common.LogError(err) + return + } + + for _, s := range status { + if s.Path == ".gitmodules" && s.Status == common.GitStatus_Unmerged { + if err := git.GitResolveSubmoduleFileConflict(s, "", MergeBase, "HEAD", "MERGE_HEAD"); err != nil { + common.LogError(err) + } + } + } +} -- 2.51.1 From 66a2c0565efa981542805c80d14393155b94242107b685bbb77cddfaedc9b7c0 Mon Sep 17 00:00:00 2001 From: Adam Majer Date: Wed, 29 Oct 2025 13:26:16 +0100 Subject: [PATCH 3/3] utils: specfile fix --- autogits.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/autogits.spec b/autogits.spec index a1a1c64..2a80251 100644 --- a/autogits.spec +++ b/autogits.spec @@ -179,7 +179,7 @@ install -D -m0644 systemd/obs-status-service.service install -D -m0755 workflow-direct/workflow-direct %{buildroot}%{_bindir}/workflow-direct install -D -m0755 workflow-pr/workflow-pr %{buildroot}%{_bindir}/workflow-pr install -D -m0755 utils/hujson/hujson %{buildroot}%{_bindir}/hujson -install -D -m0755 utils/hujson/gitmodules-automerge %{buildroot}%{_bindir}/gitmodules-automerge +install -D -m0755 utils/gitmodules-automerge/gitmodules-automerge %{buildroot}%{_bindir}/gitmodules-automerge %pre gitea-events-rabbitmq-publisher %service_add_pre gitea-events-rabbitmq-publisher.service -- 2.51.1