From abfe8ddb92217ba9cce9248ca7d1a92aca75b5e5 Mon Sep 17 00:00:00 2001 From: Cody Oss <6331106+codyoss@users.noreply.github.com> Date: Wed, 24 May 2023 12:38:28 -0500 Subject: [PATCH] chore: fix genbot (#7982) - Fix error: "Tried to write the same file twice." - remove some cruft - skip ai libs until we explicitly onboard - update some deps --- internal/gapicgen/cmd/genbot/Dockerfile | 4 ++-- internal/gapicgen/cmd/genbot/local.go | 30 ++++++++++-------------- internal/gapicgen/cmd/genbot/main.go | 16 +++++-------- internal/gapicgen/generator/generator.go | 24 ++++++++----------- internal/gapicgen/generator/genproto.go | 20 +++++++--------- 5 files changed, 40 insertions(+), 54 deletions(-) diff --git a/internal/gapicgen/cmd/genbot/Dockerfile b/internal/gapicgen/cmd/genbot/Dockerfile index 1de0a051ba0f..3742e22e7678 100644 --- a/internal/gapicgen/cmd/genbot/Dockerfile +++ b/internal/gapicgen/cmd/genbot/Dockerfile @@ -27,11 +27,11 @@ ENV PATH /usr/local/go/bin:$PATH RUN go version # Install Go tools. -RUN go install github.com/golang/protobuf/protoc-gen-go@v1.5.2 && \ +RUN go install github.com/golang/protobuf/protoc-gen-go@v1.5.3 && \ go install golang.org/x/lint/golint@latest && \ go install golang.org/x/tools/cmd/goimports@latest && \ go install honnef.co/go/tools/cmd/staticcheck@latest && \ - go install github.com/googleapis/gapic-generator-go/cmd/protoc-gen-go_gapic@v0.35.2 + go install github.com/googleapis/gapic-generator-go/cmd/protoc-gen-go_gapic@v0.37.0 ENV PATH="${PATH}:/root/go/bin" # Source: http://debuggable.com/posts/disable-strict-host-checking-for-git-clone:49896ff3-0ac0-4263-9703-1eae4834cda3 diff --git a/internal/gapicgen/cmd/genbot/local.go b/internal/gapicgen/cmd/genbot/local.go index e05fa4ae1ec8..b22719071d07 100644 --- a/internal/gapicgen/cmd/genbot/local.go +++ b/internal/gapicgen/cmd/genbot/local.go @@ -29,14 +29,12 @@ import ( ) type localConfig struct { - googleapisDir string - genprotoDir string - protoDir string - gapicToGenerate string - onlyGapics bool - regenOnly bool - forceAll bool - genAlias bool + googleapisDir string + genprotoDir string + protoDir string + regenOnly bool + forceAll bool + genAlias bool } func genLocal(ctx context.Context, c localConfig) error { @@ -61,15 +59,13 @@ func genLocal(ctx context.Context, c localConfig) error { // Regen. conf := &generator.Config{ - GoogleapisDir: defaultDir(tmpGoogleapisDir, c.googleapisDir), - GenprotoDir: defaultDir(tmpGenprotoDir, c.genprotoDir), - ProtoDir: defaultDir(tmpProtoDir, c.protoDir), - GapicToGenerate: c.gapicToGenerate, - OnlyGenerateGapic: c.onlyGapics, - LocalMode: true, - RegenOnly: c.regenOnly, - ForceAll: c.forceAll, - GenAlias: c.genAlias, + GoogleapisDir: defaultDir(tmpGoogleapisDir, c.googleapisDir), + GenprotoDir: defaultDir(tmpGenprotoDir, c.genprotoDir), + ProtoDir: defaultDir(tmpProtoDir, c.protoDir), + LocalMode: true, + RegenOnly: c.regenOnly, + ForceAll: c.forceAll, + GenAlias: c.genAlias, } if _, err := generator.Generate(ctx, conf); err != nil { log.Printf("Generator ran (and failed) in %s\n", tmpDir) diff --git a/internal/gapicgen/cmd/genbot/main.go b/internal/gapicgen/cmd/genbot/main.go index 4dfbf27ab880..5c84b1e721d0 100644 --- a/internal/gapicgen/cmd/genbot/main.go +++ b/internal/gapicgen/cmd/genbot/main.go @@ -48,8 +48,6 @@ func main() { googleapisDir := flag.String("googleapis-dir", os.Getenv("GOOGLEAPIS_DIR"), "Directory where sources of googleapis/googleapis resides. If unset the sources will be cloned to a temporary directory that is not cleaned up.") genprotoDir := flag.String("genproto-dir", os.Getenv("GENPROTO_DIR"), "Directory where sources of googleapis/go-genproto resides. If unset the sources will be cloned to a temporary directory that is not cleaned up.") protoDir := flag.String("proto-dir", os.Getenv("PROTO_DIR"), "Directory where sources of google/protobuf resides. If unset the sources will be cloned to a temporary directory that is not cleaned up.") - gapicToGenerate := flag.String("gapic", os.Getenv("GAPIC_TO_GENERATE"), `Specifies which gapic to generate. The value should be in the form of an import path (Ex: cloud.google.com/go/pubsub/apiv1). The default "" generates all gapics.`) - onlyGapics := flag.Bool("only-gapics", strToBool(os.Getenv("ONLY_GAPICS")), "Enabling stops regenerating genproto.") regenOnly := flag.Bool("regen-only", strToBool(os.Getenv("REGEN_ONLY")), "Enabling means no vetting, manifest updates, or compilation.") genAlias := flag.Bool("generate-alias", strToBool(os.Getenv("GENERATE_ALIAS")), "Enabling means alias files will be generated.") @@ -57,14 +55,12 @@ func main() { if *localMode { if err := genLocal(ctx, localConfig{ - googleapisDir: *googleapisDir, - genprotoDir: *genprotoDir, - protoDir: *protoDir, - gapicToGenerate: *gapicToGenerate, - onlyGapics: *onlyGapics, - regenOnly: *regenOnly, - forceAll: *forceAll, - genAlias: *genAlias, + googleapisDir: *googleapisDir, + genprotoDir: *genprotoDir, + protoDir: *protoDir, + regenOnly: *regenOnly, + forceAll: *forceAll, + genAlias: *genAlias, }); err != nil { log.Fatal(err) } diff --git a/internal/gapicgen/generator/generator.go b/internal/gapicgen/generator/generator.go index 19731ac24f45..9290811e583b 100644 --- a/internal/gapicgen/generator/generator.go +++ b/internal/gapicgen/generator/generator.go @@ -29,24 +29,20 @@ import ( // Config contains inputs needed to generate sources. type Config struct { - GoogleapisDir string - GenprotoDir string - ProtoDir string - GapicToGenerate string - OnlyGenerateGapic bool - LocalMode bool - RegenOnly bool - ForceAll bool - GenAlias bool + GoogleapisDir string + GenprotoDir string + ProtoDir string + LocalMode bool + RegenOnly bool + ForceAll bool + GenAlias bool } // Generate generates genproto and gapics. func Generate(ctx context.Context, conf *Config) ([]*git.ChangeInfo, error) { - if !conf.OnlyGenerateGapic { - protoGenerator := NewGenprotoGenerator(conf) - if err := protoGenerator.Regen(ctx); err != nil { - return nil, fmt.Errorf("error generating genproto (may need to check logs for more errors): %v", err) - } + protoGenerator := NewGenprotoGenerator(conf) + if err := protoGenerator.Regen(ctx); err != nil { + return nil, fmt.Errorf("error generating genproto (may need to check logs for more errors): %v", err) } var changes []*git.ChangeInfo diff --git a/internal/gapicgen/generator/genproto.go b/internal/gapicgen/generator/genproto.go index 34bcdd9d83b8..861724b00bdf 100644 --- a/internal/gapicgen/generator/genproto.go +++ b/internal/gapicgen/generator/genproto.go @@ -35,27 +35,26 @@ var goPkgOptRe = regexp.MustCompile(`(?m)^option go_package = (.*);`) // GenprotoGenerator is used to generate code for googleapis/go-genproto. type GenprotoGenerator struct { - genprotoDir string - googleapisDir string - protoSrcDir string - gapicToGenerate string - forceAll bool + genprotoDir string + googleapisDir string + protoSrcDir string + forceAll bool } // NewGenprotoGenerator creates a new GenprotoGenerator. func NewGenprotoGenerator(c *Config) *GenprotoGenerator { return &GenprotoGenerator{ - genprotoDir: c.GenprotoDir, - googleapisDir: c.GoogleapisDir, - protoSrcDir: filepath.Join(c.ProtoDir, "/src"), - gapicToGenerate: c.GapicToGenerate, - forceAll: c.ForceAll, + genprotoDir: c.GenprotoDir, + googleapisDir: c.GoogleapisDir, + protoSrcDir: filepath.Join(c.ProtoDir, "/src"), + forceAll: c.ForceAll, } } // TODO: consider flipping this to an allowlist var skipPrefixes = []string{ "google.golang.org/genproto/googleapis/ads/", + "google.golang.org/genproto/googleapis/ai/", "google.golang.org/genproto/googleapis/analytics/", "google.golang.org/genproto/googleapis/api/servicecontrol/", "google.golang.org/genproto/googleapis/api/servicemanagement/", @@ -190,7 +189,6 @@ func goPkg(fileName string) (string, error) { func (g *GenprotoGenerator) protoc(fileNames []string) error { args := []string{ "--experimental_allow_proto3_optional", - fmt.Sprintf("--go_out=%s/generated", g.genprotoDir), fmt.Sprintf("--go_out=plugins=grpc:%s/generated", g.genprotoDir), "-I", g.googleapisDir, "-I", g.protoSrcDir,