forked from git-workflow/autogits
Compare commits
5 Commits
new_state
...
git-deadlo
| Author | SHA256 | Date | |
|---|---|---|---|
| 35558ef272 | |||
| 51b0487b29 | |||
| 49e32c0ab1 | |||
| 01e4f5f59e | |||
| 19d9fc5f1e |
@@ -391,12 +391,17 @@ func (e *GitHandlerImpl) GitExecQuietOrPanic(cwd string, params ...string) {
|
||||
}
|
||||
|
||||
type ChanIO struct {
|
||||
ch chan byte
|
||||
ch chan byte
|
||||
done chan struct{}
|
||||
}
|
||||
|
||||
func (c *ChanIO) Write(p []byte) (int, error) {
|
||||
for _, b := range p {
|
||||
c.ch <- b
|
||||
select {
|
||||
case c.ch <- b:
|
||||
case <-c.done:
|
||||
return 0, io.EOF
|
||||
}
|
||||
}
|
||||
return len(p), nil
|
||||
}
|
||||
@@ -405,21 +410,32 @@ func (c *ChanIO) Write(p []byte) (int, error) {
|
||||
func (c *ChanIO) Read(data []byte) (idx int, err error) {
|
||||
var ok bool
|
||||
|
||||
data[idx], ok = <-c.ch
|
||||
if !ok {
|
||||
err = io.EOF
|
||||
return
|
||||
}
|
||||
idx++
|
||||
|
||||
for len(c.ch) > 0 && idx < len(data) {
|
||||
data[idx], ok = <-c.ch
|
||||
select {
|
||||
case data[idx], ok = <-c.ch:
|
||||
if !ok {
|
||||
err = io.EOF
|
||||
return
|
||||
}
|
||||
|
||||
idx++
|
||||
case <-c.done:
|
||||
err = io.EOF
|
||||
return
|
||||
}
|
||||
|
||||
for len(c.ch) > 0 && idx < len(data) {
|
||||
select {
|
||||
case data[idx], ok = <-c.ch:
|
||||
if !ok {
|
||||
err = io.EOF
|
||||
return
|
||||
}
|
||||
idx++
|
||||
case <-c.done:
|
||||
err = io.EOF
|
||||
return
|
||||
default:
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
return
|
||||
@@ -466,7 +482,14 @@ func parseGitMsg(data <-chan byte) (GitMsg, error) {
|
||||
var size int
|
||||
|
||||
pos := 0
|
||||
for c := <-data; c != ' '; c = <-data {
|
||||
for {
|
||||
c, ok := <-data
|
||||
if !ok {
|
||||
return GitMsg{}, io.EOF
|
||||
}
|
||||
if c == ' ' {
|
||||
break
|
||||
}
|
||||
if (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') {
|
||||
id[pos] = c
|
||||
pos++
|
||||
@@ -478,7 +501,15 @@ func parseGitMsg(data <-chan byte) (GitMsg, error) {
|
||||
|
||||
pos = 0
|
||||
var c byte
|
||||
for c = <-data; c != ' ' && c != '\x00'; c = <-data {
|
||||
for {
|
||||
var ok bool
|
||||
c, ok = <-data
|
||||
if !ok {
|
||||
return GitMsg{}, io.EOF
|
||||
}
|
||||
if c == ' ' || c == '\x00' {
|
||||
break
|
||||
}
|
||||
if c >= 'a' && c <= 'z' {
|
||||
msgType[pos] = c
|
||||
pos++
|
||||
@@ -504,7 +535,14 @@ func parseGitMsg(data <-chan byte) (GitMsg, error) {
|
||||
return GitMsg{}, fmt.Errorf("Invalid object type: '%s'", string(msgType))
|
||||
}
|
||||
|
||||
for c = <-data; c != '\000'; c = <-data {
|
||||
for {
|
||||
c, ok := <-data
|
||||
if !ok {
|
||||
return GitMsg{}, io.EOF
|
||||
}
|
||||
if c == '\x00' {
|
||||
break
|
||||
}
|
||||
if c >= '0' && c <= '9' {
|
||||
size = size*10 + (int(c) - '0')
|
||||
} else {
|
||||
@@ -523,18 +561,37 @@ func parseGitCommitHdr(oldHdr [2]string, data <-chan byte) ([2]string, int, erro
|
||||
hdr := make([]byte, 0, 60)
|
||||
val := make([]byte, 0, 1000)
|
||||
|
||||
c := <-data
|
||||
c, ok := <-data
|
||||
if !ok {
|
||||
return [2]string{}, 0, io.EOF
|
||||
}
|
||||
size := 1
|
||||
if c != '\n' { // end of header marker
|
||||
for ; c != ' '; c = <-data {
|
||||
for {
|
||||
if c == ' ' {
|
||||
break
|
||||
}
|
||||
hdr = append(hdr, c)
|
||||
size++
|
||||
var ok bool
|
||||
c, ok = <-data
|
||||
if !ok {
|
||||
return [2]string{}, size, io.EOF
|
||||
}
|
||||
}
|
||||
if size == 1 { // continuation header here
|
||||
hdr = []byte(oldHdr[0])
|
||||
val = append([]byte(oldHdr[1]), '\n')
|
||||
}
|
||||
for c := <-data; c != '\n'; c = <-data {
|
||||
for {
|
||||
var ok bool
|
||||
c, ok = <-data
|
||||
if !ok {
|
||||
return [2]string{}, size, io.EOF
|
||||
}
|
||||
if c == '\n' {
|
||||
break
|
||||
}
|
||||
val = append(val, c)
|
||||
size++
|
||||
}
|
||||
@@ -547,7 +604,14 @@ func parseGitCommitHdr(oldHdr [2]string, data <-chan byte) ([2]string, int, erro
|
||||
func parseGitCommitMsg(data <-chan byte, l int) (string, error) {
|
||||
msg := make([]byte, 0, l)
|
||||
|
||||
for c := <-data; c != '\x00'; c = <-data {
|
||||
for {
|
||||
c, ok := <-data
|
||||
if !ok {
|
||||
return string(msg), io.EOF
|
||||
}
|
||||
if c == '\x00' {
|
||||
break
|
||||
}
|
||||
msg = append(msg, c)
|
||||
l--
|
||||
}
|
||||
@@ -573,7 +637,7 @@ func parseGitCommit(data <-chan byte) (GitCommit, error) {
|
||||
var hdr [2]string
|
||||
hdr, size, err := parseGitCommitHdr(hdr, data)
|
||||
if err != nil {
|
||||
return GitCommit{}, nil
|
||||
return GitCommit{}, err
|
||||
}
|
||||
l -= size
|
||||
|
||||
@@ -594,14 +658,28 @@ func parseGitCommit(data <-chan byte) (GitCommit, error) {
|
||||
func parseTreeEntry(data <-chan byte, hashLen int) (GitTreeEntry, error) {
|
||||
var e GitTreeEntry
|
||||
|
||||
for c := <-data; c != ' '; c = <-data {
|
||||
for {
|
||||
c, ok := <-data
|
||||
if !ok {
|
||||
return e, io.EOF
|
||||
}
|
||||
if c == ' ' {
|
||||
break
|
||||
}
|
||||
e.mode = e.mode*8 + int(c-'0')
|
||||
e.size++
|
||||
}
|
||||
e.size++
|
||||
|
||||
name := make([]byte, 0, 128)
|
||||
for c := <-data; c != '\x00'; c = <-data {
|
||||
for {
|
||||
c, ok := <-data
|
||||
if !ok {
|
||||
return e, io.EOF
|
||||
}
|
||||
if c == '\x00' {
|
||||
break
|
||||
}
|
||||
name = append(name, c)
|
||||
e.size++
|
||||
}
|
||||
@@ -612,7 +690,10 @@ func parseTreeEntry(data <-chan byte, hashLen int) (GitTreeEntry, error) {
|
||||
|
||||
hash := make([]byte, 0, hashLen*2)
|
||||
for range hashLen {
|
||||
c := <-data
|
||||
c, ok := <-data
|
||||
if !ok {
|
||||
return e, io.EOF
|
||||
}
|
||||
hash = append(hash, hexBinToAscii[((c&0xF0)>>4)], hexBinToAscii[c&0xF])
|
||||
}
|
||||
e.hash = string(hash)
|
||||
@@ -633,13 +714,16 @@ func parseGitTree(data <-chan byte) (GitTree, error) {
|
||||
for parsedLen < hdr.size {
|
||||
entry, err := parseTreeEntry(data, len(hdr.hash)/2)
|
||||
if err != nil {
|
||||
return GitTree{}, nil
|
||||
return GitTree{}, err
|
||||
}
|
||||
|
||||
t.items = append(t.items, entry)
|
||||
parsedLen += entry.size
|
||||
}
|
||||
c := <-data // \0 read
|
||||
c, ok := <-data // \0 read
|
||||
if !ok {
|
||||
return t, io.EOF
|
||||
}
|
||||
|
||||
if c != '\x00' {
|
||||
return t, fmt.Errorf("Unexpected character during git tree data read")
|
||||
@@ -660,9 +744,16 @@ func parseGitBlob(data <-chan byte) ([]byte, error) {
|
||||
|
||||
d := make([]byte, hdr.size)
|
||||
for l := 0; l < hdr.size; l++ {
|
||||
d[l] = <-data
|
||||
var ok bool
|
||||
d[l], ok = <-data
|
||||
if !ok {
|
||||
return d, io.EOF
|
||||
}
|
||||
}
|
||||
eob, ok := <-data
|
||||
if !ok {
|
||||
return d, io.EOF
|
||||
}
|
||||
eob := <-data
|
||||
if eob != '\x00' {
|
||||
return d, fmt.Errorf("invalid byte read in parseGitBlob")
|
||||
}
|
||||
@@ -674,16 +765,25 @@ func (e *GitHandlerImpl) GitParseCommits(cwd string, commitIDs []string) (parsed
|
||||
var done sync.Mutex
|
||||
|
||||
done.Lock()
|
||||
data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
|
||||
done_signal := make(chan struct{})
|
||||
var once sync.Once
|
||||
close_done := func() {
|
||||
once.Do(func() {
|
||||
close(done_signal)
|
||||
})
|
||||
}
|
||||
|
||||
data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
|
||||
parsedCommits = make([]GitCommit, 0, len(commitIDs))
|
||||
|
||||
go func() {
|
||||
defer done.Unlock()
|
||||
defer close_done()
|
||||
defer close(data_out.ch)
|
||||
|
||||
for _, id := range commitIDs {
|
||||
data_out.Write([]byte(id))
|
||||
data_out.ch <- '\x00'
|
||||
data_out.Write([]byte{0})
|
||||
c, e := parseGitCommit(data_in.ch)
|
||||
if e != nil {
|
||||
err = fmt.Errorf("Error parsing git commit: %w", e)
|
||||
@@ -710,12 +810,14 @@ func (e *GitHandlerImpl) GitParseCommits(cwd string, commitIDs []string) (parsed
|
||||
LogDebug("command run:", cmd.Args)
|
||||
if e := cmd.Run(); e != nil {
|
||||
LogError(e)
|
||||
close_done()
|
||||
close(data_in.ch)
|
||||
close(data_out.ch)
|
||||
return nil, e
|
||||
}
|
||||
|
||||
done.Lock()
|
||||
close_done()
|
||||
close(data_in.ch)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -724,15 +826,21 @@ func (e *GitHandlerImpl) GitCatFile(cwd, commitId, filename string) (data []byte
|
||||
var done sync.Mutex
|
||||
|
||||
done.Lock()
|
||||
data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
|
||||
done_signal := make(chan struct{})
|
||||
var once sync.Once
|
||||
close_done := func() {
|
||||
once.Do(func() {
|
||||
close(done_signal)
|
||||
})
|
||||
}
|
||||
data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
|
||||
|
||||
go func() {
|
||||
defer done.Unlock()
|
||||
defer close_done()
|
||||
defer close(data_out.ch)
|
||||
|
||||
data_out.Write([]byte(commitId))
|
||||
data_out.ch <- '\x00'
|
||||
|
||||
data_out.Write([]byte{0})
|
||||
var c GitCommit
|
||||
c, err = parseGitCommit(data_in.ch)
|
||||
if err != nil {
|
||||
@@ -740,11 +848,9 @@ func (e *GitHandlerImpl) GitCatFile(cwd, commitId, filename string) (data []byte
|
||||
return
|
||||
}
|
||||
data_out.Write([]byte(c.Tree))
|
||||
data_out.ch <- '\x00'
|
||||
|
||||
data_out.Write([]byte{0})
|
||||
var tree GitTree
|
||||
tree, err = parseGitTree(data_in.ch)
|
||||
|
||||
if err != nil {
|
||||
LogError("Error parsing git tree:", err)
|
||||
return
|
||||
@@ -754,7 +860,7 @@ func (e *GitHandlerImpl) GitCatFile(cwd, commitId, filename string) (data []byte
|
||||
if te.isBlob() && te.name == filename {
|
||||
LogInfo("blob", te.hash)
|
||||
data_out.Write([]byte(te.hash))
|
||||
data_out.ch <- '\x00'
|
||||
data_out.Write([]byte{0})
|
||||
data, err = parseGitBlob(data_in.ch)
|
||||
return
|
||||
}
|
||||
@@ -779,11 +885,13 @@ func (e *GitHandlerImpl) GitCatFile(cwd, commitId, filename string) (data []byte
|
||||
LogDebug("command run:", cmd.Args)
|
||||
if e := cmd.Run(); e != nil {
|
||||
LogError(e)
|
||||
close_done()
|
||||
close(data_in.ch)
|
||||
close(data_out.ch)
|
||||
return nil, e
|
||||
}
|
||||
done.Lock()
|
||||
close_done()
|
||||
close(data_in.ch)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -793,16 +901,24 @@ func (e *GitHandlerImpl) GitDirectoryList(gitPath, commitId string) (directoryLi
|
||||
directoryList = make(map[string]string)
|
||||
|
||||
done.Lock()
|
||||
data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
|
||||
done_signal := make(chan struct{})
|
||||
var once sync.Once
|
||||
close_done := func() {
|
||||
once.Do(func() {
|
||||
close(done_signal)
|
||||
})
|
||||
}
|
||||
data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
|
||||
|
||||
LogDebug("Getting directory for:", commitId)
|
||||
|
||||
go func() {
|
||||
defer done.Unlock()
|
||||
defer close_done()
|
||||
defer close(data_out.ch)
|
||||
|
||||
data_out.Write([]byte(commitId))
|
||||
data_out.ch <- '\x00'
|
||||
data_out.Write([]byte{0})
|
||||
var c GitCommit
|
||||
c, err = parseGitCommit(data_in.ch)
|
||||
if err != nil {
|
||||
@@ -818,7 +934,7 @@ func (e *GitHandlerImpl) GitDirectoryList(gitPath, commitId string) (directoryLi
|
||||
delete(trees, p)
|
||||
|
||||
data_out.Write([]byte(tree))
|
||||
data_out.ch <- '\x00'
|
||||
data_out.Write([]byte{0})
|
||||
var tree GitTree
|
||||
tree, err = parseGitTree(data_in.ch)
|
||||
|
||||
@@ -852,12 +968,14 @@ func (e *GitHandlerImpl) GitDirectoryList(gitPath, commitId string) (directoryLi
|
||||
LogDebug("command run:", cmd.Args)
|
||||
if e := cmd.Run(); e != nil {
|
||||
LogError(e)
|
||||
close_done()
|
||||
close(data_in.ch)
|
||||
close(data_out.ch)
|
||||
return directoryList, e
|
||||
}
|
||||
|
||||
done.Lock()
|
||||
close_done()
|
||||
close(data_in.ch)
|
||||
return directoryList, err
|
||||
}
|
||||
|
||||
@@ -867,16 +985,24 @@ func (e *GitHandlerImpl) GitSubmoduleList(gitPath, commitId string) (submoduleLi
|
||||
submoduleList = make(map[string]string)
|
||||
|
||||
done.Lock()
|
||||
data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
|
||||
done_signal := make(chan struct{})
|
||||
var once sync.Once
|
||||
close_done := func() {
|
||||
once.Do(func() {
|
||||
close(done_signal)
|
||||
})
|
||||
}
|
||||
data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
|
||||
|
||||
LogDebug("Getting submodules for:", commitId)
|
||||
|
||||
go func() {
|
||||
defer done.Unlock()
|
||||
defer close_done()
|
||||
defer close(data_out.ch)
|
||||
|
||||
data_out.Write([]byte(commitId))
|
||||
data_out.ch <- '\x00'
|
||||
data_out.Write([]byte{0})
|
||||
var c GitCommit
|
||||
c, err = parseGitCommit(data_in.ch)
|
||||
if err != nil {
|
||||
@@ -892,7 +1018,7 @@ func (e *GitHandlerImpl) GitSubmoduleList(gitPath, commitId string) (submoduleLi
|
||||
delete(trees, p)
|
||||
|
||||
data_out.Write([]byte(tree))
|
||||
data_out.ch <- '\x00'
|
||||
data_out.Write([]byte{0})
|
||||
var tree GitTree
|
||||
tree, err = parseGitTree(data_in.ch)
|
||||
|
||||
@@ -929,17 +1055,26 @@ func (e *GitHandlerImpl) GitSubmoduleList(gitPath, commitId string) (submoduleLi
|
||||
LogDebug("command run:", cmd.Args)
|
||||
if e := cmd.Run(); e != nil {
|
||||
LogError(e)
|
||||
close_done()
|
||||
close(data_in.ch)
|
||||
close(data_out.ch)
|
||||
return submoduleList, e
|
||||
}
|
||||
|
||||
done.Lock()
|
||||
close_done()
|
||||
close(data_in.ch)
|
||||
return submoduleList, err
|
||||
}
|
||||
|
||||
func (e *GitHandlerImpl) GitSubmoduleCommitId(cwd, packageName, commitId string) (subCommitId string, valid bool) {
|
||||
data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
|
||||
done_signal := make(chan struct{})
|
||||
var once sync.Once
|
||||
close_done := func() {
|
||||
once.Do(func() {
|
||||
close(done_signal)
|
||||
})
|
||||
}
|
||||
data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
|
||||
var wg sync.WaitGroup
|
||||
|
||||
wg.Add(1)
|
||||
@@ -955,17 +1090,18 @@ func (e *GitHandlerImpl) GitSubmoduleCommitId(cwd, packageName, commitId string)
|
||||
}()
|
||||
|
||||
defer wg.Done()
|
||||
defer close_done()
|
||||
defer close(data_out.ch)
|
||||
|
||||
data_out.Write([]byte(commitId))
|
||||
data_out.ch <- '\x00'
|
||||
data_out.Write([]byte{0})
|
||||
c, err := parseGitCommit(data_in.ch)
|
||||
if err != nil {
|
||||
LogError("Error parsing git commit:", err)
|
||||
panic(err)
|
||||
}
|
||||
data_out.Write([]byte(c.Tree))
|
||||
data_out.ch <- '\x00'
|
||||
data_out.Write([]byte{0})
|
||||
tree, err := parseGitTree(data_in.ch)
|
||||
|
||||
if err != nil {
|
||||
@@ -997,12 +1133,14 @@ func (e *GitHandlerImpl) GitSubmoduleCommitId(cwd, packageName, commitId string)
|
||||
LogDebug("command run:", cmd.Args)
|
||||
if e := cmd.Run(); e != nil {
|
||||
LogError(e)
|
||||
close_done()
|
||||
close(data_in.ch)
|
||||
close(data_out.ch)
|
||||
return subCommitId, false
|
||||
}
|
||||
|
||||
wg.Wait()
|
||||
close_done()
|
||||
close(data_in.ch)
|
||||
return subCommitId, len(subCommitId) > 0
|
||||
}
|
||||
|
||||
|
||||
@@ -27,6 +27,7 @@ import (
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestGitClone(t *testing.T) {
|
||||
@@ -596,3 +597,47 @@ func TestGitStatusParse(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGitDeadlockFix(t *testing.T) {
|
||||
gitDir := t.TempDir()
|
||||
testDir, _ := os.Getwd()
|
||||
|
||||
cmd := exec.Command("/usr/bin/bash", path.Join(testDir, "tsetup.sh"))
|
||||
cmd.Dir = gitDir
|
||||
_, err := cmd.CombinedOutput()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
gh, err := AllocateGitWorkTree(gitDir, "Test", "test@example.com")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
h, err := gh.ReadExistingPath(".")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer h.Close()
|
||||
|
||||
// Use a blob ID to trigger error in GitParseCommits
|
||||
// This ensures that the function returns error immediately and doesn't deadlock
|
||||
blobId := "81aba862107f1e2f5312e165453955485f424612f313d6c2fb1b31fef9f82a14"
|
||||
|
||||
done := make(chan error)
|
||||
go func() {
|
||||
_, err := h.GitParseCommits("", []string{blobId})
|
||||
done <- err
|
||||
}()
|
||||
|
||||
select {
|
||||
case err := <-done:
|
||||
if err == nil {
|
||||
t.Error("Expected error from GitParseCommits with blob ID, got nil")
|
||||
} else {
|
||||
// This is expected
|
||||
t.Logf("Got expected error: %v", err)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("GitParseCommits deadlocked! Fix is NOT working.")
|
||||
}
|
||||
}
|
||||
|
||||
26
common/pr.go
26
common/pr.go
@@ -23,7 +23,8 @@ type PRSet struct {
|
||||
PRs []*PRInfo
|
||||
Config *AutogitConfig
|
||||
|
||||
BotUser string
|
||||
BotUser string
|
||||
HasAutoStaging bool
|
||||
}
|
||||
|
||||
func (prinfo *PRInfo) PRComponents() (org string, repo string, idx int64) {
|
||||
@@ -123,14 +124,19 @@ func LastPrjGitRefOnTimeline(botUser string, gitea GiteaPRTimelineReviewFetcher,
|
||||
}
|
||||
|
||||
pr, err := gitea.GetPullRequest(prjGitOrg, prjGitRepo, issue.Index)
|
||||
switch err.(type) {
|
||||
case *repository.RepoGetPullRequestNotFound: // deleted?
|
||||
continue
|
||||
default:
|
||||
LogDebug("PrjGit RefIssue fetch error from timeline", issue.Index, err)
|
||||
if err != nil {
|
||||
switch err.(type) {
|
||||
case *repository.RepoGetPullRequestNotFound: // deleted?
|
||||
continue
|
||||
default:
|
||||
LogDebug("PrjGit RefIssue fetch error from timeline", issue.Index, err)
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
if pr.Base.Ref != prjGitBranch {
|
||||
LogDebug("found ref PR on timeline:", PRtoString(pr))
|
||||
if pr.Base.Name != prjGitBranch {
|
||||
LogDebug(" -> not matching:", pr.Base.Name, prjGitBranch)
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -303,6 +309,9 @@ func (rs *PRSet) FindMissingAndExtraReviewers(maintainers MaintainershipData, id
|
||||
pr := rs.PRs[idx]
|
||||
if rs.IsPrjGitPR(pr.PR) {
|
||||
missing = slices.Concat(configReviewers.Prj, configReviewers.PrjOptional)
|
||||
if rs.HasAutoStaging {
|
||||
missing = append(missing, Bot_BuildReview)
|
||||
}
|
||||
LogDebug("PrjGit submitter:", pr.PR.User.UserName)
|
||||
// only need project maintainer reviews if:
|
||||
// * not created by a bot and has other PRs, or
|
||||
@@ -491,6 +500,9 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData
|
||||
var pkg string
|
||||
if rs.IsPrjGitPR(pr.PR) {
|
||||
reviewers = configReviewers.Prj
|
||||
if rs.HasAutoStaging {
|
||||
reviewers = append(reviewers, Bot_BuildReview)
|
||||
}
|
||||
pkg = ""
|
||||
} else {
|
||||
reviewers = configReviewers.Pkg
|
||||
|
||||
@@ -631,6 +631,8 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
|
||||
prset *common.PRSet
|
||||
maintainers common.MaintainershipData
|
||||
|
||||
noAutoStaging bool
|
||||
|
||||
expected_missing_reviewers [][]string
|
||||
expected_extra_reviewers [][]string
|
||||
}{
|
||||
@@ -688,7 +690,40 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
|
||||
[]string{"autogits_obs_staging_bot", "user1"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "One project reviewer only and no auto staging",
|
||||
noAutoStaging: true,
|
||||
prset: &common.PRSet{
|
||||
PRs: []*common.PRInfo{
|
||||
{
|
||||
PR: &models.PullRequest{
|
||||
User: &models.User{UserName: "foo"},
|
||||
Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
|
||||
},
|
||||
Reviews: &common.PRReviews{},
|
||||
},
|
||||
{
|
||||
PR: &models.PullRequest{
|
||||
User: &models.User{UserName: "foo"},
|
||||
Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "prg"}}},
|
||||
},
|
||||
Reviews: &common.PRReviews{},
|
||||
},
|
||||
},
|
||||
Config: &common.AutogitConfig{
|
||||
GitProjectName: "prg/repo#main",
|
||||
Organization: "org",
|
||||
Branch: "main",
|
||||
Reviewers: []string{"-user1"},
|
||||
},
|
||||
},
|
||||
maintainers: &common.MaintainershipMap{Data: map[string][]string{}},
|
||||
|
||||
expected_missing_reviewers: [][]string{
|
||||
nil,
|
||||
{"user1"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "One project reviewer and one pkg reviewer only",
|
||||
prset: &common.PRSet{
|
||||
@@ -1064,6 +1099,7 @@ func TestFindMissingAndExtraReviewers(t *testing.T) {
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
test.prset.HasAutoStaging = !test.noAutoStaging
|
||||
for idx, pr := range test.prset.PRs {
|
||||
missing, extra := test.prset.FindMissingAndExtraReviewers(test.maintainers, idx)
|
||||
|
||||
|
||||
@@ -1,9 +1,5 @@
|
||||
package common
|
||||
|
||||
import (
|
||||
"slices"
|
||||
)
|
||||
|
||||
type Reviewers struct {
|
||||
Prj []string
|
||||
Pkg []string
|
||||
@@ -36,10 +32,5 @@ func ParseReviewers(input []string) *Reviewers {
|
||||
*pkg = append(*pkg, reviewer)
|
||||
}
|
||||
}
|
||||
|
||||
if !slices.Contains(r.Prj, Bot_BuildReview) {
|
||||
r.Prj = append(r.Prj, Bot_BuildReview)
|
||||
}
|
||||
|
||||
return r
|
||||
}
|
||||
|
||||
@@ -21,14 +21,14 @@ func TestReviewers(t *testing.T) {
|
||||
name: "project and package reviewers",
|
||||
input: []string{"1", "2", "3", "*5", "+6", "-7"},
|
||||
|
||||
prj: []string{"5", "7", common.Bot_BuildReview},
|
||||
prj: []string{"5", "7"},
|
||||
pkg: []string{"1", "2", "3", "5", "6"},
|
||||
},
|
||||
{
|
||||
name: "optional project and package reviewers",
|
||||
input: []string{"~1", "2", "3", "~*5", "+6", "-7"},
|
||||
|
||||
prj: []string{"7", common.Bot_BuildReview},
|
||||
prj: []string{"7"},
|
||||
pkg: []string{"2", "3", "6"},
|
||||
prj_optional: []string{"5"},
|
||||
pkg_optional: []string{"1", "5"},
|
||||
|
||||
@@ -27,10 +27,87 @@ import (
|
||||
"regexp"
|
||||
"slices"
|
||||
"strings"
|
||||
"unicode"
|
||||
|
||||
"src.opensuse.org/autogits/common/gitea-generated/models"
|
||||
)
|
||||
|
||||
type NewRepos struct {
|
||||
Repos []struct {
|
||||
Organization, Repository, Branch string
|
||||
PackageName string
|
||||
}
|
||||
IsMaintainer bool
|
||||
}
|
||||
|
||||
const maintainership_line = "MAINTAINER"
|
||||
|
||||
var true_lines []string = []string{"1", "TRUE", "YES", "OK", "T"}
|
||||
|
||||
func HasSpace(s string) bool {
|
||||
return strings.IndexFunc(s, unicode.IsSpace) >= 0
|
||||
}
|
||||
|
||||
func FindNewReposInIssueBody(body string) *NewRepos {
|
||||
Issues := &NewRepos{}
|
||||
for _, line := range strings.Split(body, "\n") {
|
||||
line = strings.TrimSpace(line)
|
||||
if ul := strings.ToUpper(line); strings.HasPrefix(ul, "MAINTAINER") {
|
||||
value := ""
|
||||
if idx := strings.IndexRune(ul, ':'); idx > 0 && len(ul) > idx+2 {
|
||||
value = ul[idx+1:]
|
||||
} else if idx := strings.IndexRune(ul, ' '); idx > 0 && len(ul) > idx+2 {
|
||||
value = ul[idx+1:]
|
||||
}
|
||||
|
||||
if slices.Contains(true_lines, strings.TrimSpace(value)) {
|
||||
Issues.IsMaintainer = true
|
||||
}
|
||||
}
|
||||
// line = strings.TrimSpace(line)
|
||||
issue := struct{ Organization, Repository, Branch, PackageName string }{}
|
||||
|
||||
branch := strings.Split(line, "#")
|
||||
repo := strings.Split(branch[0], "/")
|
||||
|
||||
if len(branch) == 2 {
|
||||
issue.Branch = strings.TrimSpace(branch[1])
|
||||
}
|
||||
if len(repo) == 2 {
|
||||
issue.Organization = strings.TrimSpace(repo[0])
|
||||
issue.Repository = strings.TrimSpace(repo[1])
|
||||
issue.PackageName = issue.Repository
|
||||
|
||||
if idx := strings.Index(strings.ToUpper(issue.Branch), " AS "); idx > 0 && len(issue.Branch) > idx+5 {
|
||||
issue.PackageName = strings.TrimSpace(issue.Branch[idx+3:])
|
||||
issue.Branch = strings.TrimSpace(issue.Branch[0:idx])
|
||||
}
|
||||
|
||||
if HasSpace(issue.Organization) || HasSpace(issue.Repository) || HasSpace(issue.PackageName) || HasSpace(issue.Branch) {
|
||||
continue
|
||||
}
|
||||
} else {
|
||||
continue
|
||||
}
|
||||
Issues.Repos = append(Issues.Repos, issue)
|
||||
//PackageNameIdx := strings.Index(strings.ToUpper(line), " AS ")
|
||||
//words := strings.Split(line)
|
||||
}
|
||||
|
||||
if len(Issues.Repos) == 0 {
|
||||
return nil
|
||||
}
|
||||
return Issues
|
||||
}
|
||||
|
||||
func IssueToString(issue *models.Issue) string {
|
||||
if issue == nil {
|
||||
return "(nil)"
|
||||
}
|
||||
|
||||
return fmt.Sprintf("%s/%s#%d", issue.Repository.Owner, issue.Repository.Name, issue.Index)
|
||||
}
|
||||
|
||||
func SplitLines(str string) []string {
|
||||
return SplitStringNoEmpty(str, "\n")
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package common_test
|
||||
|
||||
import (
|
||||
"reflect"
|
||||
"testing"
|
||||
|
||||
"src.opensuse.org/autogits/common"
|
||||
@@ -220,3 +221,87 @@ func TestRemovedBranchName(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewPackageIssueParsing(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
input string
|
||||
issues *common.NewRepos
|
||||
}{
|
||||
{
|
||||
name: "Nothing",
|
||||
},
|
||||
{
|
||||
name: "Basic repo",
|
||||
input: "org/repo#branch",
|
||||
issues: &common.NewRepos{
|
||||
Repos: []struct{ Organization, Repository, Branch, PackageName string }{
|
||||
{Organization: "org", Repository: "repo", Branch: "branch", PackageName: "repo"},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Default branch and junk lines and approval for maintainership",
|
||||
input: "\n\nsome comments\n\norg1/repo2\n\nmaintainership: yes",
|
||||
issues: &common.NewRepos{
|
||||
Repos: []struct{ Organization, Repository, Branch, PackageName string }{
|
||||
{Organization: "org1", Repository: "repo2", Branch: "", PackageName: "repo2"},
|
||||
},
|
||||
IsMaintainer: true,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Default branch and junk lines and no maintainership",
|
||||
input: "\n\nsome comments\n\norg1/repo2\n\nmaintainership: NEVER",
|
||||
issues: &common.NewRepos{
|
||||
Repos: []struct{ Organization, Repository, Branch, PackageName string }{
|
||||
{Organization: "org1", Repository: "repo2", Branch: "", PackageName: "repo2"},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "3 repos with comments and maintainership",
|
||||
input: "\n\nsome comments for org1/repo2 are here and more\n\norg1/repo2#master\n org2/repo3#master\n some/repo3#m\nMaintainer ok",
|
||||
issues: &common.NewRepos{
|
||||
Repos: []struct{ Organization, Repository, Branch, PackageName string }{
|
||||
{Organization: "org1", Repository: "repo2", Branch: "master", PackageName: "repo2"},
|
||||
{Organization: "org2", Repository: "repo3", Branch: "master", PackageName: "repo3"},
|
||||
{Organization: "some", Repository: "repo3", Branch: "m", PackageName: "repo3"},
|
||||
},
|
||||
IsMaintainer: true,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Invalid repos with spaces",
|
||||
input: "or g/repo#branch\norg/r epo#branch\norg/repo#br anch\norg/repo#branch As foo ++",
|
||||
},
|
||||
{
|
||||
name: "Valid repos with spaces",
|
||||
input: " org / repo # branch",
|
||||
issues: &common.NewRepos{
|
||||
Repos: []struct{ Organization, Repository, Branch, PackageName string }{
|
||||
{Organization: "org", Repository: "repo", Branch: "branch", PackageName: "repo"},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Package name is not repo name",
|
||||
input: " org / repo # branch as repo++ \nmaintainer true",
|
||||
issues: &common.NewRepos{
|
||||
Repos: []struct{ Organization, Repository, Branch, PackageName string }{
|
||||
{Organization: "org", Repository: "repo", Branch: "branch", PackageName: "repo++"},
|
||||
},
|
||||
IsMaintainer: true,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
issue := common.FindNewReposInIssueBody(test.input)
|
||||
if !reflect.DeepEqual(test.issues, issue) {
|
||||
t.Error("Expected", test.issues, "but have", issue)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -170,7 +170,7 @@ func main() {
|
||||
common.RequestType_PRSync: req,
|
||||
common.RequestType_PRReviewAccepted: req,
|
||||
common.RequestType_PRReviewRejected: req,
|
||||
common.RequestType_IssueComment: req,
|
||||
common.RequestType_PRComment: req,
|
||||
},
|
||||
}
|
||||
listenDefs.Connection().RabbitURL, _ = url.Parse(*rabbitUrl)
|
||||
|
||||
@@ -554,6 +554,14 @@ func (pr *PRProcessor) Process(req *models.PullRequest) error {
|
||||
return err
|
||||
}
|
||||
|
||||
// update prset if we should build it or not
|
||||
if prjGitPR != nil {
|
||||
if file, err := git.GitCatFile(common.DefaultGitPrj, prjGitPR.PR.Head.Sha, "staging.config"); err == nil {
|
||||
prset.HasAutoStaging = (file != nil)
|
||||
common.LogDebug(" -> automatic staging enabled?:", prset.HasAutoStaging)
|
||||
}
|
||||
}
|
||||
|
||||
// handle case where PrjGit PR is only one left and there are no changes, then we can just close the PR
|
||||
if len(prset.PRs) == 1 && prjGitPR != nil && prset.PRs[0] == prjGitPR && prjGitPR.PR.User.UserName == prset.BotUser {
|
||||
common.LogDebug(" --> checking if superflous PR")
|
||||
|
||||
Reference in New Issue
Block a user