diff --git a/private/buf/bufcurl/invoker.go b/private/buf/bufcurl/invoker.go index f3e09012e3..cf4f96714d 100644 --- a/private/buf/bufcurl/invoker.go +++ b/private/buf/bufcurl/invoker.go @@ -84,7 +84,7 @@ type invoker struct { // in JSON format. The given resolver is used to resolve Any messages and // extensions that appear in the input or output. Other parameters are used // to create a Connect client, for issuing the RPC. -func NewInvoker(container appext.Container, md protoreflect.MethodDescriptor, res protoencoding.Resolver, emitDefaults bool, httpClient connect.HTTPClient, opts []connect.ClientOption, url string, out io.Writer) Invoker { +func NewInvoker(container appext.Container, verbosePrinter verbose.Printer, md protoreflect.MethodDescriptor, res protoencoding.Resolver, emitDefaults bool, httpClient connect.HTTPClient, opts []connect.ClientOption, url string, out io.Writer) Invoker { opts = append(opts, connect.WithCodec(protoCodec{})) // TODO: could also provide custom compressor implementations that could give us // optics into when request and response messages are compressed (which could be @@ -94,7 +94,7 @@ func NewInvoker(container appext.Container, md protoreflect.MethodDescriptor, re res: res, emitDefaults: emitDefaults, output: out, - printer: container.VerbosePrinter(), + printer: verbosePrinter, errOutput: container.Stderr(), client: connect.NewClient[dynamicpb.Message, deferredMessage](httpClient, url, opts...), } diff --git a/private/buf/cmd/buf/command/curl/curl.go b/private/buf/cmd/buf/command/curl/curl.go index d1dce27c34..fb238c2b79 100644 --- a/private/buf/cmd/buf/command/curl/curl.go +++ b/private/buf/cmd/buf/command/curl/curl.go @@ -99,6 +99,9 @@ const ( outputFlagName = "output" outputFlagShortName = "o" emitDefaultsFlagName = "emit-defaults" + + verboseFlagName = "verbose" + verboseFlagShortName = "v" ) // NewCommand returns a new Command. @@ -237,6 +240,8 @@ type flags struct { Output string EmitDefaults bool + Verbose bool + // so we can inquire about which flags present on command-line // TODO: ideally we'd use cobra directly instead of having the appcmd wrapper, // which prevents a lot of basic functionality by not exposing many cobra features @@ -519,6 +524,14 @@ provided via stdin as a file descriptor set or image`, false, `Emit default values for JSON-encoded responses.`, ) + + flagSet.BoolVarP( + &f.Verbose, + verboseFlagName, + verboseFlagShortName, + false, + "Turn on verbose mode", + ) } func (f *flags) validate(hasURL, isSecure bool) error { @@ -648,10 +661,8 @@ func (f *flags) validate(hasURL, isSecure bool) error { func (f *flags) determineCredentials( ctx context.Context, - container interface { - app.Container - appext.VerboseContainer - }, + container app.Container, + verbosePrinter verbose.Printer, host string, ) (string, error) { if f.User != "" { @@ -687,7 +698,7 @@ func (f *flags) determineCredentials( if _, err := os.Stat(netrcFile); err != nil { if errors.Is(err, os.ErrNotExist) { // This mirrors the behavior of curl when a netrc file does not exist ¯\_(ツ)_/¯ - container.VerbosePrinter().Printf("* Couldn't find host %s in the file %q; using no credentials", host, netrcFile) + verbosePrinter.Printf("* Couldn't find host %s in the file %q; using no credentials", host, netrcFile) return "", nil } if !strings.Contains(err.Error(), netrcFile) { @@ -702,7 +713,7 @@ func (f *flags) determineCredentials( } if machine == nil { // no creds found for this host - container.VerbosePrinter().Printf("* Couldn't find host %s in the file %q; using no credentials", host, netrcFile) + verbosePrinter.Printf("* Couldn't find host %s in the file %q; using no credentials", host, netrcFile) return "", nil } username := machine.Login() @@ -868,6 +879,11 @@ func run(ctx context.Context, container appext.Container, f *flags) (err error) return err } + var verbosePrinter verbose.Printer = verbose.NopPrinter + if f.Verbose { + verbosePrinter = verbose.NewPrinter(container.Stderr(), container.AppName()) + } + var clientOptions []connect.ClientOption switch f.Protocol { case connect.ProtocolGRPC: @@ -881,7 +897,7 @@ func run(ctx context.Context, container appext.Container, f *flags) (err error) // in an end-of-stream message for streaming calls. So this interceptor // will print the trailers for streaming calls when the response stream // is drained. - clientOptions = append(clientOptions, connect.WithInterceptors(bufcurl.TraceTrailersInterceptor(container.VerbosePrinter()))) + clientOptions = append(clientOptions, connect.WithInterceptors(bufcurl.TraceTrailersInterceptor(verbosePrinter))) } dataSource := "(argument)" @@ -910,7 +926,7 @@ func run(ctx context.Context, container appext.Container, f *flags) (err error) } var basicCreds *string if len(requestHeaders.Values("authorization")) == 0 { - creds, err := f.determineCredentials(ctx, container, host) + creds, err := f.determineCredentials(ctx, container, verbosePrinter, host) if err != nil { return err } @@ -950,11 +966,11 @@ func run(ctx context.Context, container appext.Container, f *flags) (err error) // This shouldn't be possible since we check in flags.validate, but just in case return nil, errors.New("URL positional argument is missing") } - roundTripper, err := makeHTTPRoundTripper(f, isSecure, bufcurl.GetAuthority(host, requestHeaders), container.VerbosePrinter()) + roundTripper, err := makeHTTPRoundTripper(f, isSecure, bufcurl.GetAuthority(host, requestHeaders), verbosePrinter) if err != nil { return nil, err } - return bufcurl.NewVerboseHTTPClient(roundTripper, container.VerbosePrinter()), nil + return bufcurl.NewVerboseHTTPClient(roundTripper, verbosePrinter), nil }) output := container.Stdout() @@ -976,7 +992,7 @@ func run(ctx context.Context, container appext.Container, f *flags) (err error) if basicCreds != nil { creds = *basicCreds } else { - if creds, err = f.determineCredentials(ctx, container, host); err != nil { + if creds, err = f.determineCredentials(ctx, container, verbosePrinter, host); err != nil { return err } } @@ -995,7 +1011,7 @@ func run(ctx context.Context, container appext.Container, f *flags) (err error) if err != nil { return err } - res, closeRes := bufcurl.NewServerReflectionResolver(ctx, transport, clientOptions, baseURL, reflectProtocol, reflectHeaders, container.VerbosePrinter()) + res, closeRes := bufcurl.NewServerReflectionResolver(ctx, transport, clientOptions, baseURL, reflectProtocol, reflectHeaders, verbosePrinter) defer closeRes() resolvers = append(resolvers, res) } @@ -1064,7 +1080,7 @@ func run(ctx context.Context, container appext.Container, f *flags) (err error) if err != nil { return err } - invoker := bufcurl.NewInvoker(container, methodDescriptor, res, f.EmitDefaults, transport, clientOptions, urlArg, output) + invoker := bufcurl.NewInvoker(container, verbosePrinter, methodDescriptor, res, f.EmitDefaults, transport, clientOptions, urlArg, output) return invoker.Invoke(ctx, dataSource, dataReader, requestHeaders) } } diff --git a/private/buf/cmd/internal/internal.go b/private/buf/cmd/internal/internal.go index b4d35ad70e..10d1da76ba 100644 --- a/private/buf/cmd/internal/internal.go +++ b/private/buf/cmd/internal/internal.go @@ -24,7 +24,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/pkg/app" "github.com/bufbuild/buf/private/pkg/app/appext" - "github.com/bufbuild/buf/private/pkg/verbose" "github.com/bufbuild/buf/private/pkg/zaputil" "github.com/bufbuild/protoplugin" ) @@ -103,7 +102,6 @@ func NewAppextContainerForPluginEnv( appContainer, appName, logger, - verbose.NopPrinter, ) } diff --git a/private/pkg/app/appext/appext.go b/private/pkg/app/appext/appext.go index 1888e17909..dd9f0c0a46 100644 --- a/private/pkg/app/appext/appext.go +++ b/private/pkg/app/appext/appext.go @@ -26,7 +26,6 @@ import ( "github.com/bufbuild/buf/private/pkg/app" "github.com/bufbuild/buf/private/pkg/encoding" - "github.com/bufbuild/buf/private/pkg/verbose" "github.com/spf13/pflag" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -90,25 +89,11 @@ func NewLoggerContainer(logger *zap.Logger) LoggerContainer { return newLoggerContainer(logger) } -// VerboseContainer provides a verbose.Printer. -type VerboseContainer interface { - // VerboseEnabled returns true if verbose mode is enabled. - VerboseEnabled() bool - // VerbosePrinter returns a verbose.Printer to use for verbose printing. - VerbosePrinter() verbose.Printer -} - -// NewVerboseContainer returns a new VerboseContainer. -func NewVerboseContainer(verbosePrinter verbose.Printer) VerboseContainer { - return newVerboseContainer(verbosePrinter) -} - // Container contains not just the base app container, but all extended containers. type Container interface { app.Container NameContainer LoggerContainer - VerboseContainer } // NewContainer returns a new Container. @@ -116,13 +101,11 @@ func NewContainer( baseContainer app.Container, appName string, logger *zap.Logger, - verbosePrinter verbose.Printer, ) (Container, error) { return newContainer( baseContainer, appName, logger, - verbosePrinter, ) } diff --git a/private/pkg/app/appext/builder.go b/private/pkg/app/appext/builder.go index 5faf3d8bfa..dfa982e5dd 100644 --- a/private/pkg/app/appext/builder.go +++ b/private/pkg/app/appext/builder.go @@ -22,7 +22,6 @@ import ( "github.com/bufbuild/buf/private/pkg/app" "github.com/bufbuild/buf/private/pkg/thread" - "github.com/bufbuild/buf/private/pkg/verbose" "github.com/bufbuild/buf/private/pkg/zaputil" "github.com/pkg/profile" "github.com/spf13/pflag" @@ -34,7 +33,6 @@ import ( type builder struct { appName string - verbose bool debug bool noWarn bool logFormat string @@ -67,7 +65,6 @@ func newBuilder(appName string, options ...BuilderOption) *builder { } func (b *builder) BindRoot(flagSet *pflag.FlagSet) { - flagSet.BoolVarP(&b.verbose, "verbose", "v", false, "Turn on verbose mode") flagSet.BoolVar(&b.debug, "debug", false, "Turn on debug logging") flagSet.StringVar(&b.logFormat, "log-format", "color", "The log format [text,color,json]") if b.defaultTimeout > 0 { @@ -90,6 +87,11 @@ func (b *builder) BindRoot(flagSet *pflag.FlagSet) { _ = flagSet.MarkHidden("no-warn") flagSet.IntVar(&b.parallelism, "parallelism", 0, "Manually control the parallelism") _ = flagSet.MarkHidden("parallelism") + + // We used to have this as a global flag, so we still need to not error when it is called. + var verbose bool + flagSet.BoolVarP(&verbose, "verbose", "v", false, "") + _ = flagSet.MarkHidden("verbose") } func (b *builder) NewRunFunc( @@ -120,8 +122,7 @@ func (b *builder) run( defer func() { retErr = multierr.Append(retErr, logger.Sync()) }() - verbosePrinter := verbose.NewPrinterForFlagValue(appContainer.Stderr(), b.appName, b.verbose) - container, err := newContainer(appContainer, b.appName, logger, verbosePrinter) + container, err := newContainer(appContainer, b.appName, logger) if err != nil { return err } diff --git a/private/pkg/app/appext/container.go b/private/pkg/app/appext/container.go index 838b5859e1..4d6fecd1e7 100644 --- a/private/pkg/app/appext/container.go +++ b/private/pkg/app/appext/container.go @@ -16,7 +16,6 @@ package appext import ( "github.com/bufbuild/buf/private/pkg/app" - "github.com/bufbuild/buf/private/pkg/verbose" "go.uber.org/zap" ) @@ -24,23 +23,20 @@ type container struct { app.Container NameContainer LoggerContainer - VerboseContainer } func newContainer( baseContainer app.Container, appName string, logger *zap.Logger, - verbosePrinter verbose.Printer, ) (*container, error) { nameContainer, err := newNameContainer(baseContainer, appName) if err != nil { return nil, err } return &container{ - Container: baseContainer, - NameContainer: nameContainer, - LoggerContainer: newLoggerContainer(logger), - VerboseContainer: newVerboseContainer(verbosePrinter), + Container: baseContainer, + NameContainer: nameContainer, + LoggerContainer: newLoggerContainer(logger), }, nil } diff --git a/private/pkg/app/appext/verbose_container.go b/private/pkg/app/appext/verbose_container.go deleted file mode 100644 index a0e6e4c33f..0000000000 --- a/private/pkg/app/appext/verbose_container.go +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2020-2024 Buf Technologies, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package appext - -import ( - "github.com/bufbuild/buf/private/pkg/verbose" -) - -type verboseContainer struct { - verbosePrinter verbose.Printer -} - -func newVerboseContainer(verbosePrinter verbose.Printer) *verboseContainer { - return &verboseContainer{ - verbosePrinter: verbosePrinter, - } -} - -func (c *verboseContainer) VerboseEnabled() bool { - return c.verbosePrinter.Enabled() -} - -func (c *verboseContainer) VerbosePrinter() verbose.Printer { - return c.verbosePrinter -}