post merge

.dockerignore

@@ -8,14 +8,14 @@
 cmd/buf/buf
 cmd/protoc-gen-buf-breaking/protoc-gen-buf-breaking
 cmd/protoc-gen-buf-lint/protoc-gen-buf-lint
+private/buf/buftesting/cache/
+private/buf/bufwkt/cmd/wkt-go-data/wkt-go-data
 private/buf/cmd/buf-digest/buf-digest
 private/buf/cmd/buf/command/alpha/protoc/internal/protoc-gen-insertion-point-receiver/protoc-gen-insertion-point-receiver
 private/buf/cmd/buf/command/alpha/protoc/internal/protoc-gen-insertion-point-writer/protoc-gen-insertion-point-writer
 private/buf/cmd/buf/command/alpha/protoc/test.txt
 private/buf/cmd/buf/workspacetests/other/proto/workspacetest/cache/
 private/bufpkg/bufstyle/cmd/bufstyle/bufstyle
-private/bufpkg/buftesting/cache/
-private/bufpkg/bufwkt/cmd/wkt-go-data/wkt-go-data
 private/pkg/bandeps/cmd/bandeps/bandeps
 private/pkg/git/cmd/git-ls-files-unstaged/git-ls-files-unstaged
 private/pkg/licenseheader/cmd/license-header/license-header

.gitattributes

@@ -4,8 +4,8 @@
 * text=auto
 
 # Never attempt EOL conversion on files within testdata
