diff --git a/CHANGELOG.md b/CHANGELOG.md index 874eb5ac..2c3936c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Accept `prototool.json` files for configuation in addition to `prototool.yaml` files. - Add `--config-data` flag. +- Add `protoc-bin-path` and `protoc-wkt-path` flags to manually + set the paths for where `protoc` is run and where the + well-known types are included from. ## [1.2.0] - 2018-08-29 diff --git a/internal/cmd/flags.go b/internal/cmd/flags.go index 005cee32..0b6afbc2 100644 --- a/internal/cmd/flags.go +++ b/internal/cmd/flags.go @@ -47,6 +47,8 @@ type flags struct { overwrite bool pkg string printFields string + protocBinPath string + protocWKTPath string protocURL string stdin bool uncomment bool @@ -140,6 +142,14 @@ func (f *flags) bindProtocURL(flagSet *pflag.FlagSet) { flagSet.StringVar(&f.protocURL, "protoc-url", "", "The url to use to download the protoc zip file, otherwise uses GitHub Releases. Setting this option will ignore the config protoc.version setting.") } +func (f *flags) bindProtocBinPath(flagSet *pflag.FlagSet) { + flagSet.StringVar(&f.protocBinPath, "protoc-bin-path", "", "The path to the protoc binary. Setting this option will ignore the config protoc.version setting. This flag must be used with protoc-wkt-path and must not be used with the protoc-url flag.") +} + +func (f *flags) bindProtocWKTPath(flagSet *pflag.FlagSet) { + flagSet.StringVar(&f.protocWKTPath, "protoc-wkt-path", "", "The path to the well-known types. Setting this option will ignore the config protoc.version setting. This flag must be used with protoc-bin-path and must not be used with the protoc-url flag.") +} + func (f *flags) bindStdin(flagSet *pflag.FlagSet) { flagSet.BoolVar(&f.stdin, "stdin", false, "Read the GRPC request data from stdin in JSON format. Either this or --data is required.") } diff --git a/internal/cmd/templates.go b/internal/cmd/templates.go index 1a8547f5..f29af2ed 100644 --- a/internal/cmd/templates.go +++ b/internal/cmd/templates.go @@ -51,6 +51,8 @@ var ( flags.bindJSON(flagSet) flags.bindFix(flagSet) flags.bindProtocURL(flagSet) + flags.bindProtocBinPath(flagSet) + flags.bindProtocWKTPath(flagSet) }, } @@ -88,6 +90,8 @@ var ( flags.bindDryRun(flagSet) flags.bindJSON(flagSet) flags.bindProtocURL(flagSet) + flags.bindProtocBinPath(flagSet) + flags.bindProtocWKTPath(flagSet) }, } @@ -225,6 +229,8 @@ If Vim integration is set up, files will be generated when you open a new Protob flags.bindOverwrite(flagSet) flags.bindFix(flagSet) flags.bindProtocURL(flagSet) + flags.bindProtocBinPath(flagSet) + flags.bindProtocWKTPath(flagSet) }, } @@ -240,6 +246,8 @@ If Vim integration is set up, files will be generated when you open a new Protob flags.bindDryRun(flagSet) flags.bindJSON(flagSet) flags.bindProtocURL(flagSet) + flags.bindProtocBinPath(flagSet) + flags.bindProtocWKTPath(flagSet) }, } @@ -329,6 +337,8 @@ $ cat input.json | prototool grpc example \ flags.bindMethod(flagSet) flags.bindStdin(flagSet) flags.bindProtocURL(flagSet) + flags.bindProtocBinPath(flagSet) + flags.bindProtocWKTPath(flagSet) }, } @@ -371,6 +381,8 @@ $ cat input.json | prototool grpc example \ flags.bindListAllLinters(flagSet) flags.bindListLinters(flagSet) flags.bindProtocURL(flagSet) + flags.bindProtocBinPath(flagSet) + flags.bindProtocWKTPath(flagSet) }, } @@ -508,6 +520,18 @@ func getRunner(stdin io.Reader, stdout io.Writer, stderr io.Writer, flags *flags exec.RunnerWithJSON(), ) } + if flags.protocBinPath != "" { + runnerOptions = append( + runnerOptions, + exec.RunnerWithProtocBinPath(flags.protocBinPath), + ) + } + if flags.protocWKTPath != "" { + runnerOptions = append( + runnerOptions, + exec.RunnerWithProtocWKTPath(flags.protocWKTPath), + ) + } if flags.printFields != "" { runnerOptions = append( runnerOptions, diff --git a/internal/exec/exec.go b/internal/exec/exec.go index e6149751..d2f522c3 100644 --- a/internal/exec/exec.go +++ b/internal/exec/exec.go @@ -107,6 +107,20 @@ func RunnerWithPrintFields(printFields string) RunnerOption { } } +// RunnerWithProtocBinPath returns a RunnerOption that uses the given protoc binary path. +func RunnerWithProtocBinPath(protocBinPath string) RunnerOption { + return func(runner *runner) { + runner.protocBinPath = protocBinPath + } +} + +// RunnerWithProtocWKTPath returns a RunnerOption that uses the given path to include the well-known types. +func RunnerWithProtocWKTPath(protocWKTPath string) RunnerOption { + return func(runner *runner) { + runner.protocWKTPath = protocWKTPath + } +} + // RunnerWithProtocURL returns a RunnerOption that uses the given protoc zip file URL. func RunnerWithProtocURL(protocURL string) RunnerOption { return func(runner *runner) { diff --git a/internal/exec/runner.go b/internal/exec/runner.go index c7be9226..161c6026 100644 --- a/internal/exec/runner.go +++ b/internal/exec/runner.go @@ -65,12 +65,14 @@ type runner struct { input io.Reader output io.Writer - logger *zap.Logger - cachePath string - configData string - protocURL string - printFields string - json bool + logger *zap.Logger + cachePath string + configData string + protocBinPath string + protocWKTPath string + protocURL string + printFields string + json bool } func newRunner(workDirPath string, input io.Reader, output io.Writer, options ...RunnerOption) *runner { @@ -185,7 +187,11 @@ func (r *runner) Download() error { if err != nil { return err } - path, err := r.newDownloader(config).Download() + d, err := r.newDownloader(config) + if err != nil { + return err + } + path, err := d.Download() if err != nil { return err } @@ -197,7 +203,11 @@ func (r *runner) Clean() error { if err != nil { return err } - return r.newDownloader(config).Delete() + d, err := r.newDownloader(config) + if err != nil { + return err + } + return d.Delete() } func (r *runner) Files(args []string) error { @@ -649,7 +659,7 @@ func (r *runner) GRPC(args, headers []string, address, method, data, callTimeout ).Invoke(fileDescriptorSets, address, method, reader, r.output) } -func (r *runner) newDownloader(config settings.Config) protoc.Downloader { +func (r *runner) newDownloader(config settings.Config) (protoc.Downloader, error) { downloaderOptions := []protoc.DownloaderOption{ protoc.DownloaderWithLogger(r.logger), } @@ -659,6 +669,18 @@ func (r *runner) newDownloader(config settings.Config) protoc.Downloader { protoc.DownloaderWithCachePath(r.cachePath), ) } + if r.protocBinPath != "" { + downloaderOptions = append( + downloaderOptions, + protoc.DownloaderWithProtocBinPath(r.protocBinPath), + ) + } + if r.protocWKTPath != "" { + downloaderOptions = append( + downloaderOptions, + protoc.DownloaderWithProtocWKTPath(r.protocWKTPath), + ) + } if r.protocURL != "" { downloaderOptions = append( downloaderOptions, @@ -678,6 +700,18 @@ func (r *runner) newCompiler(doGen bool, doFileDescriptorSet bool) protoc.Compil protoc.CompilerWithCachePath(r.cachePath), ) } + if r.protocBinPath != "" { + compilerOptions = append( + compilerOptions, + protoc.CompilerWithProtocBinPath(r.protocBinPath), + ) + } + if r.protocWKTPath != "" { + compilerOptions = append( + compilerOptions, + protoc.CompilerWithProtocWKTPath(r.protocWKTPath), + ) + } if r.protocURL != "" { compilerOptions = append( compilerOptions, diff --git a/internal/protoc/compiler.go b/internal/protoc/compiler.go index a9873da9..be90f1b1 100644 --- a/internal/protoc/compiler.go +++ b/internal/protoc/compiler.go @@ -68,6 +68,8 @@ var ( type compiler struct { logger *zap.Logger cachePath string + protocBinPath string + protocWKTPath string protocURL string doGen bool doFileDescriptorSet bool @@ -268,7 +270,10 @@ func (c *compiler) getCmdMetas(protoSet *file.ProtoSet) (cmdMetas []*cmdMeta, re }() // you need a new downloader for every ProtoSet as each configuration file could // have a different protoc.version value - downloader := c.newDownloader(protoSet.Config) + downloader, err := c.newDownloader(protoSet.Config) + if err != nil { + return nil, err + } if _, err := downloader.Download(); err != nil { return cmdMetas, err } @@ -354,7 +359,7 @@ func (c *compiler) getCmdMetas(protoSet *file.ProtoSet) (cmdMetas []*cmdMeta, re return cmdMetas, nil } -func (c *compiler) newDownloader(config settings.Config) Downloader { +func (c *compiler) newDownloader(config settings.Config) (Downloader, error) { downloaderOptions := []DownloaderOption{ DownloaderWithLogger(c.logger), } @@ -364,6 +369,18 @@ func (c *compiler) newDownloader(config settings.Config) Downloader { DownloaderWithCachePath(c.cachePath), ) } + if c.protocBinPath != "" { + downloaderOptions = append( + downloaderOptions, + DownloaderWithProtocBinPath(c.protocBinPath), + ) + } + if c.protocWKTPath != "" { + downloaderOptions = append( + downloaderOptions, + DownloaderWithProtocWKTPath(c.protocWKTPath), + ) + } if c.protocURL != "" { downloaderOptions = append( downloaderOptions, diff --git a/internal/protoc/downloader.go b/internal/protoc/downloader.go index 6562a93c..06f40313 100644 --- a/internal/protoc/downloader.go +++ b/internal/protoc/downloader.go @@ -43,17 +43,24 @@ import ( ) type downloader struct { + lock sync.RWMutex + logger *zap.Logger cachePath string protocURL string config settings.Config - lock sync.RWMutex // the looked-up and verified to exist base path cachedBasePath string + + // If set, Prototool will invoke protoc and include + // the well-known-types, from the configured binPath + // and wktPath. + protocBinPath string + protocWKTPath string } -func newDownloader(config settings.Config, options ...DownloaderOption) *downloader { +func newDownloader(config settings.Config, options ...DownloaderOption) (*downloader, error) { downloader := &downloader{ config: config, logger: zap.NewNop(), @@ -64,7 +71,33 @@ func newDownloader(config settings.Config, options ...DownloaderOption) *downloa if downloader.config.Compile.ProtobufVersion == "" { downloader.config.Compile.ProtobufVersion = vars.DefaultProtocVersion } - return downloader + if downloader.protocBinPath != "" || downloader.protocWKTPath != "" { + if downloader.protocURL != "" { + return nil, fmt.Errorf("cannot use protoc-url in combination with either protoc-bin-path or protoc-wkt-path") + } + if downloader.protocBinPath == "" || downloader.protocWKTPath == "" { + return nil, fmt.Errorf("both protoc-bin-path and protoc-wkt-path must be set") + } + cleanBinPath := filepath.Clean(downloader.protocBinPath) + if _, err := os.Stat(cleanBinPath); os.IsNotExist(err) { + return nil, err + } + cleanWKTPath := filepath.Clean(downloader.protocWKTPath) + if _, err := os.Stat(cleanWKTPath); os.IsNotExist(err) { + return nil, err + } + protobufPath := filepath.Join(cleanWKTPath, "google", "protobuf") + info, err := os.Stat(protobufPath) + if os.IsNotExist(err) { + return nil, err + } + if !info.IsDir() { + return nil, fmt.Errorf("%q is not a valid well-known types directory", protobufPath) + } + downloader.protocBinPath = cleanBinPath + downloader.protocWKTPath = cleanWKTPath + } + return downloader, nil } func (d *downloader) Download() (string, error) { @@ -78,6 +111,9 @@ func (d *downloader) Download() (string, error) { } func (d *downloader) ProtocPath() (string, error) { + if d.protocBinPath != "" { + return d.protocBinPath, nil + } basePath, err := d.Download() if err != nil { return "", err @@ -86,6 +122,9 @@ func (d *downloader) ProtocPath() (string, error) { } func (d *downloader) WellKnownTypesIncludePath() (string, error) { + if d.protocWKTPath != "" { + return d.protocWKTPath, nil + } basePath, err := d.Download() if err != nil { return "", err @@ -104,6 +143,10 @@ func (d *downloader) Delete() error { } func (d *downloader) cache() (string, error) { + if d.protocBinPath != "" { + return d.protocBinPath, nil + } + d.lock.Lock() defer d.lock.Unlock() diff --git a/internal/protoc/downloader_test.go b/internal/protoc/downloader_test.go index f98a61c4..b815d36c 100644 --- a/internal/protoc/downloader_test.go +++ b/internal/protoc/downloader_test.go @@ -21,10 +21,16 @@ package protoc import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/uber/prototool/internal/settings" ) func TestGetDefaultBasePath(t *testing.T) { @@ -93,6 +99,95 @@ func TestGetDefaultBasePath(t *testing.T) { } } +func TestNewDownloaderProtocValidation(t *testing.T) { + tests := []struct { + desc string + url string + binPath string + wktPath string + createBinPath bool + createWKTPath bool + err error + }{ + { + desc: "No options", + }, + { + desc: "protocURL option", + url: "http://example.com", + }, + { + desc: "protocBinPath with protocWKTPath", + binPath: "protoc", + wktPath: "include", + createBinPath: true, + createWKTPath: true, + }, + { + desc: "protocURL set with protocBinPath and protocWKTPath", + url: "http://example.com", + binPath: "protoc", + wktPath: "include", + err: fmt.Errorf("cannot use protoc-url in combination with either protoc-bin-path or protoc-wkt-path"), + }, + { + desc: "protocBinPath set without protocWKTPath", + binPath: "protoc", + err: fmt.Errorf("both protoc-bin-path and protoc-wkt-path must be set"), + }, + { + desc: "protocBinPath does not exist", + binPath: "protoc", + wktPath: "include", + err: fmt.Errorf("stat protoc: no such file or directory"), + }, + { + desc: "protocWKTPath does not exist", + binPath: "protoc", + wktPath: "include", + createBinPath: true, + err: fmt.Errorf("stat include: no such file or directory"), + }, + } + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + tmpRoot, err := ioutil.TempDir("", "test") + require.NoError(t, err) + + // Clean up all the created test directories. + defer os.RemoveAll(tmpRoot) + + if tt.createBinPath { + tt.binPath, err = ioutil.TempDir(tmpRoot, tt.binPath) + require.NoError(t, err) + } + + if tt.createWKTPath { + tt.wktPath, err = ioutil.TempDir(tmpRoot, tt.wktPath) + require.NoError(t, err) + } + + if tt.createBinPath && tt.createWKTPath { + require.NoError(t, os.MkdirAll(filepath.Join(tt.wktPath, "google", "protobuf"), 0755)) + } + + _, err = newDownloader( + settings.Config{}, + DownloaderWithProtocURL(tt.url), + DownloaderWithProtocBinPath(tt.binPath), + DownloaderWithProtocWKTPath(tt.wktPath), + ) + + if tt.err != nil { + require.Error(t, err) + assert.EqualError(t, err, tt.err.Error()) + } else { + assert.Nil(t, err) + } + }) + } +} + func newTestGetenvFunc(xdgCacheHome string, home string) func(string) string { m := make(map[string]string) if xdgCacheHome != "" { diff --git a/internal/protoc/protoc.go b/internal/protoc/protoc.go index 2fd640d6..fc1913ce 100644 --- a/internal/protoc/protoc.go +++ b/internal/protoc/protoc.go @@ -84,6 +84,21 @@ func DownloaderWithCachePath(cachePath string) DownloaderOption { } } +// DownloaderWithProtocBinPath returns a DownloaderOption that uses the given protoc binary path. +func DownloaderWithProtocBinPath(protocBinPath string) DownloaderOption { + return func(downloader *downloader) { + downloader.protocBinPath = protocBinPath + } +} + +// DownloaderWithProtocWKTPath returns a DownloaderOption that uses the given path to include +// the well-known types. +func DownloaderWithProtocWKTPath(protocWKTPath string) DownloaderOption { + return func(downloader *downloader) { + downloader.protocWKTPath = protocWKTPath + } +} + // DownloaderWithProtocURL returns a DownloaderOption that uses the given protoc zip file URL. // // The default is https://github.com/protocolbuffers/protobuf/releases/download/vVERSION/protoc-VERSION-OS-ARCH.zip. @@ -94,7 +109,7 @@ func DownloaderWithProtocURL(protocURL string) DownloaderOption { } // NewDownloader returns a new Downloader for the given config and DownloaderOptions. -func NewDownloader(config settings.Config, options ...DownloaderOption) Downloader { +func NewDownloader(config settings.Config, options ...DownloaderOption) (Downloader, error) { return newDownloader(config, options...) } @@ -146,6 +161,22 @@ func CompilerWithCachePath(cachePath string) CompilerOption { } } +// CompilerWithProtocBinPath returns a CompilerOption that uses the given protoc binary path. +// +func CompilerWithProtocBinPath(protocBinPath string) CompilerOption { + return func(compiler *compiler) { + compiler.protocBinPath = protocBinPath + } +} + +// CompilerWithProtocWKTPath returns a CompilerOption that uses the given path to include the +// well-known types. +func CompilerWithProtocWKTPath(protocWKTPath string) CompilerOption { + return func(compiler *compiler) { + compiler.protocWKTPath = protocWKTPath + } +} + // CompilerWithProtocURL returns a CompilerOption that uses the given protoc zip file URL. // // The default is https://github.com/protocolbuffers/protobuf/releases/download/vVERSION/protoc-VERSION-OS-ARCH.zip.