diff --git a/autogits.spec b/autogits.spec index d763fe6..8487a9c 100644 --- a/autogits.spec +++ b/autogits.spec @@ -129,6 +129,9 @@ go build \ go build \ -C utils/hujson \ -buildmode=pie +go build \ + -C utils/maintainer-update \ + -buildmode=pie go build \ -C gitea-events-rabbitmq-publisher \ -buildmode=pie @@ -160,6 +163,7 @@ go test -C group-review -v go test -C obs-staging-bot -v go test -C obs-status-service -v go test -C workflow-direct -v +go test -C utils/maintainer-update # TODO build fails #go test -C workflow-pr -v @@ -179,6 +183,7 @@ install -D -m0755 workflow-direct/workflow-direct install -D -m0644 systemd/workflow-direct@.service %{buildroot}%{_unitdir}/workflow-direct@.service install -D -m0755 workflow-pr/workflow-pr %{buildroot}%{_bindir}/workflow-pr install -D -m0755 utils/hujson/hujson %{buildroot}%{_bindir}/hujson +install -D -m0755 utils/maintainer-update/maintainer-update %{buildroot}${_bindir}/maintainer-update %pre gitea-events-rabbitmq-publisher %service_add_pre gitea-events-rabbitmq-publisher.service @@ -285,6 +290,7 @@ install -D -m0755 utils/hujson/hujson %files utils %license COPYING %{_bindir}/hujson +%{_bindir}/maintainer-update %files workflow-direct %license COPYING diff --git a/common/maintainership.go b/common/maintainership.go index 39d2528..ec65fe6 100644 --- a/common/maintainership.go +++ b/common/maintainership.go @@ -1,10 +1,12 @@ package common import ( + "bytes" "encoding/json" "fmt" "io" "slices" + "strings" "src.opensuse.org/autogits/common/gitea-generated/client/repository" "src.opensuse.org/autogits/common/gitea-generated/models" @@ -26,11 +28,13 @@ type MaintainershipMap struct { Data map[string][]string IsDir bool FetchPackage func(string) ([]byte, error) + Raw []byte } -func parseMaintainershipData(data []byte) (*MaintainershipMap, error) { +func ParseMaintainershipData(data []byte) (*MaintainershipMap, error) { maintainers := &MaintainershipMap{ Data: make(map[string][]string), + Raw: data, } if err := json.Unmarshal(data, &maintainers.Data); err != nil { return nil, err @@ -59,7 +63,7 @@ func FetchProjectMaintainershipData(gitea GiteaMaintainershipReader, org, prjGit } } - m, err := parseMaintainershipData(data) + m, err := ParseMaintainershipData(data) if m != nil { m.IsDir = dir m.FetchPackage = func(pkg string) ([]byte, error) { @@ -164,13 +168,135 @@ func (data *MaintainershipMap) IsApproved(pkg string, reviews []*models.PullRevi return false } +func (data *MaintainershipMap) modifyInplace(writer io.StringWriter) error { + var original map[string][]string + if err := json.Unmarshal(data.Raw, &original); err != nil { + return err + } + + dec := json.NewDecoder(bytes.NewReader(data.Raw)) + _, err := dec.Token() + if err != nil { + return err + } + + output := "" + lastPos := 0 + modified := false + + type entry struct { + key string + valStart int + valEnd int + } + var entries []entry + + for dec.More() { + kToken, _ := dec.Token() + key := kToken.(string) + var raw json.RawMessage + dec.Decode(&raw) + valEnd := int(dec.InputOffset()) + valStart := valEnd - len(raw) + entries = append(entries, entry{key, valStart, valEnd}) + } + + changed := make(map[string]bool) + for k, v := range data.Data { + if ov, ok := original[k]; !ok || !slices.Equal(v, ov) { + changed[k] = true + } + } + for k := range original { + if _, ok := data.Data[k]; !ok { + changed[k] = true + } + } + + if len(changed) == 0 { + _, err = writer.WriteString(string(data.Raw)) + return err + } + + for _, e := range entries { + if v, ok := data.Data[e.key]; ok { + prefix := string(data.Raw[lastPos:e.valStart]) + if modified && strings.TrimSpace(output) == "{" { + if commaIdx := strings.Index(prefix, ","); commaIdx != -1 { + if quoteIdx := strings.Index(prefix, "\""); quoteIdx == -1 || commaIdx < quoteIdx { + prefix = prefix[:commaIdx] + prefix[commaIdx+1:] + } + } + } + output += prefix + if changed[e.key] { + slices.Sort(v) + newVal, _ := json.Marshal(v) + output += string(newVal) + modified = true + } else { + output += string(data.Raw[e.valStart:e.valEnd]) + } + } else { + // Deleted + modified = true + } + lastPos = e.valEnd + } + output += string(data.Raw[lastPos:]) + + // Handle additions (simplistic: at the end) + for k, v := range data.Data { + if _, ok := original[k]; !ok { + slices.Sort(v) + newVal, _ := json.Marshal(v) + keyStr, _ := json.Marshal(k) + + // Insert before closing brace + if idx := strings.LastIndex(output, "}"); idx != -1 { + prefix := output[:idx] + suffix := output[idx:] + + trimmedPrefix := strings.TrimRight(prefix, " \n\r\t") + if !strings.HasSuffix(trimmedPrefix, "{") && !strings.HasSuffix(trimmedPrefix, ",") { + // find the actual position of the last non-whitespace character in prefix + lastCharIdx := strings.LastIndexAny(prefix, "]}0123456789\"") + if lastCharIdx != -1 { + prefix = prefix[:lastCharIdx+1] + "," + prefix[lastCharIdx+1:] + } + } + + insertion := fmt.Sprintf(" %s: %s", string(keyStr), string(newVal)) + if !strings.HasSuffix(prefix, "\n") { + insertion = "\n" + insertion + } + output = prefix + insertion + "\n" + suffix + modified = true + } + } + } + + if modified { + _, err := writer.WriteString(output) + return err + } + _, err = writer.WriteString(string(data.Raw)) + return err +} + func (data *MaintainershipMap) WriteMaintainershipFile(writer io.StringWriter) error { if data.IsDir { return fmt.Errorf("Not implemented") } - writer.WriteString("{\n") + if len(data.Raw) > 0 { + if err := data.modifyInplace(writer); err == nil { + return nil + } + } + // Fallback to full write + writer.WriteString("{\n") if d, ok := data.Data[""]; ok { eol := "," if len(data.Data) == 1 { @@ -181,17 +307,12 @@ func (data *MaintainershipMap) WriteMaintainershipFile(writer io.StringWriter) e writer.WriteString(fmt.Sprintf(" \"\": %s%s\n", string(str), eol)) } - keys := make([]string, len(data.Data)) - i := 0 + keys := make([]string, 0, len(data.Data)) for pkg := range data.Data { if pkg == "" { continue } - keys[i] = pkg - i++ - } - if len(keys) >= i { - keys = slices.Delete(keys, i, len(keys)) + keys = append(keys, pkg) } slices.Sort(keys) for i, pkg := range keys { diff --git a/common/maintainership_test.go b/common/maintainership_test.go index e8c8573..73d0f6e 100644 --- a/common/maintainership_test.go +++ b/common/maintainership_test.go @@ -208,6 +208,7 @@ func TestMaintainershipFileWrite(t *testing.T) { name string is_dir bool maintainers map[string][]string + raw []byte expected_output string expected_error error }{ @@ -231,6 +232,43 @@ func TestMaintainershipFileWrite(t *testing.T) { }, expected_output: "{\n \"\": [\"one\",\"two\"],\n \"foo\": [\"byte\",\"four\"],\n \"pkg1\": []\n}\n", }, + { + name: "surgical modification", + maintainers: map[string][]string{ + "": {"one", "two"}, + "foo": {"byte", "four", "newone"}, + "pkg1": {}, + }, + raw: []byte("{\n \"\": [\"one\",\"two\"],\n \"foo\": [\"byte\",\"four\"],\n \"pkg1\": []\n}\n"), + expected_output: "{\n \"\": [\"one\",\"two\"],\n \"foo\": [\"byte\",\"four\",\"newone\"],\n \"pkg1\": []\n}\n", + }, + { + name: "no change", + maintainers: map[string][]string{ + "": {"one", "two"}, + "foo": {"byte", "four"}, + "pkg1": {}, + }, + raw: []byte("{\n \"\": [\"one\",\"two\"],\n \"foo\": [\"byte\",\"four\"],\n \"pkg1\": []\n}\n"), + expected_output: "{\n \"\": [\"one\",\"two\"],\n \"foo\": [\"byte\",\"four\"],\n \"pkg1\": []\n}\n", + }, + { + name: "surgical addition", + maintainers: map[string][]string{ + "": {"one"}, + "new": {"user"}, + }, + raw: []byte("{\n \"\": [ \"one\" ]\n}\n"), + expected_output: "{\n \"\": [ \"one\" ],\n \"new\": [\"user\"]\n}\n", + }, + { + name: "surgical deletion", + maintainers: map[string][]string{ + "": {"one"}, + }, + raw: []byte("{\n \"\": [\"one\"],\n \"old\": [\"user\"]\n}\n"), + expected_output: "{\n \"\": [\"one\"]\n}\n", + }, } for _, test := range tests { @@ -239,6 +277,7 @@ func TestMaintainershipFileWrite(t *testing.T) { data := common.MaintainershipMap{ Data: test.maintainers, IsDir: test.is_dir, + Raw: test.raw, } if err := data.WriteMaintainershipFile(&b); err != test.expected_error { @@ -248,7 +287,7 @@ func TestMaintainershipFileWrite(t *testing.T) { output := b.String() if test.expected_output != output { - t.Fatal("unexpected output:", output, "Expecting:", test.expected_output) + t.Fatalf("unexpected output:\n%q\nExpecting:\n%q", output, test.expected_output) } }) } diff --git a/utils/maintainer-update/main.go b/utils/maintainer-update/main.go new file mode 100644 index 0000000..428a8e2 --- /dev/null +++ b/utils/maintainer-update/main.go @@ -0,0 +1,98 @@ +package main + +import ( + "flag" + "fmt" + "os" + "slices" + + "src.opensuse.org/autogits/common" +) + +func WriteNewMaintainershipFile(m *common.MaintainershipMap, filename string) { + f, err := os.Create(filename + ".new") + common.PanicOnError(err) + common.PanicOnError(m.WriteMaintainershipFile(f)) + common.PanicOnError(f.Sync()) + common.PanicOnError(f.Close()) + common.PanicOnError(os.Rename(filename+".new", filename)) +} + +func run() error { + pkg := flag.String("package", "", "Package to modify") + rm := flag.Bool("rm", false, "Remove maintainer from package") + add := flag.Bool("add", false, "Add maintainer to package") + lint := flag.Bool("lint-only", false, "Reformat entire _maintainership.json only") + flag.Parse() + + if (*add == *rm) && !*lint { + return fmt.Errorf("Need to either add or remove a maintainer, or lint") + } + + filename := common.MaintainershipFile + if *lint { + if len(flag.Args()) > 0 { + filename = flag.Arg(0) + } + } + + data, err := os.ReadFile(filename) + if os.IsNotExist(err) { + return err + } + if err != nil { + return err + } + + m, err := common.ParseMaintainershipData(data) + if err != nil { + return fmt.Errorf("Failed to parse JSON: %w", err) + } + + if *lint { + m.Raw = nil // forces a rewrite + } else { + users := flag.Args() + if len(users) > 0 { + maintainers, ok := m.Data[*pkg] + if !ok && !*add { + return fmt.Errorf("No package %s and not adding one.", *pkg) + } + + if *add { + for _, u := range users { + if !slices.Contains(maintainers, u) { + maintainers = append(maintainers, u) + } + } + } + + if *rm { + newMaintainers := make([]string, 0, len(maintainers)) + for _, m := range maintainers { + if !slices.Contains(users, m) { + newMaintainers = append(newMaintainers, m) + } + } + maintainers = newMaintainers + } + + if len(maintainers) > 0 { + slices.Sort(maintainers) + m.Data[*pkg] = maintainers + } else { + delete(m.Data, *pkg) + } + } + } + + WriteNewMaintainershipFile(m, filename) + return nil +} + +func main() { + if err := run(); err != nil { + common.LogError(err) + os.Exit(1) + } +} diff --git a/utils/maintainer-update/main_test.go b/utils/maintainer-update/main_test.go new file mode 100644 index 0000000..935fbea --- /dev/null +++ b/utils/maintainer-update/main_test.go @@ -0,0 +1,242 @@ +package main + +import ( + "encoding/json" + "flag" + "os" + "os/exec" + "reflect" + "strings" + "testing" + + "src.opensuse.org/autogits/common" +) + +func TestMain(m *testing.M) { + if os.Getenv("BE_MAIN") == "1" { + main() + return + } + os.Exit(m.Run()) +} + +func TestRun(t *testing.T) { + tests := []struct { + name string + inData string + expectedOut string + params []string + expectedError string + isDir bool + }{ + { + name: "add user to existing package", + inData: `{"pkg1": ["user1"]}`, + params: []string{"-package", "pkg1", "-add", "user2"}, + expectedOut: `{"pkg1": ["user1", "user2"]}`, + }, + { + name: "add user to new package", + inData: `{"pkg1": ["user1"]}`, + params: []string{"-package", "pkg2", "-add", "user2"}, + expectedOut: `{"pkg1": ["user1"], "pkg2": ["user2"]}`, + }, + { + name: "no-op with no users", + inData: `{"pkg1": ["user1"]}`, + params: []string{"-package", "pkg1", "-add"}, + expectedOut: `{"pkg1": ["user1"]}`, + }, + { + name: "add existing user", + inData: `{"pkg1": ["user1", "user2"]}`, + params: []string{"-package", "pkg1", "-add", "user2"}, + expectedOut: `{"pkg1": ["user1", "user2"]}`, + }, + { + name: "remove user from package", + inData: `{"pkg1": ["user1", "user2"]}`, + params: []string{"-package", "pkg1", "-rm", "user2"}, + expectedOut: `{"pkg1": ["user1"]}`, + }, + { + name: "remove last user from package", + inData: `{"pkg1": ["user1"]}`, + params: []string{"-package", "pkg1", "-rm", "user1"}, + expectedOut: `{}`, + }, + { + name: "remove non-existent user", + inData: `{"pkg1": ["user1"]}`, + params: []string{"-package", "pkg1", "-rm", "user2"}, + expectedOut: `{"pkg1": ["user1"]}`, + }, + { + name: "lint only unsorted", + inData: `{"pkg1": ["user2", "user1"]}`, + params: []string{"-lint-only"}, + expectedOut: `{"pkg1": ["user1", "user2"]}`, + }, + { + name: "lint only no changes", + inData: `{"pkg1": ["user1", "user2"]}`, + params: []string{"-lint-only"}, + expectedOut: `{"pkg1": ["user1", "user2"]}`, + }, + { + name: "no file", + params: []string{"-add"}, + expectedError: "no such file or directory", + }, + { + name: "invalid json", + inData: `{"pkg1": ["user1"`, + params: []string{"-add"}, + expectedError: "Failed to parse JSON", + }, + { + name: "add", + inData: `{"pkg1": ["user1", "user2"]}`, + params: []string{"-package", "pkg1", "-add", "user3"}, + expectedOut: `{"pkg1": ["user1", "user2", "user3"]}`, + }, + { + name: "lint specific file", + inData: `{"pkg1": ["user2", "user1"]}`, + params: []string{"-lint-only", "other.json"}, + expectedOut: `{"pkg1": ["user1", "user2"]}`, + }, + { + name: "add user to package when it was not there before", + inData: `{}`, + params: []string{"-package", "newpkg", "-add", "user1"}, + expectedOut: `{"newpkg": ["user1"]}`, + }, + { + name: "unreadable file (is a directory)", + isDir: true, + params: []string{"-rm"}, + expectedError: "is a directory", + }, + { + name: "remove user from non-existent package", + inData: `{"pkg1": ["user1"]}`, + params: []string{"-package", "pkg2", "-rm", "user2"}, + expectedError: "No package pkg2 and not adding one.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + oldWd, _ := os.Getwd() + _ = os.Chdir(dir) + defer os.Chdir(oldWd) + + targetFile := common.MaintainershipFile + if tt.name == "lint specific file" { + targetFile = "other.json" + } + + if tt.isDir { + _ = os.Mkdir(targetFile, 0755) + } else if tt.inData != "" { + _ = os.WriteFile(targetFile, []byte(tt.inData), 0644) + } + + flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ContinueOnError) + + os.Args = append([]string{"cmd"}, tt.params...) + err := run() + + if tt.expectedError != "" { + if err == nil { + t.Fatalf("expected error containing %q, but got none", tt.expectedError) + } + if !strings.Contains(err.Error(), tt.expectedError) { + t.Fatalf("expected error containing %q, got %q", tt.expectedError, err.Error()) + } + } else if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if tt.expectedOut != "" { + data, _ := os.ReadFile(targetFile) + var got, expected map[string][]string + _ = json.Unmarshal(data, &got) + _ = json.Unmarshal([]byte(tt.expectedOut), &expected) + + if len(got) == 0 && len(expected) == 0 { + return + } + + if !reflect.DeepEqual(got, expected) { + t.Fatalf("expected %v, got %v", expected, got) + } + } + }) + } +} + +func TestMainRecursive(t *testing.T) { + tests := []struct { + name string + inData string + expectedOut string + params []string + expectExit bool + }{ + { + name: "test main() via recursive call", + inData: `{"pkg1": ["user1"]}`, + params: []string{"-package", "pkg1", "-add", "user2"}, + expectedOut: `{"pkg1": ["user1", "user2"]}`, + }, + { + name: "test main() failure", + params: []string{"-package", "pkg1"}, + expectExit: true, + }, + } + + exe, _ := os.Executable() + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + oldWd, _ := os.Getwd() + _ = os.Chdir(dir) + defer os.Chdir(oldWd) + + if tt.inData != "" { + _ = os.WriteFile(common.MaintainershipFile, []byte(tt.inData), 0644) + } + + cmd := exec.Command(exe, append([]string{"-test.run=None"}, tt.params...)...) + cmd.Env = append(os.Environ(), "BE_MAIN=1") + out, runErr := cmd.CombinedOutput() + + if tt.expectExit { + if runErr == nil { + t.Fatalf("expected exit with error, but it succeeded") + } + return + } + + if runErr != nil { + t.Fatalf("unexpected error: %v: %s", runErr, string(out)) + } + + if tt.expectedOut != "" { + data, _ := os.ReadFile(common.MaintainershipFile) + var got, expected map[string][]string + _ = json.Unmarshal(data, &got) + _ = json.Unmarshal([]byte(tt.expectedOut), &expected) + + if !reflect.DeepEqual(got, expected) { + t.Fatalf("expected %v, got %v", expected, got) + } + } + }) + } +}