From fbd4d0494c59ad4e61714ff4cb569da375f94b4e Mon Sep 17 00:00:00 2001 From: Thomas Rooney Date: Tue, 21 Nov 2023 16:11:51 +0000 Subject: [PATCH] fix: reduce spurious releases (#72) * fix: reduce spurious releases * chore: better messaging * chore: also pass in docs checksum --- go.mod | 1 + go.sum | 8 +- internal/cli/cli.go | 4 + internal/generate/generate.go | 9 +- internal/git/diff.go | 72 ++------ internal/git/diff_test.go | 336 ++++++++++++++++++++++++---------- internal/git/git.go | 20 +- 7 files changed, 283 insertions(+), 167 deletions(-) diff --git a/go.mod b/go.mod index 2cbc27c4..cec0f9dc 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/google/go-github/v54 v54.0.0 github.com/hashicorp/go-version v1.6.0 github.com/pb33f/libopenapi v0.13.8 + github.com/speakeasy-api/git-diff-parser v0.0.3 github.com/speakeasy-api/sdk-gen-config v1.2.0 github.com/stretchr/testify v1.8.4 golang.org/x/exp v0.0.0-20230811145659-89c5cff77bcb diff --git a/go.sum b/go.sum index bcecb16f..3e52bd93 100644 --- a/go.sum +++ b/go.sum @@ -112,8 +112,6 @@ github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1y github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY= github.com/onsi/gomega v1.19.0 h1:4ieX6qQjPP/BfC3mpsAtIGGlxTWPeA3Inl/7DtXw1tw= github.com/onsi/gomega v1.19.0/go.mod h1:LY+I3pBVzYsTBU1AnDwOSxaYi9WoWiqgwooUqq9yPro= -github.com/pb33f/libopenapi v0.12.1 h1:DRhgbg1t32OSYoHT/Bk3stVruqquAkKj+S+OqOQIbBc= -github.com/pb33f/libopenapi v0.12.1/go.mod h1:s8uj6S0DjWrwZVj20ianJBz+MMjHAbeeRYNyo9ird74= github.com/pb33f/libopenapi v0.13.8 h1:PzBTpN1jdTi/XJpHGiRomxSHfQHdabcasGXW8yEhIjw= github.com/pb33f/libopenapi v0.13.8/go.mod h1:Lv2eEtsAtbRFlF8hjH82L8SIGoUNgemMVoKoB6A9THk= github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= @@ -129,8 +127,10 @@ github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ= github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= github.com/sirupsen/logrus v1.4.1/go.mod h1:ni0Sbl8bgC9z8RoU9G6nDWqqs/fq4eDPysMBDgk/93Q= github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= -github.com/speakeasy-api/sdk-gen-config v1.1.0 h1:gSrzInsnw8tQh/FtZQJYyNo3gQ1Nu6Sm2OUp0BazMYI= -github.com/speakeasy-api/sdk-gen-config v1.1.0/go.mod h1:94mmqrM5tP6/O8triQo9PIb6YYqihtKeoH70uwQ+JF4= +github.com/speakeasy-api/git-diff-parser v0.0.2 h1:88LebHKNede37yhQbODE+jVkNmvd33GqOTM3YEzNsJY= +github.com/speakeasy-api/git-diff-parser v0.0.2/go.mod h1:P46HmmVVmwA9P8h2wa0fDpmRM8/grbVQ+uKhWDtpkIY= +github.com/speakeasy-api/git-diff-parser v0.0.3 h1:LL12d+HMtSyj6O/hQqIn/lgDPYI6ci/DEhk0la/xA+0= +github.com/speakeasy-api/git-diff-parser v0.0.3/go.mod h1:P46HmmVVmwA9P8h2wa0fDpmRM8/grbVQ+uKhWDtpkIY= github.com/speakeasy-api/sdk-gen-config v1.2.0 h1:u5UCrS63TXpNPgsBqU4iSmG0ZnFjKg7gDC8ICKVviu0= github.com/speakeasy-api/sdk-gen-config v1.2.0/go.mod h1:94mmqrM5tP6/O8triQo9PIb6YYqihtKeoH70uwQ+JF4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/internal/cli/cli.go b/internal/cli/cli.go index b0e88433..a576d57c 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -232,6 +232,10 @@ func Suggest(docPath, maxSuggestions, docOutputPath string) (string, error) { } func Generate(docPath, lang, outputDir, installationURL string, published, outputTests bool, repoURL, repoSubDirectory string) error { + outputDir, err := filepath.Abs(outputDir) + if err != nil { + return err + } args := []string{ "generate", "sdk", diff --git a/internal/generate/generate.go b/internal/generate/generate.go index c4a328d1..3681e6dc 100644 --- a/internal/generate/generate.go +++ b/internal/generate/generate.go @@ -40,7 +40,7 @@ var ( ) type Git interface { - CheckDirDirty(dir string, ignoreMap map[string]string) (bool, error) + CheckDirDirty(dir string, ignoreMap map[string]string) (bool, string, error) } func Generate(g Git) (*GenerationInfo, map[string]string, error) { @@ -158,8 +158,10 @@ func Generate(g Git) (*GenerationInfo, map[string]string, error) { outputs[fmt.Sprintf("%s_directory", lang)] = dirForOutput - dirty, err := g.CheckDirDirty(dir, map[string]string{ - previousVersion: newVersion, + dirty, dirtyMsg, err := g.CheckDirDirty(dir, map[string]string{ + sdkVersion: newVersion, + cfg.Config.Management.GenerationVersion: generationVersion.String(), + cfg.Config.Management.DocChecksum: docChecksum, }) if err != nil { return nil, outputs, err @@ -167,6 +169,7 @@ func Generate(g Git) (*GenerationInfo, map[string]string, error) { if dirty { langGenerated[lang] = true + fmt.Printf("Regenerating %s SDK resulted in significant changes %s\n", lang, dirtyMsg) } else { langCfg.Version = sdkVersion cfg.Config.Languages[lang] = langCfg diff --git a/internal/git/diff.go b/internal/git/diff.go index f4168e04..c87263c0 100644 --- a/internal/git/diff.go +++ b/internal/git/diff.go @@ -2,69 +2,37 @@ package git import ( "fmt" - "regexp" "strings" + diffParser "github.com/speakeasy-api/git-diff-parser" "github.com/speakeasy-api/sdk-generation-action/internal/environment" ) -var ( - fileBoundaryRegex = regexp.MustCompile(`(?m)^diff --git a\/.*? b\/.*?$`) - fileNameRegex = regexp.MustCompile(`(?m)^--- a\/(.*?)$`) - versionChangeRegex = regexp.MustCompile(`_?(sdk|gen|Gen|SDK)_?[vV]ersion`) - userAgentRegex = regexp.MustCompile(`speakeasy-sdk/`) -) - -func IsGitDiffSignificant(diff string, ignoreChangePatterns map[string]string) bool { +func IsGitDiffSignificant(diff string, ignoreChangePatterns map[string]string) (bool, string, error) { if environment.ForceGeneration() { - return true + return true, "", nil } - diffs := fileBoundaryRegex.Split(diff, -1) - - significantChanges := false - -outer: - for _, diff := range diffs { - if strings.TrimSpace(diff) == "" { - continue + isSignificant, signifanceMsg, err := diffParser.SignificantChange(diff, func(diff *diffParser.FileDiff, change *diffParser.ContentChange) (bool, string) { + if diff.ToFile == "gen.yaml" || diff.ToFile == "RELEASES.md" { + return false, "" } - - matches := fileNameRegex.FindStringSubmatch(diff) - if len(matches) != 2 { - continue - } - - filename := fileNameRegex.FindStringSubmatch(diff)[1] - if strings.Contains(filename, "gen.yaml") { - continue + if change.Type == diffParser.ContentChangeTypeNOOP { + return false, "" } - - lines := strings.Split(diff, "\n") - for i, line := range lines { - isAddition := strings.HasPrefix(line, "+ ") || strings.HasPrefix(line, "+\t") - isSpecificPatternIgnored := false - if i > 1 && isAddition && strings.HasPrefix(lines[i-1], "- ") { - priorLine := lines[i-1] - for fromPattern, toPattern := range ignoreChangePatterns { - if strings.Contains(priorLine, fromPattern) && strings.Contains(line, toPattern) { - isSpecificPatternIgnored = true - break - } - } - - } - isNotVersionChange := !versionChangeRegex.MatchString(line) - isNotUAChange := !userAgentRegex.MatchString(line) - - significantChanges = isAddition && isNotVersionChange && isNotUAChange && !isSpecificPatternIgnored - - if significantChanges { - fmt.Println(line) - break outer + for pattern, replacement := range ignoreChangePatterns { + if strings.Contains(change.From, pattern) && strings.Contains(change.To, replacement) { + return false, "" } } - } + if diff.Type == diffParser.FileDiffTypeModified { + return true, fmt.Sprintf("significant diff %#v", diff) + } - return significantChanges + return true, fmt.Sprintf("significant change %#v in %s", change, diff.ToFile) + }) + if err != nil { + return true, "", fmt.Errorf("failed to parse diff: %w", err) + } + return isSignificant, signifanceMsg, nil } diff --git a/internal/git/diff_test.go b/internal/git/diff_test.go index 9bf8bcfc..0fb3f39a 100644 --- a/internal/git/diff_test.go +++ b/internal/git/diff_test.go @@ -7,130 +7,268 @@ import ( ) func TestIsGitDiffSignificant(t *testing.T) { - type args struct { - diff string - } tests := []struct { - name string - args args - want bool + name string + diff string + ignorePatterns map[string]string + want bool }{ { - name: "detects no significant changes", - args: args{ - diff: `diff --git a/gen.yaml b/gen.yaml -index 322c845..585bc5b 100644 + name: "gen.yaml changes are never significant", + diff: ` +diff --git a/gen.yaml b/gen.yaml +index 73093ef..000ec5e 100644 --- a/gen.yaml +++ b/gen.yaml -@@ -9,5 +9,5 @@ generation: - sdkClassName: SDK - sdkFlattening: false - go: -- version: 1.3.0 -+ version: 1.3.1 - packageName: github.com/speakeasy-api/sdk-generation-action-test-repo -diff --git a/sdk.go b/sdk.go -index b26db52..fdc01f4 100755 ---- a/sdk.go -+++ b/sdk.go -@@ -120,7 +120,7 @@ func WithSecurity(security shared.Security) SDKOption { - func New(opts ...SDKOption) *SDK { - sdk := &SDK{ - _language: "go", -- _sdkVersion: "1.3.0", -+ _sdkVersion: "1.3.1", - _genVersion: "1.12.7", -+ _userAgent: "speakeasy-sdk/go 0.0.1 2.155.1 0.1.0-alpha openapi" - } - for _, opt := range opts { +@@ -4,6 +4,7 @@ management: + docVersion: 0.1.0 +~ + speakeasyVersion: 1.120.4 +~ + generationVersion: 2.192.3 +~ ++# I don't matter. +~ + generation: +~ + comments: {} +~ + sdkClassName: examplePackage +~ `, - }, want: false, }, { - name: "detects significant changes", - args: args{ - diff: `diff --git a/gen.yaml b/gen.yaml -index 322c845..585bc5b 100644 + name: "code changes matter ; and when combined with gen.yaml changes, they are significant", + diff: ` +diff --git a/gen.yaml b/gen.yaml +index 73093ef..000ec5e 100644 --- a/gen.yaml +++ b/gen.yaml -@@ -9,5 +9,5 @@ generation: - sdkClassName: SDK - sdkFlattening: false - go: -- version: 1.3.0 -+ version: 1.3.1 - packageName: github.com/speakeasy-api/sdk-generation-action-test-repo -diff --git a/sdk.go b/sdk.go -index b26db52..fdc01f4 100755 ---- a/sdk.go -+++ b/sdk.go -@@ -120,7 +120,7 @@ func WithSecurity(security shared.Security) SDKOption { - func New(opts ...SDKOption) *SDK { - sdk := &SDK{ -- _language: "go", -+ _language: "crazygo", - _sdkVersion: "1.3.0", - _genVersion: "1.12.7", - } - for _, opt := range opts { +@@ -4,6 +4,7 @@ management: + docVersion: 0.1.0 +~ + speakeasyVersion: 1.120.4 +~ + generationVersion: 2.192.3 +~ ++# I don't matter. +~ + generation: +~ + comments: {} +~ + sdkClassName: examplePackage +~ +diff --git a/internal/planmodifiers/boolplanmodifier/suppress_diff.go b/internal/planmodifiers/boolplanmodifier/suppress_diff.go +index 395e4b2..dcdc177 100644 +--- a/internal/planmodifiers/boolplanmodifier/suppress_diff.go ++++ b/internal/planmodifiers/boolplanmodifier/suppress_diff.go +@@ -2,6 +2,9 @@ + +~ + package boolplanmodifier +~ + +~ ++// code changes do matter +~ ++// multiline too +~ +~ + import ( +~ + "context" +~ + "github.com/exampleAuthor/terraform-provider-examplePackage/internal/planmodifiers/utils" +~ `, + ignorePatterns: map[string]string{ + "1.120.4": "1.120.5", }, want: true, }, { - name: "detects significant changes with tabs for spacing", - args: args{ - // Important: Preserve tabs in the follow diff - diff: `diff --git a/gen.yaml b/gen.yaml -index 322c845..585bc5b 100644 + name: "ignores a version number change, even when compiled into an unusual line", + diff: `diff --git a/README.md b/README.md +index 3db161d..f1ae144 100755 +--- a/README.md ++++ b/README.md +@@ -10,7 +10,7 @@ terraform { + required_providers { +~ + examplePackage = { +~ + source = "exampleAuthor/examplePackage" +~ + version = +-"0.13.5" ++"0.13.6" +~ + } +~ + } +~ + } +~ +diff --git a/RELEASES.md b/RELEASES.md +index e30e211..84bc6ed 100644 +--- a/RELEASES.md ++++ b/RELEASES.md +@@ -206,4 +206,12 @@ Based on: + - OpenAPI Doc 0.1.0 +~ + - Speakeasy CLI 1.120.3 (2.192.1) https://github.com/speakeasy-api/speakeasy +~ + ### Generated +~ + - [terraform v0.13.5] . +~ +~ ++## 2023-11-17 00:37:46 +~ ++### Changes +~ ++Based on: +~ ++- OpenAPI Doc 0.1.0 +~ ++- Speakeasy CLI 1.120.4 (2.192.3) https://github.com/speakeasy-api/speakeasy +~ ++### Generated +~ ++- [terraform v0.13.6] . +~ +diff --git a/docs/index.md b/docs/index.md +index d6b3918..931847a 100644 +--- a/docs/index.md ++++ b/docs/index.md +@@ -17,7 +17,7 @@ terraform { + required_providers { +~ + examplePackage = { +~ + source = "exampleAuthor/examplePackage" +~ + version = +-"0.13.5" ++"0.13.6" +~ + } +~ + } +~ + } +~ +diff --git a/examples/provider/provider.tf b/examples/provider/provider.tf +index 92f99fe..b32e0fd 100644 +--- a/examples/provider/provider.tf ++++ b/examples/provider/provider.tf +@@ -2,7 +2,7 @@ terraform { + required_providers { +~ + examplePackage = { +~ + source = "exampleAuthor/examplePackage" +~ + version = +-"0.13.5" ++"0.13.6" +~ + } +~ + } +~ + } +~ +diff --git a/gen.yaml b/gen.yaml +index de293be..73093ef 100644 --- a/gen.yaml +++ b/gen.yaml -@@ -9,5 +9,5 @@ generation: - sdkClassName: SDK - sdkFlattening: false - go: -- version: 1.3.0 -+ version: 1.3.1 - packageName: github.com/speakeasy-api/sdk-generation-action-test-repo -diff --git a/sdk.go b/sdk.go -index b26db52..fdc01f4 100755 ---- a/sdk.go -+++ b/sdk.go -@@ -120,7 +120,7 @@ func WithSecurity(security shared.Security) SDKOption { - func New(opts ...SDKOption) *SDK { - sdk := &SDK{ -- language: "go", -+ _language: "crazygo", - _sdkVersion: "1.3.0", - _genVersion: "1.12.7", - } - for _, opt := range opts { -`, - }, - want: true, - }, - { - name: "ignores a version number change, even when compiled into an unusual line", - args: args{ - // Important: Preserve tabs in the follow diff - diff: `diff --git a/gen.yaml b/gen.yaml -index 322c845..585bc5b 100644 ---- a/useragent.go -+++ b/useragent.go -- useragent := "%s/go 1.3.2 2.155.1 0.1.0-alpha openapi" -+ useragent := "%s/go 1.3.3 2.155.1 0.1.0-alpha openapi" +@@ -1,9 +1,9 @@ + configVersion: 1.0.0 +~ + management: +~ + docChecksum: +-123e1a1abd36f630cf6d5432e0649e38 ++c0a14e297e308a50b297260c06a1bfe0 +~ + docVersion: 0.1.0 +~ + speakeasyVersion: +-1.120.3 ++1.120.4 +~ + generationVersion: +-2.192.1 ++2.192.3 +~ + generation: +~ + comments: {} +~ + sdkClassName: examplePackage +~ +@@ -21,7 +21,7 @@ features: + nameOverrides: 2.81.1 +~ + unions: 2.81.5 +~ + terraform: +~ + version: +-0.13.5 ++0.13.6 +~ + author: exampleAuthor +~ + imports: +~ + option: openapi +~ +diff --git a/internal/sdk/sdk.go b/internal/sdk/sdk.go +index c093fc3..67a2a32 100644 +--- a/internal/sdk/sdk.go ++++ b/internal/sdk/sdk.go +@@ -151,9 +151,9 @@ func New(opts ...SDKOption) *examplePackage { + sdkConfiguration: sdkConfiguration{ +~ + Language: "go", +~ + OpenAPIDocVersion: "0.1.0", +~ + SDKVersion: +-"0.13.5", ++"0.13.6", +~ + GenVersion: +-"2.192.1", ++"2.192.3", +~ + UserAgent: "speakeasy-sdk/go +-0.13.5 2.192.1 ++0.13.6 2.192.3 + 0.1.0 examplePackage", +~ + }, +~ + } +~ + for _, opt := range opts { +~ `, + ignorePatterns: map[string]string{ + "0.13.5": "0.13.6", + "2.192.1": "2.192.3", }, want: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := IsGitDiffSignificant(tt.args.diff, map[string]string{ - // example version number change - "1.3.2": "1.3.3", - }) + got, _, err := IsGitDiffSignificant(tt.diff, tt.ignorePatterns) + assert.NoError(t, err) assert.Equal(t, tt.want, got) }) } diff --git a/internal/git/git.go b/internal/git/git.go index 3abfb548..48255af9 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -80,19 +80,19 @@ func (g *Git) CloneRepo() error { return nil } -func (g *Git) CheckDirDirty(dir string, ignoreChangePatterns map[string]string) (bool, error) { +func (g *Git) CheckDirDirty(dir string, ignoreChangePatterns map[string]string) (bool, string, error) { if g.repo == nil { - return false, fmt.Errorf("repo not cloned") + return false, "", fmt.Errorf("repo not cloned") } w, err := g.repo.Worktree() if err != nil { - return false, fmt.Errorf("error getting worktree: %w", err) + return false, "", fmt.Errorf("error getting worktree: %w", err) } status, err := w.Status() if err != nil { - return false, fmt.Errorf("error getting status: %w", err) + return false, "", fmt.Errorf("error getting status: %w", err) } cleanedDir := path.Clean(dir) @@ -102,6 +102,7 @@ func (g *Git) CheckDirDirty(dir string, ignoreChangePatterns map[string]string) changesFound := false fileChangesFound := false + newFiles := []string{} for f, s := range status { if strings.Contains(f, "gen.yaml") { @@ -115,6 +116,7 @@ func (g *Git) CheckDirDirty(dir string, ignoreChangePatterns map[string]string) case git.Deleted: fallthrough case git.Untracked: + newFiles = append(newFiles, f) fileChangesFound = true case git.Modified: fallthrough @@ -134,19 +136,19 @@ func (g *Git) CheckDirDirty(dir string, ignoreChangePatterns map[string]string) } if fileChangesFound { - return true, nil + return true, fmt.Sprintf("new file found: %#v", newFiles), nil } if !changesFound { - return false, nil + return false, "", nil } - diffOutput, err := runGitCommand("diff") + diffOutput, err := runGitCommand("diff", "--word-diff=porcelain") if err != nil { - return false, fmt.Errorf("error running git diff: %w", err) + return false, "", fmt.Errorf("error running git diff: %w", err) } - return IsGitDiffSignificant(diffOutput, ignoreChangePatterns), nil + return IsGitDiffSignificant(diffOutput, ignoreChangePatterns) } func (g *Git) FindExistingPR(branchName string, action environment.Action) (string, *github.PullRequest, error) {