From 4ee3e938a5f383a90e5dfe2408daea33ea6316e7 Mon Sep 17 00:00:00 2001 From: jeronimoalbi Date: Thu, 28 Jul 2022 16:33:17 +0200 Subject: [PATCH 1/4] fix: change `protoc-gen-ts_proto` to generate in random folders The script has a fixed name to comply with `protoc` plugin naming requirements but it it always generated in the same temporary folder which means that concurrent calls would overwrite the `protoc-gen-ts_proto` file. This changeset generates the script in random temporary folders. This fix might help correcting "flaky" integration tests. --- .../pkg/nodetime/programs/ts-proto/tsproto.go | 45 ++++++++++++++----- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/ignite/pkg/nodetime/programs/ts-proto/tsproto.go b/ignite/pkg/nodetime/programs/ts-proto/tsproto.go index 8826344ab4..a3bf6e6676 100644 --- a/ignite/pkg/nodetime/programs/ts-proto/tsproto.go +++ b/ignite/pkg/nodetime/programs/ts-proto/tsproto.go @@ -10,7 +10,10 @@ import ( "github.com/ignite/cli/ignite/pkg/nodetime" ) -const pluginName = "protoc-gen-ts_proto" +const ( + pluginName = "protoc-gen-ts_proto" + scriptTemplate = "#!/bin/bash\n%s $@\n" +) // BinaryPath returns the path to the binary of the ts-proto plugin so it can be passed to // protoc via --plugin option. @@ -19,22 +22,44 @@ const pluginName = "protoc-gen-ts_proto" // will be protoc-gen-ts_proto. // see why: https://github.com/stephenh/ts-proto/blob/7f76c05/README.markdown#quickstart. func BinaryPath() (path string, cleanup func(), err error) { - var command []string + // Create binary for the TypeScript protobuf generator + command, cleanupBin, err := nodetime.Command(nodetime.CommandTSProto) + if err != nil { + return + } - command, cleanup, err = nodetime.Command(nodetime.CommandTSProto) + defer func() { + if err != nil { + cleanupBin() + } + }() + + // Create a random directory for the script that runs the TypeScript protobuf generator. + // This is required to avoid potential flaky integration tests caused by one concurrent + // test overwriting the generator script while it is being run in a separate test process. + tmpDir, err := os.MkdirTemp("", "ts_proto_plugin") if err != nil { return } - tmpdir := os.TempDir() - path = filepath.Join(tmpdir, pluginName) + cleanupScriptDir := func() { os.RemoveAll(tmpDir) } + + defer func() { + if err != nil { + cleanupScriptDir() + } + }() - // comforting protoc by giving protoc-gen-ts_proto name to the plugin's binary. - script := fmt.Sprintf(`#!/bin/bash -%s "$@" -`, strings.Join(command, " ")) + cleanup = func() { + cleanupBin() + cleanupScriptDir() + } + // Wrap the TypeScript protobuf generator in a script with a fixed name + // located in a random temporary directory. + script := fmt.Sprintf(scriptTemplate, strings.Join(command, " ")) + path = filepath.Join(tmpDir, pluginName) err = os.WriteFile(path, []byte(script), 0755) - return + return path, cleanup, err } From 476910179167aabbe25cf643d201ef2ce8a4e277 Mon Sep 17 00:00:00 2001 From: jeronimoalbi Date: Tue, 2 Aug 2022 15:28:50 +0200 Subject: [PATCH 2/4] refactor(pkg/protoc): add WithCommand option to Generate This allow calling Generate multiple times using a single protoc binary. Otherwise a protoc binary would be created each time Generate is called. --- ignite/cmd/tools.go | 2 +- ignite/pkg/protoc/protoc.go | 53 +++++++++++++++++++++++++++++-------- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/ignite/cmd/tools.go b/ignite/cmd/tools.go index df803e60f4..0462cc86f6 100644 --- a/ignite/cmd/tools.go +++ b/ignite/cmd/tools.go @@ -74,7 +74,7 @@ func toolsProtocProxy(cmd *cobra.Command, args []string) error { } defer cleanup() - return toolsProxy(cmd.Context(), append(command.Command, args...)) + return toolsProxy(cmd.Context(), append(command.Command(), args...)) } func toolsProxy(ctx context.Context, command []string) error { diff --git a/ignite/pkg/protoc/protoc.go b/ignite/pkg/protoc/protoc.go index f635206a41..a584893133 100644 --- a/ignite/pkg/protoc/protoc.go +++ b/ignite/pkg/protoc/protoc.go @@ -24,6 +24,7 @@ type configs struct { isGeneratedDepsEnabled bool pluginOptions []string env []string + command Cmd } // Plugin configures a plugin for code generation. @@ -49,9 +50,29 @@ func Env(v ...string) Option { } } +// WithCommand assigns a protoc command to use for code generation. +// This allows to use a single protoc binary in multiple code generation calls. +// Otherwise `Generate` creates a new protoc binary each time it is called. +func WithCommand(command Cmd) Option { + return func(c *configs) { + c.command = command + } +} + +// Cmd contains the information necessary to execute the protoc command. type Cmd struct { - Command []string - Included []string + command []string + includes []string +} + +// Command returns the strings to execute the `protoc` command. +func (c Cmd) Command() []string { + return c.command +} + +// Includes returns the proto files import paths. +func (c Cmd) Includes() []string { + return c.includes } // Command sets the protoc binary up and returns the command needed to execute c. @@ -73,8 +94,8 @@ func Command() (command Cmd, cleanup func(), err error) { } command = Cmd{ - Command: []string{path, "-I", include}, - Included: []string{include}, + command: []string{path, "-I", include}, + includes: []string{include}, } return command, cleanup, nil @@ -88,13 +109,23 @@ func Generate(ctx context.Context, outDir, protoPath string, includePaths, proto o(&c) } - cmd, cleanup, err := Command() - if err != nil { - return err - } - defer cleanup() + var command, includes []string + + // init the string to run the protoc command and the proto files import path + if c.command.Command() == nil { + cmd, cleanup, err := Command() + if err != nil { + return err + } - command := cmd.Command + defer cleanup() + + command = cmd.Command() + includes = cmd.Includes() + } else { + command = c.command.Command() + includes = c.command.Includes() + } // add plugin if set. if c.pluginPath != "" { @@ -116,7 +147,7 @@ func Generate(ctx context.Context, outDir, protoPath string, includePaths, proto } // find out the list of proto files to generate code for and perform code generation. - files, err := discoverFiles(ctx, c, protoPath, append(cmd.Included, existentIncludePaths...), protoanalysis.NewCache()) + files, err := discoverFiles(ctx, c, protoPath, append(includes, existentIncludePaths...), protoanalysis.NewCache()) if err != nil { return err } From 863da66498765a0d57ac73dcc9f473aea6d4c90a Mon Sep 17 00:00:00 2001 From: jeronimoalbi Date: Tue, 2 Aug 2022 16:18:33 +0200 Subject: [PATCH 3/4] refactor(pkg/cosmosgen): change JS generator to reuse protoc binary --- ignite/pkg/cosmosgen/generate_javascript.go | 24 +++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/ignite/pkg/cosmosgen/generate_javascript.go b/ignite/pkg/cosmosgen/generate_javascript.go index 4d24387c4a..a26298b1db 100644 --- a/ignite/pkg/cosmosgen/generate_javascript.go +++ b/ignite/pkg/cosmosgen/generate_javascript.go @@ -57,11 +57,19 @@ func (g *generator) generateJS() error { } func (g *jsGenerator) generateModules() error { - tsprotoPluginPath, cleanup, err := tsproto.BinaryPath() + protocCmd, cleanupProtoc, err := protoc.Command() if err != nil { return err } - defer cleanup() + + defer cleanupProtoc() + + tsprotoPluginPath, cleanupPlugin, err := tsproto.BinaryPath() + if err != nil { + return err + } + + defer cleanupPlugin() gg := &errgroup.Group{} @@ -81,7 +89,8 @@ func (g *jsGenerator) generateModules() error { return nil } - if err := g.generateModule(g.g.ctx, tsprotoPluginPath, sourcePath, m); err != nil { + err = g.generateModule(g.g.ctx, protocCmd, tsprotoPluginPath, sourcePath, m) + if err != nil { return err } @@ -102,7 +111,12 @@ func (g *jsGenerator) generateModules() error { } // generateModule generates generates JS code for a module. -func (g *jsGenerator) generateModule(ctx context.Context, tsprotoPluginPath, appPath string, m module.Module) error { +func (g *jsGenerator) generateModule( + ctx context.Context, + cmd protoc.Cmd, + tsprotoPluginPath, appPath string, + m module.Module, +) error { var ( out = g.g.o.jsOut(m) storeDirPath = filepath.Dir(out) @@ -127,6 +141,7 @@ func (g *jsGenerator) generateModule(ctx context.Context, tsprotoPluginPath, app tsOut, protoc.Plugin(tsprotoPluginPath, "--ts_proto_opt=snakeToCamel=false"), protoc.Env("NODE_OPTIONS="), // unset nodejs options to avoid unexpected issues with vercel "pkg" + protoc.WithCommand(cmd), ) if err != nil { return err @@ -145,6 +160,7 @@ func (g *jsGenerator) generateModule(ctx context.Context, tsprotoPluginPath, app m.Pkg.Path, includePaths, jsOpenAPIOut, + protoc.WithCommand(cmd), ) if err != nil { return err From e10af03ad70f0c3317fa51a9f06f77a22e825734 Mon Sep 17 00:00:00 2001 From: jeronimoalbi Date: Tue, 2 Aug 2022 16:26:25 +0200 Subject: [PATCH 4/4] chore: simplify code --- ignite/pkg/protoc/protoc.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/ignite/pkg/protoc/protoc.go b/ignite/pkg/protoc/protoc.go index a584893133..5f65299d66 100644 --- a/ignite/pkg/protoc/protoc.go +++ b/ignite/pkg/protoc/protoc.go @@ -109,10 +109,11 @@ func Generate(ctx context.Context, outDir, protoPath string, includePaths, proto o(&c) } - var command, includes []string - // init the string to run the protoc command and the proto files import path - if c.command.Command() == nil { + command := c.command.Command() + includes := c.command.Includes() + + if command == nil { cmd, cleanup, err := Command() if err != nil { return err @@ -122,9 +123,6 @@ func Generate(ctx context.Context, outDir, protoPath string, includePaths, proto command = cmd.Command() includes = cmd.Includes() - } else { - command = c.command.Command() - includes = c.command.Includes() } // add plugin if set.