From 4070949859655c21faf4d9989dfbb10f190abdc7 Mon Sep 17 00:00:00 2001 From: Sally McGrath Date: Fri, 19 Jan 2024 08:12:52 -0600 Subject: [PATCH 1/3] Daniel's tooling with path changed from curriculum-labs to curriculum no other changes Co-authored by daniel@codeyourfuture.io --- .../go/cmd/local-overrides-enforcer/main.go | 74 +++++++++++++++++++ tooling/go/go.mod | 11 +++ .../checker/checker_test.go | 70 ++++++++++++++++++ 3 files changed, 155 insertions(+) create mode 100644 tooling/go/cmd/local-overrides-enforcer/main.go create mode 100644 tooling/go/go.mod create mode 100644 tooling/go/internal/local-overrides-enforcer/checker/checker_test.go diff --git a/tooling/go/cmd/local-overrides-enforcer/main.go b/tooling/go/cmd/local-overrides-enforcer/main.go new file mode 100644 index 000000000..f261ac209 --- /dev/null +++ b/tooling/go/cmd/local-overrides-enforcer/main.go @@ -0,0 +1,74 @@ +package main + +import ( + "flag" + "fmt" + "io/fs" + "log" + "os" + "path/filepath" + "strings" + + "github.com/CodeYourFuture/curriculum/tooling/go/internal/local-overrides-enforcer/checker" +) + +func main() { + var rootDirectory string + flag.StringVar(&rootDirectory, "root-dir", ".", "Root directory to search for go.mod files in") + excludeDirectoriesFlag := flag.String("exclude", filepath.Join("tooling", "go"), "Directories to exclude from searches (comma-delimited)") + var parentModule string + flag.StringVar(&parentModule, "parent-module", "github.com/CodeYourFuture/curriculum", "Parent module to search for missing overrides within") + + flag.Parse() + + var err error + rootDirectory, err = filepath.Abs(rootDirectory) + if err != nil { + log.Fatalf("Failed to get absolute path of root directory: %v", err) + } + + var excludeDirectories []string + for _, excludeDirectory := range strings.Split(*excludeDirectoriesFlag, ",") { + excludeDirectory, err = filepath.Abs(excludeDirectory) + if err != nil { + log.Fatalf("Failed to get absolute path of exclude directory: %v", err) + } + excludeDirectories = append(excludeDirectories, excludeDirectory) + } + + sawBadFile := false + + err = filepath.WalkDir(rootDirectory, func(path string, d fs.DirEntry, err error) error { + for _, excluded := range excludeDirectories { + if path == excluded { + return fs.SkipDir + } + } + if err != nil { + return err + } + + if d.Name() != "go.mod" { + return nil + } + content, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("failed to read %s: %w", path, err) + } + expectedContents, ok, err := checker.CheckFile(path, content, parentModule) + if err != nil { + return fmt.Errorf("failed to check %s: %w", path, err) + } + if !ok { + sawBadFile = true + fmt.Printf("⚠️ File at path %s didn't have some local overrides - its contents should be:\n%s\n", path, expectedContents) + } + return nil + }) + if err != nil { + log.Fatalf("Error walking filesystem: %v", err) + } + if sawBadFile { + os.Exit(1) + } +} diff --git a/tooling/go/go.mod b/tooling/go/go.mod new file mode 100644 index 000000000..a5007ba4c --- /dev/null +++ b/tooling/go/go.mod @@ -0,0 +1,11 @@ +module github.com/CodeYourFuture/curriculum/tooling/go + +go 1.21.5 + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/stretchr/testify v1.8.4 // indirect + golang.org/x/mod v0.14.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/tooling/go/internal/local-overrides-enforcer/checker/checker_test.go b/tooling/go/internal/local-overrides-enforcer/checker/checker_test.go new file mode 100644 index 000000000..a358296b4 --- /dev/null +++ b/tooling/go/internal/local-overrides-enforcer/checker/checker_test.go @@ -0,0 +1,70 @@ +package checker_test + +import ( + "testing" + + "github.com/CodeYourFuture/curriculum/tooling/go/internal/local-overrides-enforcer/checker" + "github.com/stretchr/testify/require" +) + +func TestCorrect(t *testing.T) { + goModContent := `module github.com/CodeYourFuture/curriculum/org-cyf + +go 1.21.3 + +replace github.com/CodeYourFuture/curriculum/common-content => ../common-content +replace github.com/CodeYourFuture/curriculum/common-theme => ../common-theme + +require ( + github.com/CodeYourFuture/curriculum/common-content v0.0.0-20240103071042-5b2177342232 // indirect + github.com/CodeYourFuture/curriculum/common-theme v0.0.0-20240103071042-5b2177342232 // indirect +) +` + + newContent, ok, err := checker.CheckFile("/some/go.mod", []byte(goModContent), "github.com/CodeYourFuture/curriculum") + require.NoError(t, err) + require.True(t, ok) + require.Equal(t, "", newContent) +} + +func TestMissingReplace(t *testing.T) { + goModContent := `module github.com/CodeYourFuture/curriculum/org-cyf + + go 1.21.3 + + replace github.com/CodeYourFuture/curriculum/common-theme => ../common-theme + + require ( + github.com/CodeYourFuture/curriculum/common-content v0.0.0-20240103071042-5b2177342232 // indirect + github.com/CodeYourFuture/curriculum/common-theme v0.0.0-20240103071042-5b2177342232 // indirect + ) +` + + newContent, ok, err := checker.CheckFile("/some/go.mod", []byte(goModContent), "github.com/CodeYourFuture/curriculum") + require.NoError(t, err) + require.False(t, ok) + require.Contains(t, newContent, "replace github.com/CodeYourFuture/curriculum/common-content => ../common-content") +} + +func TestModuleNotChildOfParent(t *testing.T) { + goModContent := `module github.com/CodeYourFuture/wrong + +go 1.21.3 + +replace github.com/CodeYourFuture/curriculum/common-content => ../common-content +replace github.com/CodeYourFuture/curriculum/common-theme => ../common-theme + +require ( + github.com/CodeYourFuture/curriculum/common-content v0.0.0-20240103071042-5b2177342232 // indirect + github.com/CodeYourFuture/curriculum/common-theme v0.0.0-20240103071042-5b2177342232 // indirect +) +` + + _, _, err := checker.CheckFile("/some/go.mod", []byte(goModContent), "github.com/CodeYourFuture/curriculum") + require.ErrorContains(t, err, "module at path /some/go.mod was named github.com/CodeYourFuture/wrong which isn't a child of github.com/CodeYourFuture/curriculum") +} + +func TestInvalidGoModFile(t *testing.T) { + _, _, err := checker.CheckFile("/some/go.mod", []byte("hello"), "github.com/CodeYourFuture/curriculum") + require.ErrorContains(t, err, "failed to parse /some/go.mod as go.mod file: ") +} From 832005d150adb3197b3b742f6324fbd57b86fc0c Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Sat, 20 Jan 2024 22:02:24 +0000 Subject: [PATCH 2/3] Add missing tooling portions and CI scripts --- .github/workflows/check-consistency.yml | 20 +++++++ .github/workflows/test-tooling.yml | 18 ++++++ tooling/go/.gitignore | 1 + tooling/go/go.mod | 7 ++- tooling/go/go.sum | 12 ++++ .../checker/checker.go | 56 +++++++++++++++++++ 6 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/check-consistency.yml create mode 100644 .github/workflows/test-tooling.yml create mode 100644 tooling/go/.gitignore create mode 100644 tooling/go/go.sum create mode 100644 tooling/go/internal/local-overrides-enforcer/checker/checker.go diff --git a/.github/workflows/check-consistency.yml b/.github/workflows/check-consistency.yml new file mode 100644 index 000000000..cb6e999cb --- /dev/null +++ b/.github/workflows/check-consistency.yml @@ -0,0 +1,20 @@ +name: Check consistency + +on: + pull_request: + push: + workflow_dispatch: + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Setup Go + uses: actions/setup-go@v4 + with: + go-version: 1.21 + - name: Build tooling + run: (cd tooling/go && go build ./cmd/local-overrides-enforcer) + - name: Check consistency + run: ./tooling/go/local-overrides-enforcer diff --git a/.github/workflows/test-tooling.yml b/.github/workflows/test-tooling.yml new file mode 100644 index 000000000..9a932287a --- /dev/null +++ b/.github/workflows/test-tooling.yml @@ -0,0 +1,18 @@ +name: Test tooling + +on: + pull_request: + push: + workflow_dispatch: + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Setup Go + uses: actions/setup-go@v4 + with: + go-version: 1.21 + - name: Run tooling tests + run: cd tooling/go && go test ./... diff --git a/tooling/go/.gitignore b/tooling/go/.gitignore new file mode 100644 index 000000000..406bae00b --- /dev/null +++ b/tooling/go/.gitignore @@ -0,0 +1 @@ +/local-overrides-enforcer diff --git a/tooling/go/go.mod b/tooling/go/go.mod index a5007ba4c..e5bbc74ce 100644 --- a/tooling/go/go.mod +++ b/tooling/go/go.mod @@ -2,10 +2,13 @@ module github.com/CodeYourFuture/curriculum/tooling/go go 1.21.5 +require ( + github.com/stretchr/testify v1.8.4 + golang.org/x/mod v0.14.0 +) + require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/stretchr/testify v1.8.4 // indirect - golang.org/x/mod v0.14.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/tooling/go/go.sum b/tooling/go/go.sum new file mode 100644 index 000000000..d7cde5764 --- /dev/null +++ b/tooling/go/go.sum @@ -0,0 +1,12 @@ +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +golang.org/x/mod v0.14.0 h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0= +golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/tooling/go/internal/local-overrides-enforcer/checker/checker.go b/tooling/go/internal/local-overrides-enforcer/checker/checker.go new file mode 100644 index 000000000..7d6408924 --- /dev/null +++ b/tooling/go/internal/local-overrides-enforcer/checker/checker.go @@ -0,0 +1,56 @@ +package checker + +import ( + "fmt" + "strings" + + "golang.org/x/mod/modfile" +) + +// CheckFile checks that a go.mod file has local overrides for all children of the passed parent module. +// It returns one of: +// - If the contents was correct: "", true, nil +// - If the contents was not correct: the expected contents, false, nil +// - If an error occurred: "", false, error +func CheckFile(goModPath string, contents []byte, parentModule string) (string, bool, error) { + gomodFile, err := modfile.Parse(goModPath, contents, nil) + if err != nil { + return "", false, fmt.Errorf("failed to parse %s as go.mod file: %w", goModPath, err) + } + + parentModuleWithTrailingSlash := parentModule + "/" + + if !strings.HasPrefix(gomodFile.Module.Mod.Path, parentModuleWithTrailingSlash) { + return "", false, fmt.Errorf("module at path %s was named %s which isn't a child of %s", goModPath, gomodFile.Module.Mod.Path, parentModule) + } + slashCount := strings.Count(gomodFile.Module.Mod.Path[len(parentModule):], "/") + + replaces := make(map[string]struct{}) + for _, replace := range gomodFile.Replace { + replaces[replace.Old.Path] = struct{}{} + } + + missingReplaces := false + for _, require := range gomodFile.Require { + modPath := require.Mod.Path + if !strings.HasPrefix(modPath, parentModuleWithTrailingSlash) { + continue + } + if _, isReplaced := replaces[modPath]; isReplaced { + continue + } + missingReplaces = true + rel := modPath[len(parentModuleWithTrailingSlash):] + if err := gomodFile.AddReplace(modPath, "", strings.Repeat("../", slashCount)+rel, ""); err != nil { + return "", false, fmt.Errorf("failed to add replace: %w", err) + } + } + if missingReplaces { + formatted, err := gomodFile.Format() + if err != nil { + return "", false, fmt.Errorf("failed to serialize go.mod file: %w", err) + } + return string(formatted), false, nil + } + return "", true, nil +} From 408f0a0f1d18d0eca90fd368337f393486361b47 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Sat, 20 Jan 2024 22:03:37 +0000 Subject: [PATCH 3/3] Only build once per PR not twice --- .github/workflows/check-consistency.yml | 1 - .github/workflows/test-tooling.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/workflows/check-consistency.yml b/.github/workflows/check-consistency.yml index cb6e999cb..10eb7df0f 100644 --- a/.github/workflows/check-consistency.yml +++ b/.github/workflows/check-consistency.yml @@ -2,7 +2,6 @@ name: Check consistency on: pull_request: - push: workflow_dispatch: jobs: diff --git a/.github/workflows/test-tooling.yml b/.github/workflows/test-tooling.yml index 9a932287a..af6199388 100644 --- a/.github/workflows/test-tooling.yml +++ b/.github/workflows/test-tooling.yml @@ -2,7 +2,6 @@ name: Test tooling on: pull_request: - push: workflow_dispatch: jobs: