Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ linters:
- linters:
- containedctx
# we actually want to embed a context here
path: private/bufpkg/bufimage/parser_accessor_handler.go
path: private/bufpkg/bufimage/module_file_resolver.go
- linters:
- containedctx
# we actually want to embed a context here
Expand Down Expand Up @@ -439,6 +439,13 @@ linters:
# term.IsTerminal. Safe on all supported platforms.
path: private/pkg/slogapp/console.go
text: "G115:"
- linters:
- gosec
# G115: uintptr->int conversion for passing a terminal file descriptor
# to term.IsTerminal, which requires int. Safe on all supported platforms;
# fd values fit in int.
path: private/buf/bufctl/controller.go
text: "G115:"
- linters:
- gosec
# G602: false positive — the loop index i is bounded by the length of
Expand Down
5 changes: 4 additions & 1 deletion cmd/buf/buf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4493,7 +4493,10 @@ func TestProtoFileNoWorkspaceOrModule(t *testing.T) {
nil,
bufctl.ExitCodeFileAnnotation,
"", // no stdout
filepath.FromSlash(`testdata/protofileref/noworkspaceormodule/fail/import.proto:3:8:import "`)+`google/type/date.proto": file does not exist`,
fmt.Sprintf(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (and all others) is likely a breaking change. I am not sure if this matters on stderr, I can't remember, but buf was designed to output errors in the standard file:line:column:message format, which is relied upon by many tools.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a sweep, and I think it's mostly ALE -> stdout. That being said, this is still a valid concern and @stefanvanburen had a good suggestion, which I am fully supportive of:

  • Add a new error-format=pretty, which would provide the new error report format
  • Detect if the output is TTY, which we already do for logging, and if it is, then we default to error-format=pretty, and other wise, keep the default as error-format=text. This would keep CI's consistent, ALE, etc.
  • This would apply to anything that produces compiler diagnostics and file annotations, so I can do a bit of work to structure this for lint, breaking

Let me know if this approach seems sound to you!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed this offline, putting some notes here:

  • No need for a new format name, instead we'll just default to this report format for TTY (and keep the old format for non-TTY)
  • No changes to lint, breaking's stdout output -- this will only be used for stderr compiler output

"%[1]s:3:1:imported file does not exist\n%[1]s:6:3:cannot find `google.type.Date` in this scope",
filepath.FromSlash("testdata/protofileref/noworkspaceormodule/fail/import.proto"),
),
"build",
filepath.Join("testdata", "protofileref", "noworkspaceormodule", "fail", "import.proto"),
)
Expand Down
11 changes: 9 additions & 2 deletions cmd/buf/imports_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package main