-**/testdata/** binary
+**/testdata/** -crlf
 
 **/corpus/** linguist-generated=true
-**/privateusage.gen.go linguist-generated=true
+**/usage.gen.go linguist-generated=true
 /private/gen/** linguist-generated=true

.gitignore

@@ -8,14 +8,14 @@
 /cmd/buf/buf
 /cmd/protoc-gen-buf-breaking/protoc-gen-buf-breaking
 /cmd/protoc-gen-buf-lint/protoc-gen-buf-lint
+/private/buf/buftesting/cache/
+/private/buf/bufwkt/cmd/wkt-go-data/wkt-go-data
 /private/buf/cmd/buf-digest/buf-digest
 /private/buf/cmd/buf/command/alpha/protoc/internal/protoc-gen-insertion-point-receiver/protoc-gen-insertion-point-receiver
 /private/buf/cmd/buf/command/alpha/protoc/internal/protoc-gen-insertion-point-writer/protoc-gen-insertion-point-writer
 /private/buf/cmd/buf/command/alpha/protoc/test.txt
 /private/buf/cmd/buf/workspacetests/other/proto/workspacetest/cache/
 /private/bufpkg/bufstyle/cmd/bufstyle/bufstyle
-/private/bufpkg/buftesting/cache/
-/private/bufpkg/bufwkt/cmd/wkt-go-data/wkt-go-data
 /private/pkg/bandeps/cmd/bandeps/bandeps
 /private/pkg/git/cmd/git-ls-files-unstaged/git-ls-files-unstaged
 /private/pkg/licenseheader/cmd/license-header/license-header

CHANGELOG.md

@@ -12,6 +12,9 @@
   `use_enum_numbers`. This affects output serialization. Some examples:
   - `buf convert --type foo.Bar --from input.binpb --to output.yaml#use_proto_names=true`
   - `buf convert --type foo.Bar --from input.binpb --to -#format=yaml,use_enum_numbers=true`
+- Fix issue where `buf format` would inadvertently mangle files that used
+  the [expanded `Any` syntax](https://protobuf.com/docs/language-spec#any-messages)
+  in option values.
 
 ## [v1.28.1] - 2023-11-15
 

PACKAGES.md

@@ -12,8 +12,8 @@
 
 ## Specific Packages
 
-- TODO: Move `appproto` out of `app`
-- TODO: Look into merging `appcmd` and `appflag`
-- TODO: Replicate `cobra.PositionalArgs` so that no one needs to directly import cobra
+- TODO: Move `protoplugin` out of `app` DONE
+- TODO: Look into merging `appcmd` and `appext` DONE
+- TODO: Replicate `cobra.PositionalArgs` so that no one needs to directly import cobra DONE
 - TODO: Better documentation for app
 - TODO: Move x packages to pkg/x, rename to xfilepath, xslices, etc

TODO.txt

@@ -1,5 +1,4 @@
 - last commands:
-  - export
   - format
   - generate
   - push
@@ -18,7 +17,6 @@
   - bufconfig
 
 - lots of QA, especially around workspace file searching, target paths
-- figure out what to do about AllowNotExist and how it fits in
 - document behavior of file searching, config override
 - fix tamper-proofing
 - go through all todos
@@ -43,7 +41,43 @@
     - FIELD_SAME_TYPE from v1beta1 is split into 3 rules in v1: FIELD_SAME_TYPE,
       FIELD_WIRE_COMPATIBLE_TYPE, FIELD_WIRE_JSON_COMPATIBLE_TYPE.
     - Rule `FILE_SAME_PACKAGE` has been added to PACKAGE, WIRE and WIRE_JSON in v1.
+- Un-skip relevant tests in generate_test.go and generate_unix_test.go, when
+  - the behavior of --path and --exclude-path is updated to match what's on main
+- Debug the skipped test with proto file ref in generate_test.go
+- Use syserror when possible (especially in buf gen related code).
+- Find the right place (right way to pass a logger) to print a warning when a buf.gen.yaml with
+  non-empty managed mode but without `enabled: true`. The key is to decide where to pass the logger.
+- Fix the issue where reading a buf.lock with empty digest errors. Run `buf mod update` to reproduce.
 
 NOTE: We are not allowing cross-workspace finding for include_package_files=true
 
 - Decide whether or not excludes and ignores should be relative to module directory
+
+  // This is new post-refactor. Before, we gave precedence to --path. While a change,
+  // doing --path foo/bar --exclude-path foo seems like a bug rather than expected behavior to maintain.
+
+- Changed protoc-gen-buf-lint and protoc-gen-buf-breaking configuration, deprecated old configurations
+- Need to manually test protoc-gen-buf-breaking
+
+NEW ALGORITHM
+
+- Take input, list<paths>, list<excluded_paths> -> suspected_workspace_bucket, list<workspace_paths>, list<workspace_exclude_paths>
+  - .git#subdir=proto --path proto/foo --path proto/bar.proto --exclude-path proto/baz -> .git, [proto/foo, proto/bar.proto] [proto/baz]
+  - work/mod --path work/mod/foo --path work/mod/bar.proto -> wor, [mod/foo, mod/bar.proto]
+  - Treat subdir like any other filter path, deduplicate them
+  - Treat a dirRef input path like any other filter path on "/", deduplicate them
+  - So that is, if workspace is at .. and you pass buf build foo --path foo/bar --path foo/baz, you get .. -> foo/bar, foo/baz
+  - Validate that paths fall within the suspected workspace bucket
+  - Validate no workspace_exclude_path equals or contains a workspace_path
+- Take suspected_workspace_paths, find controlling module or workspace if exists -> potential_module_dir_paths
+  - controlling module does onto being filtered into a controlling workspace, but can live on its own?
+  - No need to special case .proto files -> they could be dirs, treat them as prefixes all the same.
+  - Controlling module is any module that doesn't exclude the workspace_path via buf.yaml excludes
+  - By default, assume directory of path if no module can be found
+- Take potential_module_dir_paths, find controlling workspace -> controlling_workspace_paths
+  - Controlling workspace is the workspace that includes the potential_module_dir_paths
+    - a directory in the module equals or contains the potential_module_dir_paths
+- Make sure len(controlling_workspace_paths) == 1
+- Take workspace_paths, assign to modules in the controlling workspace
+- Split workspace_paths into <module_path, module_target_path>
+- Take workspace_exclude_paths, use to delete module_paths, otherwise create module_target_exclude_paths

make/buf/all.mk

@@ -5,9 +5,9 @@ GO_BINS := $(GO_BINS) \
 	cmd/buf \
 	cmd/protoc-gen-buf-breaking \
 	cmd/protoc-gen-buf-lint \
+	private/buf/bufwkt/cmd/wkt-go-data \
 	private/buf/cmd/buf-digest \
 	private/bufpkg/bufstyle/cmd/bufstyle \
-	private/bufpkg/bufwkt/cmd/wkt-go-data \
 	private/pkg/bandeps/cmd/bandeps \
 	private/pkg/git/cmd/git-ls-files-unstaged \
 	private/pkg/storage/cmd/ddiff \
@@ -25,7 +25,7 @@ FILE_IGNORES := $(FILE_IGNORES) \
 	.vscode/ \
 	private/buf/cmd/buf/command/alpha/protoc/test.txt \
 	private/buf/cmd/buf/workspacetests/other/proto/workspacetest/cache/ \
-	private/bufpkg/buftesting/cache/ \
+	private/buf/buftesting/cache/ \
 	private/pkg/storage/storageos/tmp/
 LICENSE_HEADER_LICENSE_TYPE := apache
 LICENSE_HEADER_COPYRIGHT_HOLDER := Buf Technologies, Inc.
@@ -54,48 +54,8 @@ CMD ?= test
 .PHONY: testbufnew
 testbufnew: installbuf
 	# TODO: remove when done with refactor
-	go $(CMD) \
-		./private/buf/bufapp/... \
-		./private/buf/bufcli/... \
-		./private/buf/bufctl/... \
-		./private/buf/bufcurl/... \
-		./private/buf/buffetch/... \
-		./private/buf/bufformat/... \
-		./private/buf/bufprint/... \
-		./private/buf/bufmigrate/... \
-		./private/buf/bufworkspace/... \
-		./private/buf/cmd/buf/command/alpha/package/... \
-		./private/buf/cmd/buf/command/alpha/protoc/... \
-		./private/buf/cmd/buf/command/alpha/registry/... \
-		./private/buf/cmd/buf/command/beta/graph/... \
-		./private/buf/cmd/buf/command/beta/price/... \
-		./private/buf/cmd/buf/command/beta/registry/... \
-		./private/buf/cmd/buf/command/beta/stats/... \
-		./private/buf/cmd/buf/command/beta/studioagent/... \
-		./private/buf/cmd/buf/command/build/... \
-		./private/buf/cmd/buf/command/breaking/... \
-		./private/buf/cmd/buf/command/convert/... \
-		./private/buf/cmd/buf/command/lint/... \
-		./private/buf/cmd/buf/command/lsfiles/... \
-		./private/buf/cmd/buf/command/mod/... \
-		./private/buf/cmd/buf/command/registry/... \
-		./private/buf/cmd/buf-digest/... \
-		./private/bufpkg/bufanalysis/... \
-		./private/bufpkg/bufapi/... \
-		./private/bufpkg/bufcas/... \
-		./private/bufpkg/bufcheck/... \
-		./private/bufpkg/bufconfig/... \
-		./private/bufpkg/bufconnect/... \
-		./private/bufpkg/bufimage/... \
-		./private/bufpkg/bufplugin/... \
-		./private/bufpkg/bufpluginexec/... \
-		./private/bufpkg/bufreflect/... \
-		./private/bufpkg/bufremoteplugin/... \
-		./private/bufpkg/bufstudioagent/... \
-		./private/bufpkg/bufstyle/... \
-		./private/bufpkg/buftesting/... \
-		./private/bufpkg/buftransport/... \
-		./private/bufpkg/bufwasm/...
+	# Still need to do push, migrate on top of this (push was commented out, migrate-v1beta1 was removed)
+	go $(CMD) $(shell go list -e ./cmd/... ./private/... | grep -v buf\/bufsync | grep -v buf\/command\/format | grep -v command\/alpha\/repo\/reposync | grep -v bufwkt\/cmd\/wkt-go-data)
 
 installtest:: $(PROTOC) $(PROTOC_GEN_GO)
 

private/buf/bufapp/bufapp.go

@@ -18,7 +18,7 @@ import (
 	"crypto/tls"
 	"fmt"
 
-	"github.com/bufbuild/buf/private/pkg/app/appname"
+	"github.com/bufbuild/buf/private/pkg/app/appext"
 	"github.com/bufbuild/buf/private/pkg/cert/certclient"
 )
 
@@ -44,7 +44,7 @@ type Config struct {
 
 // NewConfig returns a new Config for the ExternalConfig.
 func NewConfig(
-	container appname.Container,
+	container appext.NameContainer,
 	externalConfig ExternalConfig,
 ) (*Config, error) {
 	if externalConfig.Version != currentVersion && !externalConfig.IsEmpty() {

private/buf/bufcli/connectclient_config.go

@@ -20,8 +20,7 @@ import (
 	"github.com/bufbuild/buf/private/buf/bufapp"
 	"github.com/bufbuild/buf/private/bufpkg/bufconnect"
 	"github.com/bufbuild/buf/private/bufpkg/buftransport"
-	"github.com/bufbuild/buf/private/pkg/app/appflag"
-	"github.com/bufbuild/buf/private/pkg/app/appname"
+	"github.com/bufbuild/buf/private/pkg/app/appext"
 	"github.com/bufbuild/buf/private/pkg/connectclient"
 	"github.com/bufbuild/buf/private/pkg/netrc"
 	"github.com/bufbuild/buf/private/pkg/transport/http/httpclient"
@@ -30,7 +29,7 @@ import (
 // NewConnectClientConfig creates a new connect.ClientConfig which uses a token reader to look
 // up the token in the container or in netrc based on the address of each individual client.
 // It is then set in the header of all outgoing requests from clients created using this config.
-func NewConnectClientConfig(container appflag.Container) (*connectclient.Config, error) {
+func NewConnectClientConfig(container appext.Container) (*connectclient.Config, error) {
 	envTokenProvider, err := bufconnect.NewTokenProviderFromContainer(container)
 	if err != nil {
 		return nil, err
@@ -46,7 +45,7 @@ func NewConnectClientConfig(container appflag.Container) (*connectclient.Config,
 
 // NewConnectClientConfigWithToken creates a new connect.ClientConfig with a given token. The provided token is
 // set in the header of all outgoing requests from this provider
-func NewConnectClientConfigWithToken(container appflag.Container, token string) (*connectclient.Config, error) {
+func NewConnectClientConfigWithToken(container appext.Container, token string) (*connectclient.Config, error) {
 	tokenProvider, err := bufconnect.NewTokenProviderFromString(token)
 	if err != nil {
 		return nil, err
@@ -60,7 +59,7 @@ func NewConnectClientConfigWithToken(container appflag.Container, token string)
 }
 
 // Returns a registry provider with the given options applied in addition to default ones for all providers
-func newConnectClientConfigWithOptions(container appflag.Container, opts ...connectclient.ConfigOption) (*connectclient.Config, error) {
+func newConnectClientConfigWithOptions(container appext.Container, opts ...connectclient.ConfigOption) (*connectclient.Config, error) {
 	config, err := newConfig(container)
 	if err != nil {
 		return nil, err
@@ -85,9 +84,9 @@ func newConnectClientConfigWithOptions(container appflag.Container, opts ...conn
 }
 
 // newConfig creates a new Config.
-func newConfig(container appflag.Container) (*bufapp.Config, error) {
+func newConfig(container appext.Container) (*bufapp.Config, error) {
 	externalConfig := bufapp.ExternalConfig{}
-	if err := appname.ReadConfig(container, &externalConfig); err != nil {
+	if err := appext.ReadConfig(container, &externalConfig); err != nil {
 		return nil, err
 	}
 	return bufapp.NewConfig(container, externalConfig)

private/buf/bufcli/controller.go

@@ -15,15 +15,16 @@
 package bufcli
 
 import (
-	"github.com/bufbuild/buf/private/bufpkg/bufapi"
 	"github.com/bufbuild/buf/private/buf/bufctl"
+	"github.com/bufbuild/buf/private/bufpkg/bufapi"
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleapi"
-	"github.com/bufbuild/buf/private/pkg/app/appflag"
+	"github.com/bufbuild/buf/private/pkg/app/appext"
+	"github.com/bufbuild/buf/private/pkg/tracing"
 )
 
 // NewController returns a new Controller.
 func NewController(
-	container appflag.Container,
+	container appext.Container,
 	options ...bufctl.ControllerOption,
 ) (bufctl.Controller, error) {
 	clientConfig, err := NewConnectClientConfig(container)
@@ -37,6 +38,7 @@ func NewController(
 	}
 	return bufctl.NewController(
 		container.Logger(),
+		tracing.NewTracer(container.Tracer()),
 		container,
 		bufmoduleapi.NewModuleKeyProvider(container.Logger(), clientProvider),
 		moduleDataProvider,

private/buf/bufcli/env.go

@@ -18,7 +18,7 @@ import (
 	"context"
 	"net/http"
 
-	"github.com/bufbuild/buf/private/pkg/app/appflag"
+	"github.com/bufbuild/buf/private/pkg/app/appext"
 	"github.com/bufbuild/buf/private/pkg/git"
 	"github.com/bufbuild/buf/private/pkg/httpauth"
 )
@@ -58,15 +58,15 @@ var (
 
 // WarnAlphaCommand prints a warning for a alpha command unless the alphaSuppressWarningsEnvKey
 // environment variable is set.
-func WarnAlphaCommand(_ context.Context, container appflag.Container) {
+func WarnAlphaCommand(_ context.Context, container appext.Container) {
 	if container.Env(alphaSuppressWarningsEnvKey) == "" {
 		container.Logger().Warn("This command is in alpha. It is hidden for a reason. This command is purely for development purposes, and may never even be promoted to beta, do not rely on this command's functionality. To suppress this warning, set " + alphaSuppressWarningsEnvKey + "=1")
 	}
 }
 
 // WarnBetaCommand prints a warning for a beta command unless the betaSuppressWarningsEnvKey
 // environment variable is set.
-func WarnBetaCommand(_ context.Context, container appflag.Container) {
+func WarnBetaCommand(_ context.Context, container appext.Container) {
 	if container.Env(betaSuppressWarningsEnvKey) == "" {
 		container.Logger().Warn("This command is in beta. It is unstable and likely to change. To suppress this warning, set " + betaSuppressWarningsEnvKey + "=1")
 	}

private/buf/bufcli/flags_args.go

@@ -259,7 +259,7 @@ func VisibilityFlagToVisibilityAllowUnspecified(visibility string) (registryv1al
 
 // ValidateRequiredFlag validates that the required flag is set.
 //
-// TODO: Is this needed over cobra.MarkFlagRequired?
+// TODO: Is this needed over appcmd.MarkFlagRequired?
 func ValidateRequiredFlag[T comparable](flagName string, value T) error {
 	var zero T
 	if value == zero {

private/buf/bufcli/module_data_provider.go

@@ -26,7 +26,7 @@ import (
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleapi"
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmodulecache"
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmodulestore"
-	"github.com/bufbuild/buf/private/pkg/app/appflag"
+	"github.com/bufbuild/buf/private/pkg/app/appext"
 	"github.com/bufbuild/buf/private/pkg/normalpath"
 	"github.com/bufbuild/buf/private/pkg/storage/storageos"
 )
@@ -87,7 +87,7 @@ var (
 
 // NewModuleDataProvider returns a new ModuleDataProvider while creating the
 // required cache directories.
-func NewModuleDataProvider(container appflag.Container) (bufmodule.ModuleDataProvider, error) {
+func NewModuleDataProvider(container appext.Container) (bufmodule.ModuleDataProvider, error) {
 	clientConfig, err := NewConnectClientConfig(container)
 	if err != nil {
 		return nil, err
@@ -101,7 +101,7 @@ func NewModuleDataProvider(container appflag.Container) (bufmodule.ModuleDataPro
 }
 
 func newModuleDataProvider(
-	container appflag.Container,
+	container appext.Container,
 	clientProvider bufapi.ClientProvider,
 ) (bufmodule.ModuleDataProvider, error) {
 	if err := createCacheDir(container.CacheDirPath(), v3CacheModuleRelDirPath); err != nil {

private/buf/bufcli/module_key_provider.go

@@ -18,11 +18,11 @@ import (
 	"github.com/bufbuild/buf/private/bufpkg/bufapi"
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule"
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleapi"
-	"github.com/bufbuild/buf/private/pkg/app/appflag"
+	"github.com/bufbuild/buf/private/pkg/app/appext"
 )
 
 // NewModuleKeyProvider returns a new ModuleKeyProvider.
-func NewModuleKeyProvider(container appflag.Container) (bufmodule.ModuleKeyProvider, error) {
+func NewModuleKeyProvider(container appext.Container) (bufmodule.ModuleKeyProvider, error) {
 	clientConfig, err := NewConnectClientConfig(container)
 	if err != nil {
 		return nil, err