import (
"fmt"
"io"
"path/filepath"
"strings"
Expand Down Expand Up @@ -148,7 +149,10 @@ func TestInvalidNonexistentImport(t *testing.T) {
t.Parallel()
testRunStderrWithCache(
t, nil, bufctl.ExitCodeFileAnnotation,
filepath.FromSlash(`testdata/imports/failure/people/people/v1/people1.proto:5:8:import "nonexistent.proto": file does not exist`),
fmt.Sprintf(
"%[1]s:5:1:imported file does not exist",
filepath.FromSlash("testdata/imports/failure/people/people/v1/people1.proto"),
),
"build",
filepath.Join("testdata", "imports", "failure", "people"),
)
Expand All @@ -158,7 +162,10 @@ func TestInvalidNonexistentImportFromDirectDep(t *testing.T) {
t.Parallel()
testRunStderrWithCache(
t, nil, bufctl.ExitCodeFileAnnotation,
filepath.FromSlash(`testdata/imports/failure/students/students/v1/students.proto:`)+`6:8:import "people/v1/people_nonexistent.proto": file does not exist`,
fmt.Sprintf(
"%[1]s:6:1:imported file does not exist\n%[1]s:10:3:cannot find `people.v1.Person2` in this scope",
filepath.FromSlash("testdata/imports/failure/students/students/v1/students.proto"),
),
"build",
filepath.Join("testdata", "imports", "failure", "students"),
)
Expand Down
3 changes: 2 additions & 1 deletion cmd/buf/internal/command/breaking/breaking.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ func run(
container,
bufctl.WithDisableSymlinks(flags.DisableSymlinks),
bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat),
bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor),
bufctl.WithFileAnnotationsToStdout(),
)
if err != nil {
Expand Down Expand Up @@ -368,7 +369,7 @@ func run(
}
}
if len(allFileAnnotations) > 0 {
allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(allFileAnnotations...)
allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(nil, allFileAnnotations...)
if err := bufanalysis.PrintFileAnnotationSet(
container.Stdout(),
allFileAnnotationSet,
Expand Down
1 change: 1 addition & 0 deletions cmd/buf/internal/command/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ func run(
container,
bufctl.WithDisableSymlinks(flags.DisableSymlinks),
bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat),
bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor),
)
if err != nil {
return err
Expand Down
1 change: 1 addition & 0 deletions cmd/buf/internal/command/convert/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ func run(
container,
bufctl.WithDisableSymlinks(flags.DisableSymlinks),
bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat),
bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor),
)
if err != nil {
return err
Expand Down
7 changes: 6 additions & 1 deletion cmd/buf/internal/command/curl/curl.go
Original file line number Diff line number Diff line change
Expand Up @@ -1334,7 +1334,12 @@ func completeURLFromSchema(ctx context.Context, schemas []string, baseURL, rawPa
return nil, cobra.ShellCompDirectiveNoFileComp
}
// Discard log output during shell completion.
container := appext.NewContainer(nameContainer, slog.New(slog.NewTextHandler(io.Discard, nil)))
container := appext.NewContainer(
nameContainer,
slog.New(slog.NewTextHandler(io.Discard, nil)),
appext.LogLevelError, // Doesn't matter, logs are discarded
appext.LogFormatText, // Doesn't matter, logs are discarded
)
controller, err := bufcli.NewController(container)
if err != nil {
reportError("%v", err)
Expand Down
1 change: 1 addition & 0 deletions cmd/buf/internal/command/dep/depgraph/depgraph.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ func run(
container,
bufctl.WithDisableSymlinks(flags.DisableSymlinks),
bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat),
bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor),
)
if err != nil {
return err
Expand Down
1 change: 1 addition & 0 deletions cmd/buf/internal/command/format/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ func run(
container,
bufctl.WithDisableSymlinks(flags.DisableSymlinks),
bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat),
bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor),
)
if err != nil {
return err
Expand Down
1 change: 1 addition & 0 deletions cmd/buf/internal/command/generate/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,7 @@ func run(
container,
bufctl.WithDisableSymlinks(flags.DisableSymlinks),
bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat),
bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor),
)
if err != nil {
return err
Expand Down
3 changes: 2 additions & 1 deletion cmd/buf/internal/command/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ func run(
container,
bufctl.WithDisableSymlinks(flags.DisableSymlinks),
bufctl.WithFileAnnotationErrorFormat(controllerErrorFormat),
bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor),
bufctl.WithFileAnnotationsToStdout(),
)
if err != nil {
Expand Down Expand Up @@ -172,7 +173,7 @@ func run(
}
}
if len(allFileAnnotations) > 0 {
allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(allFileAnnotations...)
allFileAnnotationSet := bufanalysis.NewFileAnnotationSet(nil, allFileAnnotations...)
if flags.ErrorFormat == "config-ignore-yaml" {
if err := bufcli.PrintFileAnnotationSetLintConfigIgnoreYAMLV1(
container.Stdout(),
Expand Down
1 change: 1 addition & 0 deletions cmd/buf/internal/command/push/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ func getBuildableWorkspace(
container,
bufctl.WithDisableSymlinks(flags.DisableSymlinks),
bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat),
bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor),
)
if err != nil {
return nil, err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ func run(
container,
bufctl.WithDisableSymlinks(flags.DisableSymlinks),
bufctl.WithFileAnnotationErrorFormat(flags.ErrorFormat),
bufctl.WithColorizedFileAnnotationSetDiagnosticReport(container.LogFormat() == appext.LogFormatColor),
)
if err != nil {
return err
Expand Down
16 changes: 12 additions & 4 deletions cmd/buf/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,15 @@ func TestWorkspaceDir(t *testing.T) {
"lint",
filepath.Join("testdata", "workspace", "success", baseDirPath),
)
dirImportError := fmt.Sprintf(
"%[1]s:5:1:imported file does not exist\n%[1]s:8:5:cannot find `request.Request` in this scope",
filepath.FromSlash("testdata/workspace/success/"+baseDirPath+"/proto/rpc.proto"),
)
testRunStdoutStderrNoWarn(
t,
nil,
bufctl.ExitCodeFileAnnotation,
filepath.FromSlash(`testdata/workspace/success/`+baseDirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`),
dirImportError,
"",
"lint",
filepath.Join("testdata", "workspace", "success", baseDirPath),
Expand All @@ -156,7 +160,7 @@ func TestWorkspaceDir(t *testing.T) {
t,
nil,
bufctl.ExitCodeFileAnnotation,
filepath.FromSlash(`testdata/workspace/success/`+baseDirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`),
dirImportError,
"",
"lint",
filepath.Join("testdata", "workspace", "success", baseDirPath),
Expand Down Expand Up @@ -366,12 +370,16 @@ func TestWorkspaceDetached(t *testing.T) {
// we'd consider this a bug: you specified the proto directory, and no controlling workspace
// was discovered, therefore you build as if proto was the input directory, which results in
// request.proto not existing as an import.
detachedImportError := fmt.Sprintf(
"%[1]s:5:1:imported file does not exist\n%[1]s:8:5:cannot find `request.Request` in this scope",
filepath.FromSlash("testdata/workspace/success/"+dirPath+"/proto/rpc.proto"),
)
testRunStdoutStderrNoWarn(
t,
nil,
bufctl.ExitCodeFileAnnotation,
``,
filepath.FromSlash(`testdata/workspace/success/`+dirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`),
detachedImportError,
"build",
filepath.Join("testdata", "workspace", "success", dirPath, "proto"),
)
Expand All @@ -392,7 +400,7 @@ func TestWorkspaceDetached(t *testing.T) {
t,
nil,
bufctl.ExitCodeFileAnnotation,
filepath.FromSlash(`testdata/workspace/success/`+dirPath+`/proto/rpc.proto:5:8:import "request.proto": file does not exist`),
detachedImportError,
``,
"lint",
filepath.Join("testdata", "workspace", "success", dirPath, "proto"),
Expand Down
37 changes: 19 additions & 18 deletions cmd/buf/workspace_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package main

import (
"fmt"
"path/filepath"
"testing"

Expand All @@ -26,24 +27,24 @@ import (
func TestWorkspaceSymlinkFail(t *testing.T) {
t.Parallel()
// The workspace includes a symlink that isn't buildable.
testRunStdoutStderrNoWarn(
t,
nil,
bufctl.ExitCodeFileAnnotation,
``,
filepath.FromSlash(`testdata/workspace/fail/symlink/b/b.proto:5:8:import "c.proto": file does not exist`),
"build",
filepath.Join("testdata", "workspace", "fail", "symlink"),
)
testRunStdoutStderrNoWarn(
t,
nil,
bufctl.ExitCodeFileAnnotation,
``,
filepath.FromSlash(`testdata/workspace/fail/v2/symlink/b/b.proto:5:8:import "c.proto": file does not exist`),
"build",
filepath.Join("testdata", "workspace", "fail", "v2", "symlink"),
)
for _, dirPath := range []string{
"symlink",
"v2/symlink",
} {
symlinkImportError := fmt.Sprintf(
"%[1]s:5:1:imported file does not exist\n%[1]s:8:5:cannot find `c.C` in this scope",
filepath.FromSlash("testdata/workspace/fail/"+dirPath+"/b/b.proto"),
)
testRunStdoutStderrNoWarn(
t,
nil,
bufctl.ExitCodeFileAnnotation,
``,
symlinkImportError,
"build",
filepath.Join("testdata", "workspace", "fail", filepath.FromSlash(dirPath)),
)
}
}

func TestWorkspaceSymlink(t *testing.T) {
Expand Down
5 changes: 3 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ go 1.25.7

require (
buf.build/gen/go/bufbuild/bufplugin/protocolbuffers/go v1.36.11-20250718181942-e35f9b667443.1
buf.build/gen/go/bufbuild/protodescriptor/protocolbuffers/go v1.36.11-20250109164928-1da0de137947.1
buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.11-20260209202127-80ab13bee0bf.1
buf.build/gen/go/bufbuild/registry/connectrpc/go v1.19.1-20260126144947-819582968857.2
buf.build/gen/go/bufbuild/registry/protocolbuffers/go v1.36.11-20260126144947-819582968857.1
buf.build/go/app v0.2.1-0.20260319180457-cfbf7e2b6a75
buf.build/go/app v0.2.1-0.20260407195847-833f8f978cda
buf.build/go/bufplugin v0.9.0
buf.build/go/bufprivateusage v0.1.0
buf.build/go/protovalidate v1.1.3
Expand All @@ -17,7 +18,7 @@ require (
connectrpc.com/connect v1.19.1
connectrpc.com/grpcreflect v1.3.0
connectrpc.com/otelconnect v0.9.0
github.com/bufbuild/protocompile v0.14.2-0.20260406184405-b06501d51312
github.com/bufbuild/protocompile v0.14.2-0.20260409205102-f58612be4b0a
github.com/bufbuild/protoplugin v0.0.0-20250218205857-750e09ce93e1
github.com/cli/browser v1.3.0
github.com/gofrs/flock v0.13.0
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ buf.build/gen/go/bufbuild/registry/protocolbuffers/go v1.36.11-20260126144947-81
buf.build/gen/go/bufbuild/registry/protocolbuffers/go v1.36.11-20260126144947-819582968857.1/go.mod h1:1JJi9jvOqRxSMa+JxiZSm57doB+db/1WYCIa2lHfc40=
buf.build/gen/go/pluginrpc/pluginrpc/protocolbuffers/go v1.36.11-20241007202033-cf42259fcbfc.1 h1:iGPvEJltOXUMANWf0zajcRcbiOXLD90ZwPUFvbcuv6Q=
buf.build/gen/go/pluginrpc/pluginrpc/protocolbuffers/go v1.36.11-20241007202033-cf42259fcbfc.1/go.mod h1:nWVKKRA29zdt4uvkjka3i/y4mkrswyWwiu0TbdX0zts=
buf.build/go/app v0.2.1-0.20260319180457-cfbf7e2b6a75 h1:wb0B8XpzzfGIekqUXw4kUAvkroAiVmAg3uIpgo5S8sM=
buf.build/go/app v0.2.1-0.20260319180457-cfbf7e2b6a75/go.mod h1:0XVOYemubVbxNXVY0DnsVgWeGkcbbAvjDa1fmhBC+Wo=
buf.build/go/app v0.2.1-0.20260407195847-833f8f978cda h1:eysSyjrJtkxU1A/9+Kv+1Mwq9K6BYBw+STIOVsZ256Y=
buf.build/go/app v0.2.1-0.20260407195847-833f8f978cda/go.mod h1:V32mBaPWsfq6REAeZvvs/rQl7ZCl9Dn7eW1BBrmH0GQ=
buf.build/go/bufplugin v0.9.0 h1:ktZJNP3If7ldcWVqh46XKeiYJVPxHQxCfjzVQDzZ/lo=
buf.build/go/bufplugin v0.9.0/go.mod h1:Z0CxA3sKQ6EPz/Os4kJJneeRO6CjPeidtP1ABh5jPPY=
buf.build/go/bufprivateusage v0.1.0 h1:SzCoCcmzS3zyXHEXHeSQhGI7OTkgtljoknLzsUz9Gg4=
Expand Down Expand Up @@ -42,8 +42,8 @@ github.com/bmatcuk/doublestar/v4 v4.10.0 h1:zU9WiOla1YA122oLM6i4EXvGW62DvKZVxIe6
github.com/bmatcuk/doublestar/v4 v4.10.0/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
github.com/brianvoe/gofakeit/v6 v6.28.0 h1:Xib46XXuQfmlLS2EXRuJpqcw8St6qSZz75OUo0tgAW4=
github.com/brianvoe/gofakeit/v6 v6.28.0/go.mod h1:Xj58BMSnFqcn/fAQeSK+/PLtC5kSb7FJIq4JyGa8vEs=
github.com/bufbuild/protocompile v0.14.2-0.20260406184405-b06501d51312 h1:Cp7oCRZIqSsBrsi3GJSYzTgQNz70PJv5J+6hoXGyD2k=
github.com/bufbuild/protocompile v0.14.2-0.20260406184405-b06501d51312/go.mod h1:DhgqsRznX/F0sGkUYtTQJRP+q8xMReQRQ3qr+n1opWU=
github.com/bufbuild/protocompile v0.14.2-0.20260409205102-f58612be4b0a h1:mtZZcNBraTXNE1e/UG7Odlw8314ucNtRgnB2muW0/cs=
github.com/bufbuild/protocompile v0.14.2-0.20260409205102-f58612be4b0a/go.mod h1:DhgqsRznX/F0sGkUYtTQJRP+q8xMReQRQ3qr+n1opWU=
github.com/bufbuild/protoplugin v0.0.0-20250218205857-750e09ce93e1 h1:V1xulAoqLqVg44rY97xOR+mQpD2N+GzhMHVwJ030WEU=
github.com/bufbuild/protoplugin v0.0.0-20250218205857-750e09ce93e1/go.mod h1:c5D8gWRIZ2HLWO3gXYTtUfw/hbJyD8xikv2ooPxnklQ=
github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs=
Expand Down
2 changes: 2 additions & 0 deletions private/buf/bufcli/protoc_plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ func NewAppextContainerForPluginEnv(
return appext.NewContainer(
nameContainer,
logger,
logLevel,
logFormat,
), nil
}

Expand Down
21 changes: 17 additions & 4 deletions private/buf/bufctl/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"io/fs"
"log/slog"
"net/http"
"os"
"slices"
"sort"

Expand Down Expand Up @@ -53,6 +54,7 @@ import (
"github.com/bufbuild/buf/private/pkg/storage/storageos"
"github.com/bufbuild/buf/private/pkg/syserror"
"github.com/bufbuild/buf/private/pkg/wasm"
"golang.org/x/term"
"google.golang.org/protobuf/proto"
)

Expand Down Expand Up @@ -213,10 +215,11 @@ type controller struct {
policyDataProvider bufpolicy.PolicyDataProvider
wktStore bufwktstore.Store

disableSymlinks bool
fileAnnotationErrorFormat string
fileAnnotationsToStdout bool
copyToInMemory bool
disableSymlinks bool
fileAnnotationErrorFormat string
fileAnnotationsToStdout bool
colorizedFileAnnotationSetDiagnosticReport bool
copyToInMemory bool

storageosProvider storageos.Provider
buffetchRefParser buffetch.RefParser
Expand Down Expand Up @@ -1365,15 +1368,25 @@ func (c *controller) handleFileAnnotationSetRetError(retErrAddr *error) {
return
}
var fileAnnotationSet bufanalysis.FileAnnotationSet
var printDiagnosticReport bool
if errors.As(*retErrAddr, &fileAnnotationSet) {
writer := c.container.Stderr()
if c.fileAnnotationsToStdout {
writer = c.container.Stdout()
} else {
// When writing to stderr, check if the input is TTY, if so, allow printing the
// diagnostic report.
file, ok := c.container.Stdin().(*os.File)
if ok {
printDiagnosticReport = term.IsTerminal(int(file.Fd()))
}
}
if err := bufanalysis.PrintFileAnnotationSet(
writer,
fileAnnotationSet,
c.fileAnnotationErrorFormat,
bufanalysis.WithPrintDiagnosticReport(printDiagnosticReport),
bufanalysis.WithRenderColorizedDiagnosticReport(c.colorizedFileAnnotationSetDiagnosticReport),
); err != nil {
*retErrAddr = err
return
Expand Down
Loading
Loading