From 6cd717840e63fe114a1b470cffd8a54c306d06aa Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Mon, 23 Sep 2024 16:25:30 -0400 Subject: [PATCH 01/48] Init support for WASM plugins --- go.mod | 1 + go.sum | 2 + make/buf/all.mk | 7 + make/go/go.mk | 18 +- private/buf/bufcli/cache.go | 15 ++ private/buf/buflsp/buflsp.go | 7 +- .../buf/cmd/buf/command/breaking/breaking.go | 7 +- private/buf/cmd/buf/command/lint/lint.go | 24 ++- private/bufpkg/bufcheck/bufcheck.go | 35 ++-- private/bufpkg/bufcheck/lint_test.go | 50 ++++- private/bufpkg/bufcheck/multi_client.go | 7 + private/bufpkg/bufcheck/runner_provider.go | 184 ++++++++++++++++++ .../testdata/lint/custom_wasm_plugins/a.proto | 28 +++ .../testdata/lint/custom_wasm_plugins/b.proto | 22 +++ .../lint/custom_wasm_plugins/buf.yaml | 20 ++ private/bufpkg/bufconfig/plugin_config.go | 48 ++++- private/bufpkg/bufwasm/bufwasm.go | 81 ++++++++ private/bufpkg/bufwasm/plugin.go | 66 +++++++ private/bufpkg/bufwasm/runtime.go | 129 ++++++++++++ private/bufpkg/bufwasm/usage.gen.go | 19 ++ private/usage/usage_unix.go | 3 +- private/usage/usage_windows.go | 1 - 22 files changed, 743 insertions(+), 31 deletions(-) create mode 100644 private/bufpkg/bufcheck/runner_provider.go create mode 100644 private/bufpkg/bufcheck/testdata/lint/custom_wasm_plugins/a.proto create mode 100644 private/bufpkg/bufcheck/testdata/lint/custom_wasm_plugins/b.proto create mode 100644 private/bufpkg/bufcheck/testdata/lint/custom_wasm_plugins/buf.yaml create mode 100644 private/bufpkg/bufwasm/bufwasm.go create mode 100644 private/bufpkg/bufwasm/plugin.go create mode 100644 private/bufpkg/bufwasm/runtime.go create mode 100644 private/bufpkg/bufwasm/usage.gen.go diff --git a/go.mod b/go.mod index 76117b6431..9e0d7be733 100644 --- a/go.mod +++ b/go.mod @@ -34,6 +34,7 @@ require ( github.com/spf13/cobra v1.8.1 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.9.0 + github.com/tetratelabs/wazero v1.8.0 go.lsp.dev/jsonrpc2 v0.10.0 go.lsp.dev/protocol v0.12.0 go.opentelemetry.io/otel v1.30.0 diff --git a/go.sum b/go.sum index c49d5bf29d..437a89e4f8 100644 --- a/go.sum +++ b/go.sum @@ -252,6 +252,8 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/tetratelabs/wazero v1.8.0 h1:iEKu0d4c2Pd+QSRieYbnQC9yiFlMS9D+Jr0LsRmcF4g= +github.com/tetratelabs/wazero v1.8.0/go.mod h1:yAI0XTsMBhREkM/YDAK/zNou3GoiAce1P6+rp/wQhjs= github.com/vbatts/tar-split v0.11.5 h1:3bHCTIheBm1qFTcgh9oPu+nNBtX+XJIupG/vacinCts= github.com/vbatts/tar-split v0.11.5/go.mod h1:yZbwRsSeGjusneWgA781EKej9HF8vme8okylkAeNKLk= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/make/buf/all.mk b/make/buf/all.mk index 4f0aa08257..84512e4bf7 100644 --- a/make/buf/all.mk +++ b/make/buf/all.mk @@ -25,6 +25,13 @@ GO_TEST_BINS := $(GO_TEST_BINS) \ private/bufpkg/bufcheck/internal/cmd/buf-plugin-rpc-ext \ private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-category \ private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-rule +GO_TEST_WASM_BINS := $(GO_TEST_WASM_BINS) \ + private/bufpkg/bufcheck/internal/cmd/buf-plugin-panic \ + private/bufpkg/bufcheck/internal/cmd/buf-plugin-suffix \ + private/bufpkg/bufcheck/internal/cmd/buf-plugin-protovalidate-ext \ + private/bufpkg/bufcheck/internal/cmd/buf-plugin-rpc-ext \ + private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-category \ + private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-rule GO_MOD_VERSION := 1.22 DOCKER_BINS := $(DOCKER_BINS) buf FILE_IGNORES := $(FILE_IGNORES) \ diff --git a/make/go/go.mk b/make/go/go.mk index 503d72a53b..36b356efcb 100644 --- a/make/go/go.mk +++ b/make/go/go.mk @@ -16,6 +16,8 @@ GO_BINS ?= # Settable GO_TEST_BINS ?= # Settable +GO_TEST_WASM_BINS ?= +# Settable GO_GET_PKGS ?= # Settable GO_MOD_VERSION ?= 1.21 @@ -142,7 +144,7 @@ build: prebuild ## Run go build. pretest:: .PHONY: test -test: pretest installtest ## Run all go tests. +test: pretest installtest installtestwasm ## Run all go tests. go test $(GO_TEST_FLAGS) $(GOPKGS) .PHONY: testrace @@ -203,3 +205,17 @@ endef $(foreach gobin,$(sort $(GO_TEST_BINS)),$(eval $(call gotestbinfunc,$(gobin)))) $(foreach gobin,$(sort $(GO_TEST_BINS)),$(eval FILE_IGNORES := $(FILE_IGNORES) $(gobin)/$(notdir $(gobin)))) + +.PHONY: installtestwasm +installtestwasm:: + +define gotestwasmfunc +.PHONY: installtestwasm$(notdir $(1)) +installtestwasm$(notdir $(1)): + GOOS=wasip1 GOARCH=wasm go build -o $(GOBIN)/$(notdir $(1)).wasm ./$(1) + +installtestwasm:: installtestwasm$(notdir $(1)) +endef + +$(foreach gobin,$(sort $(GO_TEST_WASM_BINS)),$(eval $(call gotestwasmfunc,$(gobin)))) +$(foreach gobin,$(sort $(GO_TEST_WASM_BINS)),$(eval FILE_IGNORES := $(FILE_IGNORES) $(gobin)/$(notdir $(gobin)))) diff --git a/private/buf/bufcli/cache.go b/private/buf/bufcli/cache.go index bd9a11f876..17f1900424 100644 --- a/private/buf/bufcli/cache.go +++ b/private/buf/bufcli/cache.go @@ -103,6 +103,10 @@ var ( // // Normalized. v3CacheModuleLockRelDirPath = normalpath.Join("v3", "modulelocks") + // v3CachePluginsRelDirPath is the relative path to the plugins cache directory in its newest iteration. + // + // Normalized. + v3CachePluginsRelDirPath = normalpath.Join("v3", "plugins") ) // NewModuleDataProvider returns a new ModuleDataProvider while creating the @@ -135,6 +139,17 @@ func NewCommitProvider(container appext.Container) (bufmodule.CommitProvider, er ) } +// CreatePluginCacheDir creates the cache directory for plugins. +// +// This is used by the [bufwasm.WithLocalCacheDir] option. +func CreatePluginCacheDir(container appext.Container) (string, error) { + if err := createCacheDir(container.CacheDirPath(), v3CachePluginsRelDirPath); err != nil { + return "", err + } + fullCacheDirPath := normalpath.Join(container.CacheDirPath(), v3CachePluginsRelDirPath) + return fullCacheDirPath, nil +} + // newWKTStore returns a new bufwktstore.Store while creating the required cache directories. func newWKTStore(container appext.Container) (bufwktstore.Store, error) { if err := createCacheDir(container.CacheDirPath(), v3CacheWKTRelDirPath); err != nil { diff --git a/private/buf/buflsp/buflsp.go b/private/buf/buflsp/buflsp.go index da572dfc11..5865d91bc4 100644 --- a/private/buf/buflsp/buflsp.go +++ b/private/buf/buflsp/buflsp.go @@ -58,7 +58,12 @@ func Serve( } tracer := tracing.NewTracer(container.Tracer()) - checkClient, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr())) + checkClient, err := bufcheck.NewClient( + container.Logger(), + tracer, + bufcheck.NewRunnerProvider(command.NewRunner()), + bufcheck.ClientWithStderr(container.Stderr()), + ) if err != nil { return nil, err } diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index 5b8263d20e..007e860b77 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -209,7 +209,12 @@ func run( tracer := tracing.NewTracer(container.Tracer()) var allFileAnnotations []bufanalysis.FileAnnotation for i, imageWithConfig := range imageWithConfigs { - client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr())) + client, err := bufcheck.NewClient( + container.Logger(), + tracer, + bufcheck.NewRunnerProvider(command.NewRunner()), + bufcheck.ClientWithStderr(container.Stderr()), + ) if err != nil { return err } diff --git a/private/buf/cmd/buf/command/lint/lint.go b/private/buf/cmd/buf/command/lint/lint.go index 136b7b97dc..534c1c9b57 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -23,12 +23,14 @@ import ( "github.com/bufbuild/buf/private/buf/bufctl" "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" + "github.com/bufbuild/buf/private/bufpkg/bufwasm" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/bufbuild/buf/private/pkg/tracing" "github.com/spf13/pflag" + "go.uber.org/multierr" ) const ( @@ -131,10 +133,30 @@ func run( if err != nil { return err } + pluginCacheDir, err := bufcli.CreatePluginCacheDir(container) + if err != nil { + return err + } + wasmRuntime, err := bufwasm.NewRuntime( + ctx, + bufwasm.WithLocalCacheDir(pluginCacheDir), + ) + if err != nil { + return err + } + defer func() { retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) }() tracer := tracing.NewTracer(container.Tracer()) var allFileAnnotations []bufanalysis.FileAnnotation for _, imageWithConfig := range imageWithConfigs { - client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr())) + client, err := bufcheck.NewClient( + container.Logger(), + tracer, + bufcheck.NewRunnerProvider( + command.NewRunner(), + bufcheck.RunnerProviderWithWASMRuntime(wasmRuntime), + ), + bufcheck.ClientWithStderr(container.Stderr()), + ) if err != nil { return err } diff --git a/private/bufpkg/bufcheck/bufcheck.go b/private/bufpkg/bufcheck/bufcheck.go index c921624fb3..403fb0ed54 100644 --- a/private/bufpkg/bufcheck/bufcheck.go +++ b/private/bufpkg/bufcheck/bufcheck.go @@ -21,8 +21,8 @@ import ( "buf.build/go/bufplugin/check" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/bufpkg/bufwasm" "github.com/bufbuild/buf/private/pkg/command" - "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/bufbuild/buf/private/pkg/tracing" @@ -170,21 +170,24 @@ func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (plug } // NewRunnerProvider returns a new RunnerProvider for the command.Runner. -func NewRunnerProvider(delegate command.Runner) RunnerProvider { - return RunnerProviderFunc( - func(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) { - if pluginConfig.Type() != bufconfig.PluginConfigTypeLocal { - return nil, syserror.New("only local plugins are supported") - } - path := pluginConfig.Path() - return pluginrpcutil.NewRunner( - delegate, - // We know that Path is of at least length 1. - path[0], - path[1:]..., - ), nil - }, - ) +// +// This implementation should only be used for local applications. +func NewRunnerProvider( + delegate command.Runner, + options ...RunnerProviderOption, +) RunnerProvider { + return newRunnerProvider(delegate, options...) +} + +// RunnerProviderOption is an option for NewRunnerProvider. +type RunnerProviderOption func(*runnerProviderOptions) + +// RunnerProviderWithWASMRuntime returns a new RunnerProviderOption that +// specifies a WASM runtime. This is required for local WASM plugins. +func RunnerProviderWithWASMRuntime(wasmRuntime bufwasm.Runtime) RunnerProviderOption { + return func(runnerProviderOptions *runnerProviderOptions) { + runnerProviderOptions.wasmRuntime = wasmRuntime + } } // NewClient returns a new Client. diff --git a/private/bufpkg/bufcheck/lint_test.go b/private/bufpkg/bufcheck/lint_test.go index 4e93324b0d..610fafe406 100644 --- a/private/bufpkg/bufcheck/lint_test.go +++ b/private/bufpkg/bufcheck/lint_test.go @@ -27,6 +27,7 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/bufpkg/bufmodule" + "github.com/bufbuild/buf/private/bufpkg/bufwasm" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/tracing" @@ -511,6 +512,7 @@ func TestRunPackageNoImportCycle(t *testing.T) { require.NoError(t, err) return newImage }, + false, bufanalysistesting.NewFileAnnotation(t, "c1.proto", 5, 1, 5, 19, "PACKAGE_NO_IMPORT_CYCLE"), bufanalysistesting.NewFileAnnotation(t, "d1.proto", 5, 1, 5, 19, "PACKAGE_NO_IMPORT_CYCLE"), ) @@ -593,7 +595,8 @@ func TestRunProtovalidate(t *testing.T) { t, "protovalidate", "buf.testing/lint/protovalidate", - nil, + nil, // no image modification + false, // no wasm runtime bufanalysistesting.NewFileAnnotation(t, "bool.proto", 18, 51, 18, 84, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "bool.proto", 19, 31, 19, 69, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "bool.proto", 20, 50, 20, 88, "PROTOVALIDATE"), @@ -1026,7 +1029,8 @@ func TestRunV2WorkspaceIgnores(t *testing.T) { t, "v2/ignores", "ignores1", - nil, + nil, // no image modification + false, // no wasm runtime bufanalysistesting.NewFileAnnotation(t, "bar1/bar.proto", 6, 9, 6, 15, "FIELD_LOWER_SNAKE_CASE"), bufanalysistesting.NewFileAnnotation(t, "bar1/bar.proto", 9, 9, 9, 12, "MESSAGE_PASCAL_CASE"), bufanalysistesting.NewFileAnnotation(t, "bar1/bar.proto", 13, 6, 13, 9, "ENUM_PASCAL_CASE"), @@ -1050,7 +1054,8 @@ func TestRunV2WorkspaceIgnores(t *testing.T) { t, "v2/ignores", "ignores2", - nil, + nil, // no image modification + false, // no wasm runtime bufanalysistesting.NewFileAnnotation(t, "bar2/bar.proto", 6, 9, 6, 15, "FIELD_LOWER_SNAKE_CASE"), bufanalysistesting.NewFileAnnotation(t, "bar2/bar.proto", 9, 9, 9, 12, "MESSAGE_PASCAL_CASE"), bufanalysistesting.NewFileAnnotation(t, "bar2/bar.proto", 13, 6, 13, 9, "ENUM_PASCAL_CASE"), @@ -1062,7 +1067,8 @@ func TestRunV2WorkspaceIgnores(t *testing.T) { t, "v2/ignores", "ignores3", - nil, + nil, // no image modification + false, // no wasm runtime bufanalysistesting.NewFileAnnotation(t, "bar3/bar.proto", 6, 9, 6, 15, "FIELD_LOWER_SNAKE_CASE"), bufanalysistesting.NewFileAnnotation(t, "bar3/bar2.proto", 6, 9, 6, 15, "FIELD_LOWER_SNAKE_CASE"), bufanalysistesting.NewFileAnnotation(t, "bar3/bar2.proto", 9, 9, 9, 13, "MESSAGE_PASCAL_CASE"), @@ -1214,6 +1220,25 @@ func TestRunLintCustomPlugins(t *testing.T) { ) } +func TestRunLintCustomWASMPlugins(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping test in short mode") + } + testLintWithOptions( + t, + "custom_wasm_plugins", + "", + nil, // no image modification + true, // wasm runtime + bufanalysistesting.NewFileAnnotationNoLocation(t, "a.proto", "PACKAGE_DEFINED"), + bufanalysistesting.NewFileAnnotation(t, "a.proto", 8, 1, 10, 2, "SERVICE_BANNED_SUFFIXES"), + bufanalysistesting.NewFileAnnotation(t, "b.proto", 6, 3, 6, 66, "RPC_BANNED_SUFFIXES"), + bufanalysistesting.NewFileAnnotation(t, "b.proto", 14, 5, 14, 24, "ENUM_VALUE_BANNED_SUFFIXES"), + bufanalysistesting.NewFileAnnotation(t, "b.proto", 19, 5, 19, 23, "FIELD_BANNED_SUFFIXES"), + ) +} + func testLint( t *testing.T, relDirPath string, @@ -1224,6 +1249,7 @@ func testLint( relDirPath, "", nil, + false, expectedFileAnnotations..., ) } @@ -1234,9 +1260,10 @@ func testLintWithOptions( // only set if in workspace moduleFullNameString string, imageModifier func(bufimage.Image) bufimage.Image, + wasmRuntime bool, expectedFileAnnotations ...bufanalysis.FileAnnotation, ) { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) // Increased timeout for WASM runtime defer cancel() baseDirPath := filepath.Join("testdata", "lint") @@ -1293,7 +1320,18 @@ func testLintWithOptions( lintConfig := workspace.GetLintConfigForOpaqueID(opaqueID) require.NotNil(t, lintConfig) - client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufcheck.NewRunnerProvider(command.NewRunner())) + var runnerProviderOptions []bufcheck.RunnerProviderOption + if wasmRuntime { + wasmRuntime, err := bufwasm.NewRuntime(ctx) + require.NoError(t, err) + t.Cleanup(func() { assert.NoError(t, wasmRuntime.Release(ctx)) }) + runnerProviderOptions = append(runnerProviderOptions, bufcheck.RunnerProviderWithWASMRuntime(wasmRuntime)) + } + client, err := bufcheck.NewClient( + zap.NewNop(), + tracing.NopTracer, + bufcheck.NewRunnerProvider(command.NewRunner(), runnerProviderOptions...), + ) require.NoError(t, err) err = client.Lint( ctx, diff --git a/private/bufpkg/bufcheck/multi_client.go b/private/bufpkg/bufcheck/multi_client.go index 861b55e4ce..83b6cdaa4d 100644 --- a/private/bufpkg/bufcheck/multi_client.go +++ b/private/bufpkg/bufcheck/multi_client.go @@ -20,6 +20,7 @@ import ( "sort" "strings" "sync" + "time" "buf.build/go/bufplugin/check" "github.com/bufbuild/buf/private/pkg/slicesext" @@ -94,6 +95,7 @@ func (c *multiClient) Check(ctx context.Context, request check.Request) ([]*anno jobs = append( jobs, func(ctx context.Context) error { + start := time.Now() delegateResponse, err := delegate.Client.Check(ctx, delegateRequest) if err != nil { if delegate.PluginName == "" { @@ -110,6 +112,7 @@ func (c *multiClient) Check(ctx context.Context, request check.Request) ([]*anno lock.Lock() allAnnotations = append(allAnnotations, annotations...) lock.Unlock() + c.logger.Debug("checked delegate client", zap.String("pluginName", delegate.PluginName), zap.Duration("duration", time.Since(start))) return nil }, ) @@ -151,6 +154,7 @@ func (c *multiClient) getRulesCategoriesAndChunkedIDs(ctx context.Context) ( var rules []Rule chunkedRuleIDs := make([][]string, len(c.checkClientSpecs)) for i, delegate := range c.checkClientSpecs { + start := time.Now() delegateCheckRules, err := delegate.Client.ListRules(ctx) if err != nil { if delegate.PluginName == "" { @@ -165,11 +169,13 @@ func (c *multiClient) getRulesCategoriesAndChunkedIDs(ctx context.Context) ( rules = append(rules, delegateRules...) // Already sorted. chunkedRuleIDs[i] = slicesext.Map(delegateRules, Rule.ID) + c.logger.Debug("list rules delegate client", zap.String("pluginName", delegate.PluginName), zap.Duration("duration", time.Since(start))) } var categories []Category chunkedCategoryIDs := make([][]string, len(c.checkClientSpecs)) for i, delegate := range c.checkClientSpecs { + start := time.Now() delegateCheckCategories, err := delegate.Client.ListCategories(ctx) if err != nil { if delegate.PluginName == "" { @@ -184,6 +190,7 @@ func (c *multiClient) getRulesCategoriesAndChunkedIDs(ctx context.Context) ( categories = append(categories, delegateCategories...) // Already sorted. chunkedCategoryIDs[i] = slicesext.Map(delegateCategories, Category.ID) + c.logger.Debug("list categories delegate client", zap.String("pluginName", delegate.PluginName), zap.Duration("duration", time.Since(start))) } if err := validateNoDuplicateRulesOrCategories(rules, categories); err != nil { diff --git a/private/bufpkg/bufcheck/runner_provider.go b/private/bufpkg/bufcheck/runner_provider.go new file mode 100644 index 0000000000..7279711432 --- /dev/null +++ b/private/bufpkg/bufcheck/runner_provider.go @@ -0,0 +1,184 @@ +// 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 bufcheck + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "sync" + + "github.com/bufbuild/buf/private/bufpkg/bufconfig" + "github.com/bufbuild/buf/private/bufpkg/bufwasm" + "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/pluginrpcutil" + "github.com/bufbuild/buf/private/pkg/syserror" + "pluginrpc.com/pluginrpc" +) + +type runnerProvider struct { + delegate command.Runner + wasmRuntime bufwasm.Runtime +} + +func newRunnerProvider(delegate command.Runner, options ...RunnerProviderOption) *runnerProvider { + runnerProviderOptions := newRunnerProviderOptions() + for _, option := range options { + option(runnerProviderOptions) + } + return &runnerProvider{ + delegate: delegate, + wasmRuntime: runnerProviderOptions.wasmRuntime, + } +} + +func (r *runnerProvider) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) { + switch pluginConfig.Type() { + case bufconfig.PluginConfigTypeLocal: + path := pluginConfig.Path() + return pluginrpcutil.NewRunner( + r.delegate, + // We know that Path is of at least length 1. + path[0], + path[1:]..., + ), nil + case bufconfig.PluginConfigTypeLocalWASM: + if r.wasmRuntime == nil { + return nil, syserror.Newf("wasm runtime is required for local wasm plugins") + } + return newWASMRunner( + r.wasmRuntime, + pluginConfig.Name(), + ), nil + default: + return nil, syserror.Newf("unsupported plugin type: %v", pluginConfig.Type()) + } +} + +type runnerProviderOptions struct { + wasmRuntime bufwasm.Runtime +} + +func newRunnerProviderOptions() *runnerProviderOptions { + return &runnerProviderOptions{ + wasmRuntime: bufwasm.NewUnimplementedRuntime(), + } +} + +// wasmRunner is a runner that loads a WASM plugin. +type wasmRunner struct { + programName string + wasmRuntime bufwasm.Runtime + // Once protects plugin and pluginErr. + once sync.Once + plugin bufwasm.Plugin + pluginErr error +} + +// newWASMRunner returns a new pluginrpc.Runner for the WASM binary on a +// bufwasm.Runtime and program name. This runner is only suitable for use with +// short-lived programs, compiled plugins lifetime is tied to the runtime. +// +// The program name should be the name of the program as it appears in the +// plugin config. The runner will call os.GetEnv("PATH") with os.Stat on each +// directory and file to find the program. This is similar to exec.LookPath +// but does not require the file to be executable. This is only safe for use +// in the CLI, as it is not safe to use in a server environment. +func newWASMRunner( + runtime bufwasm.Runtime, + programName string, +) *wasmRunner { + return &wasmRunner{ + programName: programName, + wasmRuntime: runtime, + } +} + +func (r *wasmRunner) Run(ctx context.Context, env pluginrpc.Env) (retErr error) { + plugin, err := r.loadPluginOnce(ctx) + if err != nil { + return err + } + return plugin.Run(ctx, env) +} + +func (r *wasmRunner) loadPluginOnce(ctx context.Context) (bufwasm.Plugin, error) { + r.once.Do(func() { + r.plugin, r.pluginErr = r.loadPlugin(ctx) + }) + return r.plugin, r.pluginErr +} + +func (r *wasmRunner) loadPlugin(ctx context.Context) (bufwasm.Plugin, error) { + path, err := lookPath(r.programName) + if err != nil { + return nil, fmt.Errorf("could not find plugin %q in PATH: %v", r.programName, err) + } + wasmModule, err := os.ReadFile(path) + if err != nil { + return nil, err + } + // Compile and run, releasing the plugin at the end. + plugin, err := r.wasmRuntime.Compile(ctx, r.programName, wasmModule) + if err != nil { + return nil, err + } + // This plugin is never released, so subsequent calls to this function + // will benefit from the cached plugin. This is only safe as the + // runner is limited to the CLI. + return plugin, nil +} + +// lookPath looks for a wasm file in the PATH, not only an executable. This +// doesn't use exec.LookPath to avoid requiring an executable bit. +func lookPath(file string) (string, error) { + // First, check in the current directory. + if ok, err := findFile(file); err != nil { + return "", err + } else if ok { + return file, nil + } + // If the file has a path separator, fail early. + if strings.Contains(file, string(os.PathSeparator)) { + return "", os.ErrNotExist + } + path := os.Getenv("PATH") + for _, dir := range filepath.SplitList(path) { + if dir == "" { + // Unix shell semantics: path element "" means "." + dir = "." + } + path := filepath.Join(dir, file) + if ok, err := findFile(path); err != nil { + return "", err + } else if ok { + return path, nil + } + } + return "", os.ErrNotExist +} + +func findFile(file string) (bool, error) { + d, err := os.Stat(file) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + return !d.Mode().IsDir(), nil +} diff --git a/private/bufpkg/bufcheck/testdata/lint/custom_wasm_plugins/a.proto b/private/bufpkg/bufcheck/testdata/lint/custom_wasm_plugins/a.proto new file mode 100644 index 0000000000..94d6ddf2a1 --- /dev/null +++ b/private/bufpkg/bufcheck/testdata/lint/custom_wasm_plugins/a.proto @@ -0,0 +1,28 @@ +syntax = "proto3"; + +service A { + rpc GetA(GetARequest) returns (GetAResponse); + rpc ListA(ListARequest) returns (ListAResponse); +} + +service AMock { + rpc GetAllA(GetAllARequest) returns (GetAllAResponse); +} + +message GetARequest {} +message GetAResponse {} + +message ListARequest { + uint32 page_size = 1; +} + +message ListAResponse { + message Value { + string id = 1; + bytes content = 2; + } + repeated Value values = 1; +} + +message GetAllARequest {} +message GetAllAResponse {} diff --git a/private/bufpkg/bufcheck/testdata/lint/custom_wasm_plugins/b.proto b/private/bufpkg/bufcheck/testdata/lint/custom_wasm_plugins/b.proto new file mode 100644 index 0000000000..2dba34b847 --- /dev/null +++ b/private/bufpkg/bufcheck/testdata/lint/custom_wasm_plugins/b.proto @@ -0,0 +1,22 @@ +syntax = "proto3"; + +package b; + +service B { + rpc GetElement(GetElementRequest) returns (GetElementResponse); +} + +message GetElementRequest {} +message GetElementResponse { + enum Status { + STATUS_UNSPECIFIED = 0; + STATUS_VALID = 1; + STATUS_INVALID = 2; + } + message Value { + string name = 1; + Status status = 2; + string a_uuid = 3; + } + Value value = 1; +} diff --git a/private/bufpkg/bufcheck/testdata/lint/custom_wasm_plugins/buf.yaml b/private/bufpkg/bufcheck/testdata/lint/custom_wasm_plugins/buf.yaml new file mode 100644 index 0000000000..36f6c2273c --- /dev/null +++ b/private/bufpkg/bufcheck/testdata/lint/custom_wasm_plugins/buf.yaml @@ -0,0 +1,20 @@ +version: v2 +lint: + use: + - PACKAGE_DEFINED + - SERVICE_BANNED_SUFFIXES + - RPC_BANNED_SUFFIXES + - FIELD_BANNED_SUFFIXES + - ENUM_VALUE_BANNED_SUFFIXES +plugins: + - plugin: buf-plugin-suffix.wasm + options: + service_banned_suffixes: + - Mock + - Test + rpc_banned_suffixes: + - Element + field_banned_suffixes: + - _uuid + enum_value_banned_suffixes: + - _INVALID diff --git a/private/bufpkg/bufconfig/plugin_config.go b/private/bufpkg/bufconfig/plugin_config.go index f3e8543b7c..ab94813798 100644 --- a/private/bufpkg/bufconfig/plugin_config.go +++ b/private/bufpkg/bufconfig/plugin_config.go @@ -18,6 +18,7 @@ import ( "errors" "strings" + "github.com/bufbuild/buf/private/bufpkg/bufmodule" "github.com/bufbuild/buf/private/pkg/encoding" "github.com/bufbuild/buf/private/pkg/syserror" ) @@ -25,6 +26,8 @@ import ( const ( // PluginConfigTypeLocal is the local plugin config type. PluginConfigTypeLocal PluginConfigType = iota + 1 + // PluginConfigTypeLocalWASM is the local WASM plugin config type. + PluginConfigTypeLocalWASM ) // PluginConfigType is a generate plugin configuration type. @@ -62,6 +65,18 @@ func NewLocalPluginConfig( ) } +// NewLocalWASMPluginConfig returns a new PluginConfig for a local WASM plugin. +func NewLocalWASMPluginConfig( + name string, + options map[string]any, + path []string, +) (PluginConfig, error) { + return newLocalWASMPluginConfig( + name, + options, + ) +} + // *** PRIVATE *** type pluginConfig struct { @@ -85,12 +100,30 @@ func newPluginConfigForExternalV2( } options[key] = value } - // TODO: differentiate between local and remote in the future - // Use the same heuristic that we do for dir vs module in buffetch + // Differentiate between local and remote plugins. + // Local plugins are specified as a path to a binary or a WASM file. + // Remote plugins are specified as a module reference. + // Paths with more than one element are assumed to be local plugin commands. + // This heuristic is based on the buffetch heuristics for inputs. path, err := encoding.InterfaceSliceOrStringToStringSlice(externalConfig.Plugin) if err != nil { return nil, err } + if len(path) == 1 { + name := path[0] + // TODO: Parse as a module reference to fail early if we find + // a remote plugin reference. This syntax is subject to change. + if _, err := bufmodule.ParseModuleRef(name); err == nil { + return nil, syserror.Newf("remote plugins are not yet supported") + } + // WASM plugins are suffixed with .wasm. Otherwise, it's a binary. + if strings.HasSuffix(name, ".wasm") { + return newLocalWASMPluginConfig( + name, + options, + ) + } + } return newLocalPluginConfig( strings.Join(path, " "), options, @@ -114,6 +147,17 @@ func newLocalPluginConfig( }, nil } +func newLocalWASMPluginConfig( + name string, + options map[string]any, +) (*pluginConfig, error) { + return &pluginConfig{ + pluginConfigType: PluginConfigTypeLocalWASM, + name: name, + options: options, + }, nil +} + func (p *pluginConfig) Type() PluginConfigType { return p.pluginConfigType } diff --git a/private/bufpkg/bufwasm/bufwasm.go b/private/bufpkg/bufwasm/bufwasm.go new file mode 100644 index 0000000000..0aa2302efd --- /dev/null +++ b/private/bufpkg/bufwasm/bufwasm.go @@ -0,0 +1,81 @@ +// 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 bufwasm provides a WASM runtime for plugins. +package bufwasm + +import ( + "context" + + "pluginrpc.com/pluginrpc" +) + +// Plugin is a WASM module. +// +// It is safe to use this plugin concurrently. Ensure that you call [Release] +// when you are done with the plugin. +// +// Memory is limited by the runtime. To restrict CPU usage, cancel the context. +type Plugin interface { + pluginrpc.Runner + // Name returns the name of the plugin. + Name() string + // Release releases all resources held by the plugin. + Release(ctx context.Context) error +} + +// Runtime is a WASM runtime. +// +// It is safe to use this runtime concurrently. Ensure that you call [Release] +// when you are done with the runtime. All plugins created by this runtime will +// be invalidated when [Release] is called. +type Runtime interface { + // Compile compiles the given module into a [Plugin]. + // + // The plugin is not validated to conform to the pluginrpc protocol. + Compile(ctx context.Context, pluginName string, pluginWASM []byte) (Plugin, error) + // Release releases all resources held by the runtime. + Release(ctx context.Context) error +} + +// NewRuntime creates a new WASM runtime. +func NewRuntime(ctx context.Context, options ...RuntimeOption) (Runtime, error) { + return newRuntime(ctx, options...) +} + +// RuntimeOption is an option for [NewRuntime]. +type RuntimeOption interface { + apply(*runtimeConfig) +} + +// WithMaxMemoryBytes sets the maximum memory size in bytes. +func WithMaxMemoryBytes(maxMemoryBytes uint32) RuntimeOption { + return runtimeOptionFunc(func(cfg *runtimeConfig) { + cfg.maxMemoryBytes = maxMemoryBytes + }) +} + +// WithLocalCacheDir sets the local cache directory. +// +// This option is only safe use in CLI environments. +func WithLocalCacheDir(cacheDir string) RuntimeOption { + return runtimeOptionFunc(func(cfg *runtimeConfig) { + cfg.cacheDir = cacheDir + }) +} + +// NewUnimplementedRuntime returns a new unimplemented Runtime. +func NewUnimplementedRuntime() Runtime { + return unimplementedRuntime{} +} diff --git a/private/bufpkg/bufwasm/plugin.go b/private/bufpkg/bufwasm/plugin.go new file mode 100644 index 0000000000..d289a22ff0 --- /dev/null +++ b/private/bufpkg/bufwasm/plugin.go @@ -0,0 +1,66 @@ +// 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 bufwasm + +import ( + "context" + "fmt" + + "github.com/tetratelabs/wazero" + "pluginrpc.com/pluginrpc" +) + +type plugin struct { + name string + runtime wazero.Runtime + compiledModule wazero.CompiledModule +} + +var _ Plugin = (*plugin)(nil) + +func (p *plugin) Name() string { + return p.name +} + +func (p *plugin) Run(ctx context.Context, env pluginrpc.Env) error { + // Create a new module config with the given environment. + config := wazero.NewModuleConfig(). + WithStdin(env.Stdin). + WithStdout(env.Stdout). + WithStderr(env.Stderr) + + // Instantiate the guest wasm module into the same runtime. + // See: https://github.com/tetratelabs/wazero/issues/985 + mod, err := p.runtime.InstantiateModule( + ctx, + p.compiledModule, + // Use an empty name to allow for multiple instances of the same module. + // See [wazero.ModuleConfig.WithName]. + config.WithName("").WithArgs( + append([]string{p.name}, env.Args...)..., + ), + ) + if err != nil { + return fmt.Errorf("failed to instantiate module: %w", err) + } + if err := mod.Close(ctx); err != nil { + return fmt.Errorf("failed to close module: %w", err) + } + return nil +} + +func (p *plugin) Release(ctx context.Context) error { + return p.compiledModule.Close(ctx) +} diff --git a/private/bufpkg/bufwasm/runtime.go b/private/bufpkg/bufwasm/runtime.go new file mode 100644 index 0000000000..223190a6db --- /dev/null +++ b/private/bufpkg/bufwasm/runtime.go @@ -0,0 +1,129 @@ +// 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 bufwasm + +import ( + "context" + "fmt" + + "github.com/bufbuild/buf/private/pkg/syserror" + "github.com/tetratelabs/wazero" + "github.com/tetratelabs/wazero/api" + "github.com/tetratelabs/wazero/imports/wasi_snapshot_preview1" + "go.uber.org/multierr" +) + +const ( + // defaultMaxMemoryBytes is the maximum memory size in bytes. + defaultMaxMemoryBytes = 1 << 29 // 512 MiB + // wasmPageSize is the page size in bytes. + wasmPageSize = 1 << 16 // 64 KiB +) + +type runtime struct { + runtime wazero.Runtime + cache wazero.CompilationCache +} + +var _ Runtime = (*runtime)(nil) + +func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) { + var cfg runtimeConfig + for _, opt := range options { + opt.apply(&cfg) + } + // Create the runtime config with enforceable limits. + runtimeConfig := wazero.NewRuntimeConfig(). + WithCoreFeatures(api.CoreFeaturesV2). + WithCloseOnContextDone(true). + WithMemoryLimitPages(cfg.getMaxMemoryBytes() / wasmPageSize) + var cache wazero.CompilationCache + if cfg.cacheDir != "" { + var err error + cache, err = wazero.NewCompilationCacheWithDir(cfg.cacheDir) + if err != nil { + return nil, fmt.Errorf("failed to create compilation cache: %w", err) + } + runtimeConfig = runtimeConfig.WithCompilationCache(cache) + } + r := wazero.NewRuntimeWithConfig(ctx, runtimeConfig) + + // Init wasi. + wasi_snapshot_preview1.MustInstantiate(ctx, r) + + return &runtime{ + runtime: r, + cache: cache, + }, nil +} + +func (r *runtime) Compile(ctx context.Context, name string, module []byte) (Plugin, error) { + if name == "" { + // The plugin is required to be named. We cannot use the name + // from the WASM binary as this is not guaranteed to be set and + // may conflict with the provided name. + return nil, fmt.Errorf("name is empty") + } + // Compile the module. This operation is hashed on module by the wazero + // runtime. + compiledModule, err := r.runtime.CompileModule(ctx, module) + if err != nil { + return nil, err + } + return &plugin{ + name: name, + runtime: r.runtime, + compiledModule: compiledModule, + }, nil +} + +func (r *runtime) Release(ctx context.Context) error { + err := r.runtime.Close(ctx) + if r.cache != nil { + err = multierr.Append(err, r.cache.Close(ctx)) + } + return err +} + +type runtimeConfig struct { + maxMemoryBytes uint32 + cacheDir string +} + +func (r *runtimeConfig) getMaxMemoryBytes() uint32 { + if r.maxMemoryBytes == 0 { + return defaultMaxMemoryBytes + } + return r.maxMemoryBytes +} + +type runtimeOptionFunc func(*runtimeConfig) + +func (f runtimeOptionFunc) apply(cfg *runtimeConfig) { + f(cfg) +} + +var _ RuntimeOption = runtimeOptionFunc(nil) + +type unimplementedRuntime struct{} + +var _ Runtime = unimplementedRuntime{} + +func (unimplementedRuntime) Compile(ctx context.Context, name string, module []byte) (Plugin, error) { + return nil, syserror.Newf("not implemented") +} +func (unimplementedRuntime) Release(ctx context.Context) error { + return nil +} diff --git a/private/bufpkg/bufwasm/usage.gen.go b/private/bufpkg/bufwasm/usage.gen.go new file mode 100644 index 0000000000..b0c8ba81b7 --- /dev/null +++ b/private/bufpkg/bufwasm/usage.gen.go @@ -0,0 +1,19 @@ +// 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. + +// Generated. DO NOT EDIT. + +package bufwasm + +import _ "github.com/bufbuild/buf/private/usage" diff --git a/private/usage/usage_unix.go b/private/usage/usage_unix.go index 55ffab50f2..5c3fd8d29a 100644 --- a/private/usage/usage_unix.go +++ b/private/usage/usage_unix.go @@ -12,8 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build aix || darwin || dragonfly || freebsd || (js && wasm) || linux || netbsd || openbsd || solaris -// +build aix darwin dragonfly freebsd js,wasm linux netbsd openbsd solaris +//go:build unix || wasip1 || js package usage diff --git a/private/usage/usage_windows.go b/private/usage/usage_windows.go index 05697bd5c3..e52aa9252f 100644 --- a/private/usage/usage_windows.go +++ b/private/usage/usage_windows.go @@ -13,7 +13,6 @@ // limitations under the License. //go:build windows -// +build windows package usage From a17f1c1c8dce473bee6f2af8acd43fddfe7f13d9 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Mon, 30 Sep 2024 14:12:37 -0400 Subject: [PATCH 02/48] Remove custom look path --- .../bufprotopluginexec/bufprotopluginexec.go | 5 ++ private/bufpkg/bufcheck/runner_provider.go | 58 +++++-------------- 2 files changed, 18 insertions(+), 45 deletions(-) diff --git a/private/buf/bufprotopluginexec/bufprotopluginexec.go b/private/buf/bufprotopluginexec/bufprotopluginexec.go index 9f18a1bae6..70cd026146 100644 --- a/private/buf/bufprotopluginexec/bufprotopluginexec.go +++ b/private/buf/bufprotopluginexec/bufprotopluginexec.go @@ -199,6 +199,11 @@ func newHandlerOptions() *handlerOptions { return &handlerOptions{} } +// FindPluginPath returns the path to the specific plugin specified by pluginName. +func FindPluginPath(pluginName string) (string, error) { + return unsafeLookPath(pluginName) +} + // unsafeLookPath is a wrapper around exec.LookPath that restores the original // pre-Go 1.19 behavior of resolving queries that would use relative PATH // entries. We consider it acceptable for the use case of locating plugins. diff --git a/private/bufpkg/bufcheck/runner_provider.go b/private/bufpkg/bufcheck/runner_provider.go index 7279711432..4e055c400c 100644 --- a/private/bufpkg/bufcheck/runner_provider.go +++ b/private/bufpkg/bufcheck/runner_provider.go @@ -18,10 +18,9 @@ import ( "context" "fmt" "os" - "path/filepath" - "strings" "sync" + "github.com/bufbuild/buf/private/buf/bufprotopluginexec" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufwasm" "github.com/bufbuild/buf/private/pkg/command" @@ -124,9 +123,18 @@ func (r *wasmRunner) loadPluginOnce(ctx context.Context) (bufwasm.Plugin, error) } func (r *wasmRunner) loadPlugin(ctx context.Context) (bufwasm.Plugin, error) { - path, err := lookPath(r.programName) - if err != nil { - return nil, fmt.Errorf("could not find plugin %q in PATH: %v", r.programName, err) + // Find the plugin path. We use the same logic as exec.LookPath, but we do + // not require the file to be executable. So check the local directory + // first before checking the PATH. + var path string + if fileInfo, err := os.Stat(r.programName); err == nil && !fileInfo.IsDir() { + path = r.programName + } else { + var err error + path, err = bufprotopluginexec.FindPluginPath(r.programName) + if err != nil { + return nil, fmt.Errorf("could not find plugin %q in PATH: %v", r.programName, err) + } } wasmModule, err := os.ReadFile(path) if err != nil { @@ -142,43 +150,3 @@ func (r *wasmRunner) loadPlugin(ctx context.Context) (bufwasm.Plugin, error) { // runner is limited to the CLI. return plugin, nil } - -// lookPath looks for a wasm file in the PATH, not only an executable. This -// doesn't use exec.LookPath to avoid requiring an executable bit. -func lookPath(file string) (string, error) { - // First, check in the current directory. - if ok, err := findFile(file); err != nil { - return "", err - } else if ok { - return file, nil - } - // If the file has a path separator, fail early. - if strings.Contains(file, string(os.PathSeparator)) { - return "", os.ErrNotExist - } - path := os.Getenv("PATH") - for _, dir := range filepath.SplitList(path) { - if dir == "" { - // Unix shell semantics: path element "" means "." - dir = "." - } - path := filepath.Join(dir, file) - if ok, err := findFile(path); err != nil { - return "", err - } else if ok { - return path, nil - } - } - return "", os.ErrNotExist -} - -func findFile(file string) (bool, error) { - d, err := os.Stat(file) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - return false, err - } - return !d.Mode().IsDir(), nil -} From 58c9e6fe944dce9fb88e88c1e95882f7b1d47c44 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Mon, 30 Sep 2024 15:55:16 -0400 Subject: [PATCH 03/48] Fix test --- make/buf/all.mk | 7 +------ .../bufprotopluginexec/bufprotopluginexec.go | 5 ----- private/bufpkg/bufcheck/runner_provider.go | 18 ++++++++++++++++-- windows/test.bash | 2 ++ 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/make/buf/all.mk b/make/buf/all.mk index 84512e4bf7..68b72912bc 100644 --- a/make/buf/all.mk +++ b/make/buf/all.mk @@ -26,12 +26,7 @@ GO_TEST_BINS := $(GO_TEST_BINS) \ private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-category \ private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-rule GO_TEST_WASM_BINS := $(GO_TEST_WASM_BINS) \ - private/bufpkg/bufcheck/internal/cmd/buf-plugin-panic \ - private/bufpkg/bufcheck/internal/cmd/buf-plugin-suffix \ - private/bufpkg/bufcheck/internal/cmd/buf-plugin-protovalidate-ext \ - private/bufpkg/bufcheck/internal/cmd/buf-plugin-rpc-ext \ - private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-category \ - private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-rule + private/bufpkg/bufcheck/internal/cmd/buf-plugin-suffix GO_MOD_VERSION := 1.22 DOCKER_BINS := $(DOCKER_BINS) buf FILE_IGNORES := $(FILE_IGNORES) \ diff --git a/private/buf/bufprotopluginexec/bufprotopluginexec.go b/private/buf/bufprotopluginexec/bufprotopluginexec.go index 70cd026146..9f18a1bae6 100644 --- a/private/buf/bufprotopluginexec/bufprotopluginexec.go +++ b/private/buf/bufprotopluginexec/bufprotopluginexec.go @@ -199,11 +199,6 @@ func newHandlerOptions() *handlerOptions { return &handlerOptions{} } -// FindPluginPath returns the path to the specific plugin specified by pluginName. -func FindPluginPath(pluginName string) (string, error) { - return unsafeLookPath(pluginName) -} - // unsafeLookPath is a wrapper around exec.LookPath that restores the original // pre-Go 1.19 behavior of resolving queries that would use relative PATH // entries. We consider it acceptable for the use case of locating plugins. diff --git a/private/bufpkg/bufcheck/runner_provider.go b/private/bufpkg/bufcheck/runner_provider.go index 4e055c400c..fd8508cde3 100644 --- a/private/bufpkg/bufcheck/runner_provider.go +++ b/private/bufpkg/bufcheck/runner_provider.go @@ -16,11 +16,12 @@ package bufcheck import ( "context" + "errors" "fmt" "os" + "os/exec" "sync" - "github.com/bufbuild/buf/private/buf/bufprotopluginexec" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufwasm" "github.com/bufbuild/buf/private/pkg/command" @@ -131,7 +132,7 @@ func (r *wasmRunner) loadPlugin(ctx context.Context) (bufwasm.Plugin, error) { path = r.programName } else { var err error - path, err = bufprotopluginexec.FindPluginPath(r.programName) + path, err = unsafeLookPath(r.programName) if err != nil { return nil, fmt.Errorf("could not find plugin %q in PATH: %v", r.programName, err) } @@ -150,3 +151,16 @@ func (r *wasmRunner) loadPlugin(ctx context.Context) (bufwasm.Plugin, error) { // runner is limited to the CLI. return plugin, nil } + +// unsafeLookPath is a wrapper around exec.LookPath that restores the original +// pre-Go 1.19 behavior of resolving queries that would use relative PATH +// entries. We consider it acceptable for the use case of locating plugins. +// +// https://pkg.go.dev/os/exec#hdr-Executables_in_the_current_directory +func unsafeLookPath(file string) (string, error) { + path, err := exec.LookPath(file) + if errors.Is(err, exec.ErrDot) { + err = nil + } + return path, err +} diff --git a/windows/test.bash b/windows/test.bash index 242d6edb54..425d7a2770 100644 --- a/windows/test.bash +++ b/windows/test.bash @@ -40,4 +40,6 @@ go install ./cmd/buf \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-rpc-ext \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-category \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-rule +GOOS=wasip1 GOARCH=wasm go install -o $(GOBIN)/buf-plugin-suffix.wasm \ + ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-suffix go test ./... From ccdb8012f9323550879460bf73db81e40e6dc75c Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Mon, 30 Sep 2024 16:37:01 -0400 Subject: [PATCH 04/48] Fix test --- windows/test.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/windows/test.bash b/windows/test.bash index 425d7a2770..fad747815a 100644 --- a/windows/test.bash +++ b/windows/test.bash @@ -40,6 +40,6 @@ go install ./cmd/buf \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-rpc-ext \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-category \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-rule -GOOS=wasip1 GOARCH=wasm go install -o $(GOBIN)/buf-plugin-suffix.wasm \ +GOOS=wasip1 GOARCH=wasm go install -o $(go env -json | jq -r .GOBIN)/buf-plugin-suffix.wasm \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-suffix go test ./... From 1fb09feffb498e161aa46d45c098fb9442d8b75e Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Mon, 30 Sep 2024 16:52:18 -0400 Subject: [PATCH 05/48] Fix test --- windows/test.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/windows/test.bash b/windows/test.bash index fad747815a..5291690240 100644 --- a/windows/test.bash +++ b/windows/test.bash @@ -40,6 +40,6 @@ go install ./cmd/buf \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-rpc-ext \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-category \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-rule -GOOS=wasip1 GOARCH=wasm go install -o $(go env -json | jq -r .GOBIN)/buf-plugin-suffix.wasm \ +GOOS=wasip1 GOARCH=wasm go build -o $(go env -json | jq -r .GOBIN)/buf-plugin-suffix.wasm \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-suffix go test ./... From 40848114247eee1bc044baebbd10ca97552adef8 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Mon, 30 Sep 2024 19:40:46 -0400 Subject: [PATCH 06/48] Use Wasm abbr --- private/buf/cmd/buf/command/lint/lint.go | 2 +- private/bufpkg/bufcheck/bufcheck.go | 6 +++--- private/bufpkg/bufcheck/lint_test.go | 6 +++--- private/bufpkg/bufcheck/runner_provider.go | 10 +++++----- private/bufpkg/bufconfig/plugin_config.go | 20 ++++++++++---------- private/bufpkg/bufwasm/bufwasm.go | 10 +++++----- private/bufpkg/bufwasm/runtime.go | 2 +- windows/test.bash | 2 +- 8 files changed, 29 insertions(+), 29 deletions(-) diff --git a/private/buf/cmd/buf/command/lint/lint.go b/private/buf/cmd/buf/command/lint/lint.go index 534c1c9b57..f22fb71cc3 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -153,7 +153,7 @@ func run( tracer, bufcheck.NewRunnerProvider( command.NewRunner(), - bufcheck.RunnerProviderWithWASMRuntime(wasmRuntime), + bufcheck.RunnerProviderWithWasmRuntime(wasmRuntime), ), bufcheck.ClientWithStderr(container.Stderr()), ) diff --git a/private/bufpkg/bufcheck/bufcheck.go b/private/bufpkg/bufcheck/bufcheck.go index 403fb0ed54..f1d1639a72 100644 --- a/private/bufpkg/bufcheck/bufcheck.go +++ b/private/bufpkg/bufcheck/bufcheck.go @@ -182,9 +182,9 @@ func NewRunnerProvider( // RunnerProviderOption is an option for NewRunnerProvider. type RunnerProviderOption func(*runnerProviderOptions) -// RunnerProviderWithWASMRuntime returns a new RunnerProviderOption that -// specifies a WASM runtime. This is required for local WASM plugins. -func RunnerProviderWithWASMRuntime(wasmRuntime bufwasm.Runtime) RunnerProviderOption { +// RunnerProviderWithWasmRuntime returns a new RunnerProviderOption that +// specifies a Wasm runtime. This is required for local Wasm plugins. +func RunnerProviderWithWasmRuntime(wasmRuntime bufwasm.Runtime) RunnerProviderOption { return func(runnerProviderOptions *runnerProviderOptions) { runnerProviderOptions.wasmRuntime = wasmRuntime } diff --git a/private/bufpkg/bufcheck/lint_test.go b/private/bufpkg/bufcheck/lint_test.go index 610fafe406..aaea76c352 100644 --- a/private/bufpkg/bufcheck/lint_test.go +++ b/private/bufpkg/bufcheck/lint_test.go @@ -1220,7 +1220,7 @@ func TestRunLintCustomPlugins(t *testing.T) { ) } -func TestRunLintCustomWASMPlugins(t *testing.T) { +func TestRunLintCustomWasmPlugins(t *testing.T) { t.Parallel() if testing.Short() { t.Skip("skipping test in short mode") @@ -1263,7 +1263,7 @@ func testLintWithOptions( wasmRuntime bool, expectedFileAnnotations ...bufanalysis.FileAnnotation, ) { - ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) // Increased timeout for WASM runtime + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) // Increased timeout for Wasm runtime defer cancel() baseDirPath := filepath.Join("testdata", "lint") @@ -1325,7 +1325,7 @@ func testLintWithOptions( wasmRuntime, err := bufwasm.NewRuntime(ctx) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, wasmRuntime.Release(ctx)) }) - runnerProviderOptions = append(runnerProviderOptions, bufcheck.RunnerProviderWithWASMRuntime(wasmRuntime)) + runnerProviderOptions = append(runnerProviderOptions, bufcheck.RunnerProviderWithWasmRuntime(wasmRuntime)) } client, err := bufcheck.NewClient( zap.NewNop(), diff --git a/private/bufpkg/bufcheck/runner_provider.go b/private/bufpkg/bufcheck/runner_provider.go index fd8508cde3..28bb8cd6e7 100644 --- a/private/bufpkg/bufcheck/runner_provider.go +++ b/private/bufpkg/bufcheck/runner_provider.go @@ -56,11 +56,11 @@ func (r *runnerProvider) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginr path[0], path[1:]..., ), nil - case bufconfig.PluginConfigTypeLocalWASM: + case bufconfig.PluginConfigTypeLocalWasm: if r.wasmRuntime == nil { return nil, syserror.Newf("wasm runtime is required for local wasm plugins") } - return newWASMRunner( + return newWasmRunner( r.wasmRuntime, pluginConfig.Name(), ), nil @@ -79,7 +79,7 @@ func newRunnerProviderOptions() *runnerProviderOptions { } } -// wasmRunner is a runner that loads a WASM plugin. +// wasmRunner is a runner that loads a Wasm plugin. type wasmRunner struct { programName string wasmRuntime bufwasm.Runtime @@ -89,7 +89,7 @@ type wasmRunner struct { pluginErr error } -// newWASMRunner returns a new pluginrpc.Runner for the WASM binary on a +// newWasmRunner returns a new pluginrpc.Runner for the Wasm binary on a // bufwasm.Runtime and program name. This runner is only suitable for use with // short-lived programs, compiled plugins lifetime is tied to the runtime. // @@ -98,7 +98,7 @@ type wasmRunner struct { // directory and file to find the program. This is similar to exec.LookPath // but does not require the file to be executable. This is only safe for use // in the CLI, as it is not safe to use in a server environment. -func newWASMRunner( +func newWasmRunner( runtime bufwasm.Runtime, programName string, ) *wasmRunner { diff --git a/private/bufpkg/bufconfig/plugin_config.go b/private/bufpkg/bufconfig/plugin_config.go index ab94813798..66fc8be9e3 100644 --- a/private/bufpkg/bufconfig/plugin_config.go +++ b/private/bufpkg/bufconfig/plugin_config.go @@ -26,8 +26,8 @@ import ( const ( // PluginConfigTypeLocal is the local plugin config type. PluginConfigTypeLocal PluginConfigType = iota + 1 - // PluginConfigTypeLocalWASM is the local WASM plugin config type. - PluginConfigTypeLocalWASM + // PluginConfigTypeLocalWasm is the local Wasm plugin config type. + PluginConfigTypeLocalWasm ) // PluginConfigType is a generate plugin configuration type. @@ -65,13 +65,13 @@ func NewLocalPluginConfig( ) } -// NewLocalWASMPluginConfig returns a new PluginConfig for a local WASM plugin. -func NewLocalWASMPluginConfig( +// NewLocalWasmPluginConfig returns a new PluginConfig for a local Wasm plugin. +func NewLocalWasmPluginConfig( name string, options map[string]any, path []string, ) (PluginConfig, error) { - return newLocalWASMPluginConfig( + return newLocalWasmPluginConfig( name, options, ) @@ -101,7 +101,7 @@ func newPluginConfigForExternalV2( options[key] = value } // Differentiate between local and remote plugins. - // Local plugins are specified as a path to a binary or a WASM file. + // Local plugins are specified as a path to a binary or a Wasm file. // Remote plugins are specified as a module reference. // Paths with more than one element are assumed to be local plugin commands. // This heuristic is based on the buffetch heuristics for inputs. @@ -116,9 +116,9 @@ func newPluginConfigForExternalV2( if _, err := bufmodule.ParseModuleRef(name); err == nil { return nil, syserror.Newf("remote plugins are not yet supported") } - // WASM plugins are suffixed with .wasm. Otherwise, it's a binary. + // Wasm plugins are suffixed with .wasm. Otherwise, it's a binary. if strings.HasSuffix(name, ".wasm") { - return newLocalWASMPluginConfig( + return newLocalWasmPluginConfig( name, options, ) @@ -147,12 +147,12 @@ func newLocalPluginConfig( }, nil } -func newLocalWASMPluginConfig( +func newLocalWasmPluginConfig( name string, options map[string]any, ) (*pluginConfig, error) { return &pluginConfig{ - pluginConfigType: PluginConfigTypeLocalWASM, + pluginConfigType: PluginConfigTypeLocalWasm, name: name, options: options, }, nil diff --git a/private/bufpkg/bufwasm/bufwasm.go b/private/bufpkg/bufwasm/bufwasm.go index 0aa2302efd..d1e179bfd3 100644 --- a/private/bufpkg/bufwasm/bufwasm.go +++ b/private/bufpkg/bufwasm/bufwasm.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package bufwasm provides a WASM runtime for plugins. +// Package bufwasm provides a Wasm runtime for plugins. package bufwasm import ( @@ -21,7 +21,7 @@ import ( "pluginrpc.com/pluginrpc" ) -// Plugin is a WASM module. +// Plugin is a Wasm module. // // It is safe to use this plugin concurrently. Ensure that you call [Release] // when you are done with the plugin. @@ -35,7 +35,7 @@ type Plugin interface { Release(ctx context.Context) error } -// Runtime is a WASM runtime. +// Runtime is a Wasm runtime. // // It is safe to use this runtime concurrently. Ensure that you call [Release] // when you are done with the runtime. All plugins created by this runtime will @@ -44,12 +44,12 @@ type Runtime interface { // Compile compiles the given module into a [Plugin]. // // The plugin is not validated to conform to the pluginrpc protocol. - Compile(ctx context.Context, pluginName string, pluginWASM []byte) (Plugin, error) + Compile(ctx context.Context, pluginName string, pluginWasm []byte) (Plugin, error) // Release releases all resources held by the runtime. Release(ctx context.Context) error } -// NewRuntime creates a new WASM runtime. +// NewRuntime creates a new Wasm runtime. func NewRuntime(ctx context.Context, options ...RuntimeOption) (Runtime, error) { return newRuntime(ctx, options...) } diff --git a/private/bufpkg/bufwasm/runtime.go b/private/bufpkg/bufwasm/runtime.go index 223190a6db..4ada6f682b 100644 --- a/private/bufpkg/bufwasm/runtime.go +++ b/private/bufpkg/bufwasm/runtime.go @@ -72,7 +72,7 @@ func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) func (r *runtime) Compile(ctx context.Context, name string, module []byte) (Plugin, error) { if name == "" { // The plugin is required to be named. We cannot use the name - // from the WASM binary as this is not guaranteed to be set and + // from the Wasm binary as this is not guaranteed to be set and // may conflict with the provided name. return nil, fmt.Errorf("name is empty") } diff --git a/windows/test.bash b/windows/test.bash index 5291690240..522f81f5b0 100644 --- a/windows/test.bash +++ b/windows/test.bash @@ -40,6 +40,6 @@ go install ./cmd/buf \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-rpc-ext \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-category \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-duplicate-rule -GOOS=wasip1 GOARCH=wasm go build -o $(go env -json | jq -r .GOBIN)/buf-plugin-suffix.wasm \ +GOOS=wasip1 GOARCH=wasm go build -o $(go env -json | jq -r .GOPATH)/bin/buf-plugin-suffix.wasm \ ./private/bufpkg/bufcheck/internal/cmd/buf-plugin-suffix go test ./... From cc424950ffc3863dcd29b6d01dd4e6b068f66990 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 10:14:03 -0400 Subject: [PATCH 07/48] Fix description --- private/bufpkg/bufcheck/runner_provider.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/private/bufpkg/bufcheck/runner_provider.go b/private/bufpkg/bufcheck/runner_provider.go index 28bb8cd6e7..f5d6c9a9de 100644 --- a/private/bufpkg/bufcheck/runner_provider.go +++ b/private/bufpkg/bufcheck/runner_provider.go @@ -94,10 +94,9 @@ type wasmRunner struct { // short-lived programs, compiled plugins lifetime is tied to the runtime. // // The program name should be the name of the program as it appears in the -// plugin config. The runner will call os.GetEnv("PATH") with os.Stat on each -// directory and file to find the program. This is similar to exec.LookPath -// but does not require the file to be executable. This is only safe for use -// in the CLI, as it is not safe to use in a server environment. +// plugin config. The runner will check the current directory and fallback to +// exec.LookPath to find the program in the PATH. This is only safe for use in +// the CLI, as it is not safe to use in a server environment. func newWasmRunner( runtime bufwasm.Runtime, programName string, From 5cd84ca487ee2e33134540bec959baa6cb543ef7 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 14:16:44 -0400 Subject: [PATCH 08/48] Move RunnerProvider to bufpluginrunner pkg --- private/buf/buflsp/buflsp.go | 3 +- private/buf/bufmigrate/migrator.go | 3 +- private/buf/cmd/buf/buf_test.go | 3 +- .../buf/cmd/buf/command/breaking/breaking.go | 19 +++++++- .../buf/command/config/internal/internal.go | 8 +++- private/buf/cmd/buf/command/lint/lint.go | 5 +- .../cmd/buf/command/mod/internal/internal.go | 7 ++- .../cmd/protoc-gen-buf-breaking/breaking.go | 8 +++- private/buf/cmd/protoc-gen-buf-lint/lint.go | 8 +++- private/bufpkg/bufcheck/breaking_test.go | 3 +- private/bufpkg/bufcheck/bufcheck.go | 23 --------- private/bufpkg/bufcheck/lint_test.go | 7 +-- private/bufpkg/bufcheck/multi_client_test.go | 5 +- .../bufpkg/bufpluginrunner/bufpluginrunner.go | 48 +++++++++++++++++++ .../provider.go} | 2 +- private/bufpkg/bufpluginrunner/usage.gen.go | 19 ++++++++ 16 files changed, 130 insertions(+), 41 deletions(-) create mode 100644 private/bufpkg/bufpluginrunner/bufpluginrunner.go rename private/bufpkg/{bufcheck/runner_provider.go => bufpluginrunner/provider.go} (99%) create mode 100644 private/bufpkg/bufpluginrunner/usage.gen.go diff --git a/private/buf/buflsp/buflsp.go b/private/buf/buflsp/buflsp.go index 5865d91bc4..b237432df8 100644 --- a/private/buf/buflsp/buflsp.go +++ b/private/buf/buflsp/buflsp.go @@ -25,6 +25,7 @@ import ( "github.com/bufbuild/buf/private/buf/bufctl" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage" @@ -61,7 +62,7 @@ func Serve( checkClient, err := bufcheck.NewClient( container.Logger(), tracer, - bufcheck.NewRunnerProvider(command.NewRunner()), + bufpluginrunner.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/bufmigrate/migrator.go b/private/buf/bufmigrate/migrator.go index 586ed1c6a3..4ba298fb15 100644 --- a/private/buf/bufmigrate/migrator.go +++ b/private/buf/bufmigrate/migrator.go @@ -26,6 +26,7 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufmodule" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/normalpath" "github.com/bufbuild/buf/private/pkg/slicesext" @@ -712,7 +713,7 @@ func equivalentCheckConfigInV2( ) (bufconfig.CheckConfig, error) { // No need for custom lint/breaking plugins since there's no plugins to migrate from <=v1. // TODO: If we ever need v3, then we will have to deal with this. - client, err := bufcheck.NewClient(logger, tracer, bufcheck.NewRunnerProvider(runner)) + client, err := bufcheck.NewClient(logger, tracer, bufpluginrunner.NewRunnerProvider(runner)) if err != nil { return nil, err } diff --git a/private/buf/cmd/buf/buf_test.go b/private/buf/cmd/buf/buf_test.go index d577800998..e3d4b18706 100644 --- a/private/buf/cmd/buf/buf_test.go +++ b/private/buf/cmd/buf/buf_test.go @@ -35,6 +35,7 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" imagev1 "github.com/bufbuild/buf/private/gen/proto/go/buf/alpha/image/v1" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appcmd/appcmdtesting" @@ -1349,7 +1350,7 @@ func TestCheckLsBreakingRulesFromConfigExceptDeprecated(t *testing.T) { t.Run(version.String(), func(t *testing.T) { t.Parallel() // Do not need any custom lint/breaking plugins here. - client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufcheck.NewRunnerProvider(command.NewRunner())) + client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufpluginrunner.NewRunnerProvider(command.NewRunner())) require.NoError(t, err) allRules, err := client.AllRules(context.Background(), check.RuleTypeBreaking, version) require.NoError(t, err) diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index 007e860b77..2dd0d4ae12 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -25,6 +25,8 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" + "github.com/bufbuild/buf/private/bufpkg/bufwasm" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" @@ -32,6 +34,7 @@ import ( "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/bufbuild/buf/private/pkg/tracing" "github.com/spf13/pflag" + "go.uber.org/multierr" ) const ( @@ -145,7 +148,7 @@ func run( ctx context.Context, container appext.Container, flags *flags, -) error { +) (retErr error) { if err := bufcli.ValidateRequiredFlag(againstFlagName, flags.Against); err != nil { return err } @@ -206,13 +209,25 @@ func run( len(againstImageWithConfigs), ) } + pluginCacheDir, err := bufcli.CreatePluginCacheDir(container) + if err != nil { + return err + } + wasmRuntime, err := bufwasm.NewRuntime(ctx, bufwasm.WithLocalCacheDir(pluginCacheDir)) + if err != nil { + return err + } + defer func() { retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) }() tracer := tracing.NewTracer(container.Tracer()) var allFileAnnotations []bufanalysis.FileAnnotation for i, imageWithConfig := range imageWithConfigs { client, err := bufcheck.NewClient( container.Logger(), tracer, - bufcheck.NewRunnerProvider(command.NewRunner()), + bufpluginrunner.NewRunnerProvider( + command.NewRunner(), + bufpluginrunner.RunnerProviderWithWasmRuntime(wasmRuntime), + ), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index 784f77717b..f6e911c5c7 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -24,6 +24,7 @@ import ( "github.com/bufbuild/buf/private/buf/bufcli" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufconfig" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" @@ -185,7 +186,12 @@ func lsRun( } } tracer := tracing.NewTracer(container.Tracer()) - client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr())) + client, err := bufcheck.NewClient( + container.Logger(), + tracer, + bufpluginrunner.NewRunnerProvider(command.NewRunner()), + bufcheck.ClientWithStderr(container.Stderr()), + ) if err != nil { return err } diff --git a/private/buf/cmd/buf/command/lint/lint.go b/private/buf/cmd/buf/command/lint/lint.go index f22fb71cc3..755aef5e7f 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -23,6 +23,7 @@ import ( "github.com/bufbuild/buf/private/buf/bufctl" "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" "github.com/bufbuild/buf/private/bufpkg/bufwasm" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" @@ -151,9 +152,9 @@ func run( client, err := bufcheck.NewClient( container.Logger(), tracer, - bufcheck.NewRunnerProvider( + bufpluginrunner.NewRunnerProvider( command.NewRunner(), - bufcheck.RunnerProviderWithWasmRuntime(wasmRuntime), + bufpluginrunner.RunnerProviderWithWasmRuntime(wasmRuntime), ), bufcheck.ClientWithStderr(container.Stderr()), ) diff --git a/private/buf/cmd/buf/command/mod/internal/internal.go b/private/buf/cmd/buf/command/mod/internal/internal.go index 577a5c9e43..3fd5ed7e5a 100644 --- a/private/buf/cmd/buf/command/mod/internal/internal.go +++ b/private/buf/cmd/buf/command/mod/internal/internal.go @@ -24,6 +24,7 @@ import ( "github.com/bufbuild/buf/private/buf/bufcli" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufconfig" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" @@ -175,7 +176,11 @@ func lsRun( } // BufYAMLFiles <=v1 never had plugins. tracer := tracing.NewTracer(container.Tracer()) - client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(container.Stderr())) + client, err := bufcheck.NewClient( + container.Logger(), tracer, + bufpluginrunner.NewRunnerProvider(command.NewRunner()), + bufcheck.ClientWithStderr(container.Stderr()), + ) if err != nil { return err } diff --git a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go index ec6f1d4bdf..9c0da8c46a 100644 --- a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go +++ b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go @@ -28,6 +28,7 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/encoding" "github.com/bufbuild/buf/private/pkg/protodescriptor" @@ -125,7 +126,12 @@ func handle( } // The protoc plugins do not support custom lint/breaking change plugins for now. tracer := tracing.NewTracer(container.Tracer()) - client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(pluginEnv.Stderr)) + client, err := bufcheck.NewClient( + container.Logger(), + tracer, + bufpluginrunner.NewRunnerProvider(command.NewRunner()), + bufcheck.ClientWithStderr(pluginEnv.Stderr), + ) if err != nil { return err } diff --git a/private/buf/cmd/protoc-gen-buf-lint/lint.go b/private/buf/cmd/protoc-gen-buf-lint/lint.go index 8f86b0ba47..0a196471a7 100644 --- a/private/buf/cmd/protoc-gen-buf-lint/lint.go +++ b/private/buf/cmd/protoc-gen-buf-lint/lint.go @@ -27,6 +27,7 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/encoding" "github.com/bufbuild/buf/private/pkg/protodescriptor" @@ -100,7 +101,12 @@ func handle( } // The protoc plugins do not support custom lint/breaking change plugins for now. tracer := tracing.NewTracer(container.Tracer()) - client, err := bufcheck.NewClient(container.Logger(), tracer, bufcheck.NewRunnerProvider(command.NewRunner()), bufcheck.ClientWithStderr(pluginEnv.Stderr)) + client, err := bufcheck.NewClient( + container.Logger(), + tracer, + bufpluginrunner.NewRunnerProvider(command.NewRunner()), + bufcheck.ClientWithStderr(pluginEnv.Stderr), + ) if err != nil { return err } diff --git a/private/bufpkg/bufcheck/breaking_test.go b/private/bufpkg/bufcheck/breaking_test.go index 53674249bd..4c9de52a68 100644 --- a/private/bufpkg/bufcheck/breaking_test.go +++ b/private/bufpkg/bufcheck/breaking_test.go @@ -30,6 +30,7 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/bufpkg/bufmodule" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/tracing" @@ -1347,7 +1348,7 @@ func testBreaking( require.NoError(t, err) breakingConfig := workspace.GetBreakingConfigForOpaqueID(opaqueID) require.NotNil(t, breakingConfig) - client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufcheck.NewRunnerProvider(command.NewRunner())) + client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufpluginrunner.NewRunnerProvider(command.NewRunner())) require.NoError(t, err) err = client.Breaking( ctx, diff --git a/private/bufpkg/bufcheck/bufcheck.go b/private/bufpkg/bufcheck/bufcheck.go index f1d1639a72..91aaf0cc89 100644 --- a/private/bufpkg/bufcheck/bufcheck.go +++ b/private/bufpkg/bufcheck/bufcheck.go @@ -21,8 +21,6 @@ import ( "buf.build/go/bufplugin/check" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufimage" - "github.com/bufbuild/buf/private/bufpkg/bufwasm" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/bufbuild/buf/private/pkg/tracing" @@ -169,27 +167,6 @@ func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (plug return r(pluginConfig) } -// NewRunnerProvider returns a new RunnerProvider for the command.Runner. -// -// This implementation should only be used for local applications. -func NewRunnerProvider( - delegate command.Runner, - options ...RunnerProviderOption, -) RunnerProvider { - return newRunnerProvider(delegate, options...) -} - -// RunnerProviderOption is an option for NewRunnerProvider. -type RunnerProviderOption func(*runnerProviderOptions) - -// RunnerProviderWithWasmRuntime returns a new RunnerProviderOption that -// specifies a Wasm runtime. This is required for local Wasm plugins. -func RunnerProviderWithWasmRuntime(wasmRuntime bufwasm.Runtime) RunnerProviderOption { - return func(runnerProviderOptions *runnerProviderOptions) { - runnerProviderOptions.wasmRuntime = wasmRuntime - } -} - // NewClient returns a new Client. func NewClient( logger *zap.Logger, diff --git a/private/bufpkg/bufcheck/lint_test.go b/private/bufpkg/bufcheck/lint_test.go index aaea76c352..d071f8ac14 100644 --- a/private/bufpkg/bufcheck/lint_test.go +++ b/private/bufpkg/bufcheck/lint_test.go @@ -27,6 +27,7 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/bufpkg/bufmodule" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" "github.com/bufbuild/buf/private/bufpkg/bufwasm" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage/storageos" @@ -1320,17 +1321,17 @@ func testLintWithOptions( lintConfig := workspace.GetLintConfigForOpaqueID(opaqueID) require.NotNil(t, lintConfig) - var runnerProviderOptions []bufcheck.RunnerProviderOption + var runnerProviderOptions []bufpluginrunner.RunnerProviderOption if wasmRuntime { wasmRuntime, err := bufwasm.NewRuntime(ctx) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, wasmRuntime.Release(ctx)) }) - runnerProviderOptions = append(runnerProviderOptions, bufcheck.RunnerProviderWithWasmRuntime(wasmRuntime)) + runnerProviderOptions = append(runnerProviderOptions, bufpluginrunner.RunnerProviderWithWasmRuntime(wasmRuntime)) } client, err := bufcheck.NewClient( zap.NewNop(), tracing.NopTracer, - bufcheck.NewRunnerProvider(command.NewRunner(), runnerProviderOptions...), + bufpluginrunner.NewRunnerProvider(command.NewRunner(), runnerProviderOptions...), ) require.NoError(t, err) err = client.Lint( diff --git a/private/bufpkg/bufcheck/multi_client_test.go b/private/bufpkg/bufcheck/multi_client_test.go index e42195d25c..76549f5d00 100644 --- a/private/bufpkg/bufcheck/multi_client_test.go +++ b/private/bufpkg/bufcheck/multi_client_test.go @@ -24,6 +24,7 @@ import ( "buf.build/go/bufplugin/check/checkutil" "buf.build/go/bufplugin/option" "github.com/bufbuild/buf/private/bufpkg/bufconfig" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/stringutil" @@ -185,7 +186,7 @@ func TestMultiClientCannotHaveOverlappingRulesWithBuiltIn(t *testing.T) { client, err := newClient( zaptest.NewLogger(t), tracing.NopTracer, - NewRunnerProvider(command.NewRunner()), + bufpluginrunner.NewRunnerProvider(command.NewRunner()), ) require.NoError(t, err) duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig( @@ -279,7 +280,7 @@ func TestMultiClientCannotHaveOverlappingCategoriesWithBuiltIn(t *testing.T) { client, err := newClient( zaptest.NewLogger(t), tracing.NopTracer, - NewRunnerProvider(command.NewRunner()), + bufpluginrunner.NewRunnerProvider(command.NewRunner()), ) require.NoError(t, err) duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig( diff --git a/private/bufpkg/bufpluginrunner/bufpluginrunner.go b/private/bufpkg/bufpluginrunner/bufpluginrunner.go new file mode 100644 index 0000000000..5c834015f1 --- /dev/null +++ b/private/bufpkg/bufpluginrunner/bufpluginrunner.go @@ -0,0 +1,48 @@ +// 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 bufpluginrunner + +import ( + "github.com/bufbuild/buf/private/bufpkg/bufconfig" + "github.com/bufbuild/buf/private/bufpkg/bufwasm" + "github.com/bufbuild/buf/private/pkg/command" + "pluginrpc.com/pluginrpc" +) + +// RunnerProvider provides pluginrpc.Runners for a given plugin config. +type RunnerProvider interface { + NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) +} + +// NewRunnerProvider returns a new RunnerProvider for the command.Runner. +// +// This implementation should only be used for local applications. +func NewRunnerProvider( + delegate command.Runner, + options ...RunnerProviderOption, +) RunnerProvider { + return newRunnerProvider(delegate, options...) +} + +// RunnerProviderOption is an option for NewRunnerProvider. +type RunnerProviderOption func(*runnerProviderOptions) + +// RunnerProviderWithWasmRuntime returns a new RunnerProviderOption that +// specifies a Wasm runtime. This is required for local Wasm plugins. +func RunnerProviderWithWasmRuntime(wasmRuntime bufwasm.Runtime) RunnerProviderOption { + return func(runnerProviderOptions *runnerProviderOptions) { + runnerProviderOptions.wasmRuntime = wasmRuntime + } +} diff --git a/private/bufpkg/bufcheck/runner_provider.go b/private/bufpkg/bufpluginrunner/provider.go similarity index 99% rename from private/bufpkg/bufcheck/runner_provider.go rename to private/bufpkg/bufpluginrunner/provider.go index f5d6c9a9de..b71f83d51b 100644 --- a/private/bufpkg/bufcheck/runner_provider.go +++ b/private/bufpkg/bufpluginrunner/provider.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package bufcheck +package bufpluginrunner import ( "context" diff --git a/private/bufpkg/bufpluginrunner/usage.gen.go b/private/bufpkg/bufpluginrunner/usage.gen.go new file mode 100644 index 0000000000..e1bfe596f7 --- /dev/null +++ b/private/bufpkg/bufpluginrunner/usage.gen.go @@ -0,0 +1,19 @@ +// 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. + +// Generated. DO NOT EDIT. + +package bufpluginrunner + +import _ "github.com/bufbuild/buf/private/usage" From 0b4321b1bf5840890c3a630e289df83e52dcabcb Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 17:49:39 -0400 Subject: [PATCH 09/48] Rename Plugin to CompiledModule --- private/bufpkg/bufpluginrunner/provider.go | 26 +++++++++++----------- private/bufpkg/bufwasm/bufwasm.go | 20 ++++++++--------- private/bufpkg/bufwasm/plugin.go | 16 ++++++------- private/bufpkg/bufwasm/runtime.go | 18 +++++++-------- 4 files changed, 39 insertions(+), 41 deletions(-) diff --git a/private/bufpkg/bufpluginrunner/provider.go b/private/bufpkg/bufpluginrunner/provider.go index b71f83d51b..8e5bb022ba 100644 --- a/private/bufpkg/bufpluginrunner/provider.go +++ b/private/bufpkg/bufpluginrunner/provider.go @@ -83,10 +83,10 @@ func newRunnerProviderOptions() *runnerProviderOptions { type wasmRunner struct { programName string wasmRuntime bufwasm.Runtime - // Once protects plugin and pluginErr. - once sync.Once - plugin bufwasm.Plugin - pluginErr error + // Once protects compiledModule and compiledModuleErr. + once sync.Once + compiledModule bufwasm.CompiledModule + compiledModuleErr error } // newWasmRunner returns a new pluginrpc.Runner for the Wasm binary on a @@ -108,21 +108,21 @@ func newWasmRunner( } func (r *wasmRunner) Run(ctx context.Context, env pluginrpc.Env) (retErr error) { - plugin, err := r.loadPluginOnce(ctx) + compiledModule, err := r.loadCompiledModuleOnce(ctx) if err != nil { return err } - return plugin.Run(ctx, env) + return compiledModule.Run(ctx, env) } -func (r *wasmRunner) loadPluginOnce(ctx context.Context) (bufwasm.Plugin, error) { +func (r *wasmRunner) loadCompiledModuleOnce(ctx context.Context) (bufwasm.CompiledModule, error) { r.once.Do(func() { - r.plugin, r.pluginErr = r.loadPlugin(ctx) + r.compiledModule, r.compiledModuleErr = r.loadCompiledModule(ctx) }) - return r.plugin, r.pluginErr + return r.compiledModule, r.compiledModuleErr } -func (r *wasmRunner) loadPlugin(ctx context.Context) (bufwasm.Plugin, error) { +func (r *wasmRunner) loadCompiledModule(ctx context.Context) (bufwasm.CompiledModule, error) { // Find the plugin path. We use the same logic as exec.LookPath, but we do // not require the file to be executable. So check the local directory // first before checking the PATH. @@ -140,15 +140,15 @@ func (r *wasmRunner) loadPlugin(ctx context.Context) (bufwasm.Plugin, error) { if err != nil { return nil, err } - // Compile and run, releasing the plugin at the end. - plugin, err := r.wasmRuntime.Compile(ctx, r.programName, wasmModule) + // Compile and run, releasing the compiledModule at the end. + compiledModule, err := r.wasmRuntime.Compile(ctx, r.programName, wasmModule) if err != nil { return nil, err } // This plugin is never released, so subsequent calls to this function // will benefit from the cached plugin. This is only safe as the // runner is limited to the CLI. - return plugin, nil + return compiledModule, nil } // unsafeLookPath is a wrapper around exec.LookPath that restores the original diff --git a/private/bufpkg/bufwasm/bufwasm.go b/private/bufpkg/bufwasm/bufwasm.go index d1e179bfd3..ec59d0c2da 100644 --- a/private/bufpkg/bufwasm/bufwasm.go +++ b/private/bufpkg/bufwasm/bufwasm.go @@ -21,17 +21,17 @@ import ( "pluginrpc.com/pluginrpc" ) -// Plugin is a Wasm module. +// CompiledModule is a Wasm module ready to be run. // -// It is safe to use this plugin concurrently. Ensure that you call [Release] -// when you are done with the plugin. +// It is safe to use this module concurrently. Ensure that you call [Release] +// to free resources associated with the module. // // Memory is limited by the runtime. To restrict CPU usage, cancel the context. -type Plugin interface { +type CompiledModule interface { pluginrpc.Runner - // Name returns the name of the plugin. - Name() string - // Release releases all resources held by the plugin. + // PluginName returns the name of the plugin. + PluginName() string + // Release releases all resources held by the compiled module. Release(ctx context.Context) error } @@ -41,10 +41,8 @@ type Plugin interface { // when you are done with the runtime. All plugins created by this runtime will // be invalidated when [Release] is called. type Runtime interface { - // Compile compiles the given module into a [Plugin]. - // - // The plugin is not validated to conform to the pluginrpc protocol. - Compile(ctx context.Context, pluginName string, pluginWasm []byte) (Plugin, error) + // Compile compiles the given Wasm module bytes into a [CompiledModule]. + Compile(ctx context.Context, pluginName string, pluginWasm []byte) (CompiledModule, error) // Release releases all resources held by the runtime. Release(ctx context.Context) error } diff --git a/private/bufpkg/bufwasm/plugin.go b/private/bufpkg/bufwasm/plugin.go index d289a22ff0..47fb5736c4 100644 --- a/private/bufpkg/bufwasm/plugin.go +++ b/private/bufpkg/bufwasm/plugin.go @@ -22,19 +22,19 @@ import ( "pluginrpc.com/pluginrpc" ) -type plugin struct { - name string +type compiledModule struct { + pluginName string runtime wazero.Runtime compiledModule wazero.CompiledModule } -var _ Plugin = (*plugin)(nil) +var _ CompiledModule = (*compiledModule)(nil) -func (p *plugin) Name() string { - return p.name +func (p *compiledModule) PluginName() string { + return p.pluginName } -func (p *plugin) Run(ctx context.Context, env pluginrpc.Env) error { +func (p *compiledModule) Run(ctx context.Context, env pluginrpc.Env) error { // Create a new module config with the given environment. config := wazero.NewModuleConfig(). WithStdin(env.Stdin). @@ -49,7 +49,7 @@ func (p *plugin) Run(ctx context.Context, env pluginrpc.Env) error { // Use an empty name to allow for multiple instances of the same module. // See [wazero.ModuleConfig.WithName]. config.WithName("").WithArgs( - append([]string{p.name}, env.Args...)..., + append([]string{p.pluginName}, env.Args...)..., ), ) if err != nil { @@ -61,6 +61,6 @@ func (p *plugin) Run(ctx context.Context, env pluginrpc.Env) error { return nil } -func (p *plugin) Release(ctx context.Context) error { +func (p *compiledModule) Release(ctx context.Context) error { return p.compiledModule.Close(ctx) } diff --git a/private/bufpkg/bufwasm/runtime.go b/private/bufpkg/bufwasm/runtime.go index 4ada6f682b..6e6f588f2b 100644 --- a/private/bufpkg/bufwasm/runtime.go +++ b/private/bufpkg/bufwasm/runtime.go @@ -69,23 +69,23 @@ func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) }, nil } -func (r *runtime) Compile(ctx context.Context, name string, module []byte) (Plugin, error) { - if name == "" { +func (r *runtime) Compile(ctx context.Context, pluginName string, pluginWasm []byte) (CompiledModule, error) { + if pluginName == "" { // The plugin is required to be named. We cannot use the name // from the Wasm binary as this is not guaranteed to be set and // may conflict with the provided name. return nil, fmt.Errorf("name is empty") } - // Compile the module. This operation is hashed on module by the wazero - // runtime. - compiledModule, err := r.runtime.CompileModule(ctx, module) + // Compile the WebAssembly. This operation is hashed on pluginWasm + // bytes by the wazero runtime. + compiledModulePlugin, err := r.runtime.CompileModule(ctx, pluginWasm) if err != nil { return nil, err } - return &plugin{ - name: name, + return &compiledModule{ + pluginName: pluginName, runtime: r.runtime, - compiledModule: compiledModule, + compiledModule: compiledModulePlugin, }, nil } @@ -121,7 +121,7 @@ type unimplementedRuntime struct{} var _ Runtime = unimplementedRuntime{} -func (unimplementedRuntime) Compile(ctx context.Context, name string, module []byte) (Plugin, error) { +func (unimplementedRuntime) Compile(ctx context.Context, name string, moduleBytes []byte) (CompiledModule, error) { return nil, syserror.Newf("not implemented") } func (unimplementedRuntime) Release(ctx context.Context) error { From ce2c42a26450b98c340846fdc0cd9cd3e293c649 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 20:40:50 -0400 Subject: [PATCH 10/48] Use tracer --- private/bufpkg/bufcheck/client.go | 2 +- private/bufpkg/bufcheck/multi_client.go | 25 +++++++++++++------- private/bufpkg/bufcheck/multi_client_test.go | 3 +++ 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/private/bufpkg/bufcheck/client.go b/private/bufpkg/bufcheck/client.go index edb0d5fd4b..cb05a4b7be 100644 --- a/private/bufpkg/bufcheck/client.go +++ b/private/bufpkg/bufcheck/client.go @@ -340,7 +340,7 @@ func (c *client) getMultiClient( newCheckClientSpec(pluginConfig.Name(), checkClient, options), ) } - return newMultiClient(c.logger, checkClientSpecs), nil + return newMultiClient(c.logger, c.tracer, checkClientSpecs), nil } func annotationsToFilteredFileAnnotationSetOrError( diff --git a/private/bufpkg/bufcheck/multi_client.go b/private/bufpkg/bufcheck/multi_client.go index 83b6cdaa4d..0dbaba77b5 100644 --- a/private/bufpkg/bufcheck/multi_client.go +++ b/private/bufpkg/bufcheck/multi_client.go @@ -20,22 +20,25 @@ import ( "sort" "strings" "sync" - "time" "buf.build/go/bufplugin/check" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/thread" + "github.com/bufbuild/buf/private/pkg/tracing" + "go.opentelemetry.io/otel/attribute" "go.uber.org/zap" ) type multiClient struct { logger *zap.Logger + tracer tracing.Tracer checkClientSpecs []*checkClientSpec } -func newMultiClient(logger *zap.Logger, checkClientSpecs []*checkClientSpec) *multiClient { +func newMultiClient(logger *zap.Logger, tracer tracing.Tracer, checkClientSpecs []*checkClientSpec) *multiClient { return &multiClient{ logger: logger, + tracer: tracer, checkClientSpecs: checkClientSpecs, } } @@ -94,8 +97,15 @@ func (c *multiClient) Check(ctx context.Context, request check.Request) ([]*anno } jobs = append( jobs, - func(ctx context.Context) error { - start := time.Now() + func(ctx context.Context) (retErr error) { + ctx, span := c.tracer.Start( + ctx, + tracing.WithErr(&retErr), + tracing.WithAttributes( + attribute.Key("plugin").String(delegate.PluginName), + ), + ) + defer span.End() delegateResponse, err := delegate.Client.Check(ctx, delegateRequest) if err != nil { if delegate.PluginName == "" { @@ -112,7 +122,6 @@ func (c *multiClient) Check(ctx context.Context, request check.Request) ([]*anno lock.Lock() allAnnotations = append(allAnnotations, annotations...) lock.Unlock() - c.logger.Debug("checked delegate client", zap.String("pluginName", delegate.PluginName), zap.Duration("duration", time.Since(start))) return nil }, ) @@ -151,10 +160,11 @@ func (c *multiClient) getRulesCategoriesAndChunkedIDs(ctx context.Context) ( retChunkedCategoryIDs [][]string, retErr error, ) { + ctx, span := c.tracer.Start(ctx, tracing.WithErr(&retErr)) + defer span.End() var rules []Rule chunkedRuleIDs := make([][]string, len(c.checkClientSpecs)) for i, delegate := range c.checkClientSpecs { - start := time.Now() delegateCheckRules, err := delegate.Client.ListRules(ctx) if err != nil { if delegate.PluginName == "" { @@ -169,13 +179,11 @@ func (c *multiClient) getRulesCategoriesAndChunkedIDs(ctx context.Context) ( rules = append(rules, delegateRules...) // Already sorted. chunkedRuleIDs[i] = slicesext.Map(delegateRules, Rule.ID) - c.logger.Debug("list rules delegate client", zap.String("pluginName", delegate.PluginName), zap.Duration("duration", time.Since(start))) } var categories []Category chunkedCategoryIDs := make([][]string, len(c.checkClientSpecs)) for i, delegate := range c.checkClientSpecs { - start := time.Now() delegateCheckCategories, err := delegate.Client.ListCategories(ctx) if err != nil { if delegate.PluginName == "" { @@ -190,7 +198,6 @@ func (c *multiClient) getRulesCategoriesAndChunkedIDs(ctx context.Context) ( categories = append(categories, delegateCategories...) // Already sorted. chunkedCategoryIDs[i] = slicesext.Map(delegateCategories, Category.ID) - c.logger.Debug("list categories delegate client", zap.String("pluginName", delegate.PluginName), zap.Duration("duration", time.Since(start))) } if err := validateNoDuplicateRulesOrCategories(rules, categories); err != nil { diff --git a/private/bufpkg/bufcheck/multi_client_test.go b/private/bufpkg/bufcheck/multi_client_test.go index 76549f5d00..f98f5834bc 100644 --- a/private/bufpkg/bufcheck/multi_client_test.go +++ b/private/bufpkg/bufcheck/multi_client_test.go @@ -108,6 +108,7 @@ func testMultiClientSimple(t *testing.T, cacheRules bool) { require.NoError(t, err) multiClient := newMultiClient( zap.NewNop(), + tracing.NopTracer, []*checkClientSpec{ newCheckClientSpec("buf-plugin-field-lower-snake-case", fieldLowerSnakeCaseClient, emptyOptions), newCheckClientSpec("buf-plugin-timestamp-suffix", timestampSuffixClient, emptyOptions), @@ -168,6 +169,7 @@ func TestMultiClientCannotHaveOverlappingRules(t *testing.T) { require.NoError(t, err) multiClient := newMultiClient( zap.NewNop(), + tracing.NopTracer, []*checkClientSpec{ newCheckClientSpec("buf-plugin-field-lower-snake-case", fieldLowerSnakeCaseClient, emptyOptions), newCheckClientSpec("buf-plugin-field-lower-snake-case", fieldLowerSnakeCaseClient, emptyOptions), @@ -262,6 +264,7 @@ func TestMultiClientCannotHaveOverlappingCategories(t *testing.T) { require.NoError(t, err) multiClient := newMultiClient( zap.NewNop(), + tracing.NopTracer, []*checkClientSpec{ newCheckClientSpec("buf-plugin-1", client1, emptyOptions), newCheckClientSpec("buf-plugin-2", client2, emptyOptions), From 8ef1070ce0bd3a2b96b78db74774b639da1faf0d Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 20:41:04 -0400 Subject: [PATCH 11/48] Unimplemented as var --- private/bufpkg/bufpluginrunner/provider.go | 2 +- private/bufpkg/bufwasm/bufwasm.go | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/private/bufpkg/bufpluginrunner/provider.go b/private/bufpkg/bufpluginrunner/provider.go index 8e5bb022ba..916132d27b 100644 --- a/private/bufpkg/bufpluginrunner/provider.go +++ b/private/bufpkg/bufpluginrunner/provider.go @@ -75,7 +75,7 @@ type runnerProviderOptions struct { func newRunnerProviderOptions() *runnerProviderOptions { return &runnerProviderOptions{ - wasmRuntime: bufwasm.NewUnimplementedRuntime(), + wasmRuntime: bufwasm.UnimplementedRuntime, } } diff --git a/private/bufpkg/bufwasm/bufwasm.go b/private/bufpkg/bufwasm/bufwasm.go index ec59d0c2da..20333335a3 100644 --- a/private/bufpkg/bufwasm/bufwasm.go +++ b/private/bufpkg/bufwasm/bufwasm.go @@ -73,7 +73,5 @@ func WithLocalCacheDir(cacheDir string) RuntimeOption { }) } -// NewUnimplementedRuntime returns a new unimplemented Runtime. -func NewUnimplementedRuntime() Runtime { - return unimplementedRuntime{} -} +// UnimplementedRuntime returns a new unimplemented Runtime. +var UnimplementedRuntime = unimplementedRuntime{} From fa269875e42e4ebedb60a6803e205ddeb13003fd Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 21:03:47 -0400 Subject: [PATCH 12/48] Move wasm pkg --- private/{bufpkg/bufwasm => pkg/wasm}/plugin.go | 2 +- private/{bufpkg/bufwasm => pkg/wasm}/runtime.go | 2 +- private/{bufpkg/bufwasm => pkg/wasm}/usage.gen.go | 2 +- private/{bufpkg/bufwasm/bufwasm.go => pkg/wasm/wasm.go} | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) rename private/{bufpkg/bufwasm => pkg/wasm}/plugin.go (99%) rename private/{bufpkg/bufwasm => pkg/wasm}/runtime.go (99%) rename private/{bufpkg/bufwasm => pkg/wasm}/usage.gen.go (97%) rename private/{bufpkg/bufwasm/bufwasm.go => pkg/wasm/wasm.go} (97%) diff --git a/private/bufpkg/bufwasm/plugin.go b/private/pkg/wasm/plugin.go similarity index 99% rename from private/bufpkg/bufwasm/plugin.go rename to private/pkg/wasm/plugin.go index 47fb5736c4..3eec12a1a8 100644 --- a/private/bufpkg/bufwasm/plugin.go +++ b/private/pkg/wasm/plugin.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package bufwasm +package wasm import ( "context" diff --git a/private/bufpkg/bufwasm/runtime.go b/private/pkg/wasm/runtime.go similarity index 99% rename from private/bufpkg/bufwasm/runtime.go rename to private/pkg/wasm/runtime.go index 6e6f588f2b..b54ba491b1 100644 --- a/private/bufpkg/bufwasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package bufwasm +package wasm import ( "context" diff --git a/private/bufpkg/bufwasm/usage.gen.go b/private/pkg/wasm/usage.gen.go similarity index 97% rename from private/bufpkg/bufwasm/usage.gen.go rename to private/pkg/wasm/usage.gen.go index b0c8ba81b7..8fbec91f11 100644 --- a/private/bufpkg/bufwasm/usage.gen.go +++ b/private/pkg/wasm/usage.gen.go @@ -14,6 +14,6 @@ // Generated. DO NOT EDIT. -package bufwasm +package wasm import _ "github.com/bufbuild/buf/private/usage" diff --git a/private/bufpkg/bufwasm/bufwasm.go b/private/pkg/wasm/wasm.go similarity index 97% rename from private/bufpkg/bufwasm/bufwasm.go rename to private/pkg/wasm/wasm.go index 20333335a3..00244b59b1 100644 --- a/private/bufpkg/bufwasm/bufwasm.go +++ b/private/pkg/wasm/wasm.go @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package bufwasm provides a Wasm runtime for plugins. -package bufwasm +// Package wasm provides a Wasm runtime for plugins. +package wasm import ( "context" From 9429c87978b1835b25fdbde9d1c59e31136aac6c Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 21:36:15 -0400 Subject: [PATCH 13/48] move to bufpluginrpcutil --- private/buf/buflsp/buflsp.go | 18 ++++- private/buf/bufmigrate/migrator.go | 7 +- private/buf/cmd/buf/buf_test.go | 5 +- .../buf/cmd/buf/command/breaking/breaking.go | 11 ++- .../buf/command/config/internal/internal.go | 17 ++++- private/buf/cmd/buf/command/lint/lint.go | 14 ++-- .../cmd/buf/command/mod/internal/internal.go | 5 +- .../cmd/protoc-gen-buf-breaking/breaking.go | 5 +- private/buf/cmd/protoc-gen-buf-lint/lint.go | 5 +- private/bufpkg/bufcheck/breaking_test.go | 5 +- private/bufpkg/bufcheck/lint_test.go | 33 +++------ private/bufpkg/bufcheck/multi_client_test.go | 7 +- .../pluginrpcutil.go} | 34 ++++----- .../bufpluginrpcutil}/runner.go | 2 +- .../bufpluginrpcutil/runner_provider.go | 55 +++++++++++++++ .../bufpluginrpcutil}/usage.gen.go | 2 +- .../wasm_runner.go} | 69 +++---------------- private/bufpkg/bufpluginrunner/usage.gen.go | 19 ----- private/pkg/pluginrpcutil/pluginrpcutil.go | 25 ------- 19 files changed, 149 insertions(+), 189 deletions(-) rename private/bufpkg/{bufpluginrunner/bufpluginrunner.go => bufpluginrpcutil/pluginrpcutil.go} (57%) rename private/{pkg/pluginrpcutil => bufpkg/bufpluginrpcutil}/runner.go (98%) create mode 100644 private/bufpkg/bufpluginrpcutil/runner_provider.go rename private/{pkg/pluginrpcutil => bufpkg/bufpluginrpcutil}/usage.gen.go (96%) rename private/bufpkg/{bufpluginrunner/provider.go => bufpluginrpcutil/wasm_runner.go} (63%) delete mode 100644 private/bufpkg/bufpluginrunner/usage.gen.go delete mode 100644 private/pkg/pluginrpcutil/pluginrpcutil.go diff --git a/private/buf/buflsp/buflsp.go b/private/buf/buflsp/buflsp.go index b237432df8..8063c11b98 100644 --- a/private/buf/buflsp/buflsp.go +++ b/private/buf/buflsp/buflsp.go @@ -22,18 +22,21 @@ import ( "fmt" "sync/atomic" + "github.com/bufbuild/buf/private/buf/bufcli" "github.com/bufbuild/buf/private/buf/bufctl" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/tracing" + "github.com/bufbuild/buf/private/pkg/wasm" "go.lsp.dev/jsonrpc2" "go.lsp.dev/protocol" "go.opentelemetry.io/otel/attribute" + "go.uber.org/multierr" "go.uber.org/zap" ) @@ -45,7 +48,7 @@ func Serve( container appext.Container, controller bufctl.Controller, stream jsonrpc2.Stream, -) (jsonrpc2.Conn, error) { +) (_ jsonrpc2.Conn, retErr error) { // The LSP protocol deals with absolute filesystem paths. This requires us to // bypass the bucket API completely, so we create a bucket pointing at the filesystem // root. @@ -57,12 +60,21 @@ func Serve( if err != nil { return nil, err } + pluginCacheDir, err := bufcli.CreatePluginCacheDir(container) + if err != nil { + return nil, err + } + wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(pluginCacheDir)) + if err != nil { + return nil, err + } + defer func() { retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) }() tracer := tracing.NewTracer(container.Tracer()) checkClient, err := bufcheck.NewClient( container.Logger(), tracer, - bufpluginrunner.NewRunnerProvider(command.NewRunner()), + bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasmRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/bufmigrate/migrator.go b/private/buf/bufmigrate/migrator.go index 4ba298fb15..a8b0bbb837 100644 --- a/private/buf/bufmigrate/migrator.go +++ b/private/buf/bufmigrate/migrator.go @@ -26,13 +26,14 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufmodule" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/normalpath" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storagemem" "github.com/bufbuild/buf/private/pkg/tracing" + "github.com/bufbuild/buf/private/pkg/wasm" "github.com/gofrs/uuid/v5" "go.uber.org/multierr" "go.uber.org/zap" @@ -712,8 +713,8 @@ func equivalentCheckConfigInV2( checkConfig bufconfig.CheckConfig, ) (bufconfig.CheckConfig, error) { // No need for custom lint/breaking plugins since there's no plugins to migrate from <=v1. - // TODO: If we ever need v3, then we will have to deal with this. - client, err := bufcheck.NewClient(logger, tracer, bufpluginrunner.NewRunnerProvider(runner)) + // TODO: If we ever need v3, then we will have to deal with this.w + client, err := bufcheck.NewClient(logger, tracer, bufpluginrpcutil.NewRunnerProvider(runner, wasm.UnimplementedRuntime)) if err != nil { return nil, err } diff --git a/private/buf/cmd/buf/buf_test.go b/private/buf/cmd/buf/buf_test.go index e3d4b18706..093011471c 100644 --- a/private/buf/cmd/buf/buf_test.go +++ b/private/buf/cmd/buf/buf_test.go @@ -35,7 +35,7 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufimage" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" imagev1 "github.com/bufbuild/buf/private/gen/proto/go/buf/alpha/image/v1" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appcmd/appcmdtesting" @@ -46,6 +46,7 @@ import ( "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/storage/storagetesting" "github.com/bufbuild/buf/private/pkg/tracing" + "github.com/bufbuild/buf/private/pkg/wasm" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/zap" @@ -1350,7 +1351,7 @@ func TestCheckLsBreakingRulesFromConfigExceptDeprecated(t *testing.T) { t.Run(version.String(), func(t *testing.T) { t.Parallel() // Do not need any custom lint/breaking plugins here. - client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufpluginrunner.NewRunnerProvider(command.NewRunner())) + client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime)) require.NoError(t, err) allRules, err := client.AllRules(context.Background(), check.RuleTypeBreaking, version) require.NoError(t, err) diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index 2dd0d4ae12..9f84e04aa2 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -25,14 +25,14 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" - "github.com/bufbuild/buf/private/bufpkg/bufwasm" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/bufbuild/buf/private/pkg/tracing" + "github.com/bufbuild/buf/private/pkg/wasm" "github.com/spf13/pflag" "go.uber.org/multierr" ) @@ -213,7 +213,7 @@ func run( if err != nil { return err } - wasmRuntime, err := bufwasm.NewRuntime(ctx, bufwasm.WithLocalCacheDir(pluginCacheDir)) + wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(pluginCacheDir)) if err != nil { return err } @@ -224,10 +224,7 @@ func run( client, err := bufcheck.NewClient( container.Logger(), tracer, - bufpluginrunner.NewRunnerProvider( - command.NewRunner(), - bufpluginrunner.RunnerProviderWithWasmRuntime(wasmRuntime), - ), + bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasmRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index f6e911c5c7..3b6d19ac47 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -24,7 +24,7 @@ import ( "github.com/bufbuild/buf/private/buf/bufcli" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufconfig" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" @@ -33,7 +33,9 @@ import ( "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/bufbuild/buf/private/pkg/tracing" + "github.com/bufbuild/buf/private/pkg/wasm" "github.com/spf13/pflag" + "go.uber.org/multierr" ) const ( @@ -150,7 +152,7 @@ func lsRun( flags *flags, commandName string, ruleType check.RuleType, -) error { +) (retErr error) { if flags.ConfiguredOnly { if flags.Version != "" { return appcmd.NewInvalidArgumentErrorf("--%s cannot be specified if --%s is specified", versionFlagName, configFlagName) @@ -185,11 +187,20 @@ func lsRun( return err } } + pluginCacheDir, err := bufcli.CreatePluginCacheDir(container) + if err != nil { + return err + } + wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(pluginCacheDir)) + if err != nil { + return err + } + defer func() { retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) }() tracer := tracing.NewTracer(container.Tracer()) client, err := bufcheck.NewClient( container.Logger(), tracer, - bufpluginrunner.NewRunnerProvider(command.NewRunner()), + bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasmRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/lint/lint.go b/private/buf/cmd/buf/command/lint/lint.go index 755aef5e7f..a054e57933 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -23,13 +23,13 @@ import ( "github.com/bufbuild/buf/private/buf/bufctl" "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" - "github.com/bufbuild/buf/private/bufpkg/bufwasm" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/bufbuild/buf/private/pkg/tracing" + "github.com/bufbuild/buf/private/pkg/wasm" "github.com/spf13/pflag" "go.uber.org/multierr" ) @@ -138,10 +138,7 @@ func run( if err != nil { return err } - wasmRuntime, err := bufwasm.NewRuntime( - ctx, - bufwasm.WithLocalCacheDir(pluginCacheDir), - ) + wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(pluginCacheDir)) if err != nil { return err } @@ -152,10 +149,7 @@ func run( client, err := bufcheck.NewClient( container.Logger(), tracer, - bufpluginrunner.NewRunnerProvider( - command.NewRunner(), - bufpluginrunner.RunnerProviderWithWasmRuntime(wasmRuntime), - ), + bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasmRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/mod/internal/internal.go b/private/buf/cmd/buf/command/mod/internal/internal.go index 3fd5ed7e5a..4a21b1a646 100644 --- a/private/buf/cmd/buf/command/mod/internal/internal.go +++ b/private/buf/cmd/buf/command/mod/internal/internal.go @@ -24,7 +24,7 @@ import ( "github.com/bufbuild/buf/private/buf/bufcli" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufconfig" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" @@ -32,6 +32,7 @@ import ( "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/bufbuild/buf/private/pkg/tracing" + "github.com/bufbuild/buf/private/pkg/wasm" "github.com/spf13/pflag" ) @@ -178,7 +179,7 @@ func lsRun( tracer := tracing.NewTracer(container.Tracer()) client, err := bufcheck.NewClient( container.Logger(), tracer, - bufpluginrunner.NewRunnerProvider(command.NewRunner()), + bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go index 9c0da8c46a..4b5f7a19b5 100644 --- a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go +++ b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go @@ -28,12 +28,13 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/encoding" "github.com/bufbuild/buf/private/pkg/protodescriptor" "github.com/bufbuild/buf/private/pkg/protoencoding" "github.com/bufbuild/buf/private/pkg/tracing" + "github.com/bufbuild/buf/private/pkg/wasm" "github.com/bufbuild/protoplugin" ) @@ -129,7 +130,7 @@ func handle( client, err := bufcheck.NewClient( container.Logger(), tracer, - bufpluginrunner.NewRunnerProvider(command.NewRunner()), + bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), bufcheck.ClientWithStderr(pluginEnv.Stderr), ) if err != nil { diff --git a/private/buf/cmd/protoc-gen-buf-lint/lint.go b/private/buf/cmd/protoc-gen-buf-lint/lint.go index 0a196471a7..a70437b178 100644 --- a/private/buf/cmd/protoc-gen-buf-lint/lint.go +++ b/private/buf/cmd/protoc-gen-buf-lint/lint.go @@ -27,12 +27,13 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/encoding" "github.com/bufbuild/buf/private/pkg/protodescriptor" "github.com/bufbuild/buf/private/pkg/protoencoding" "github.com/bufbuild/buf/private/pkg/tracing" + "github.com/bufbuild/buf/private/pkg/wasm" "github.com/bufbuild/protoplugin" ) @@ -104,7 +105,7 @@ func handle( client, err := bufcheck.NewClient( container.Logger(), tracer, - bufpluginrunner.NewRunnerProvider(command.NewRunner()), + bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), bufcheck.ClientWithStderr(pluginEnv.Stderr), ) if err != nil { diff --git a/private/bufpkg/bufcheck/breaking_test.go b/private/bufpkg/bufcheck/breaking_test.go index 4c9de52a68..65ee05b783 100644 --- a/private/bufpkg/bufcheck/breaking_test.go +++ b/private/bufpkg/bufcheck/breaking_test.go @@ -30,10 +30,11 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/bufpkg/bufmodule" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/tracing" + "github.com/bufbuild/buf/private/pkg/wasm" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/zap" @@ -1348,7 +1349,7 @@ func testBreaking( require.NoError(t, err) breakingConfig := workspace.GetBreakingConfigForOpaqueID(opaqueID) require.NotNil(t, breakingConfig) - client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufpluginrunner.NewRunnerProvider(command.NewRunner())) + client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime)) require.NoError(t, err) err = client.Breaking( ctx, diff --git a/private/bufpkg/bufcheck/lint_test.go b/private/bufpkg/bufcheck/lint_test.go index d071f8ac14..43a95f74b5 100644 --- a/private/bufpkg/bufcheck/lint_test.go +++ b/private/bufpkg/bufcheck/lint_test.go @@ -27,11 +27,11 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/bufpkg/bufmodule" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" - "github.com/bufbuild/buf/private/bufpkg/bufwasm" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/tracing" + "github.com/bufbuild/buf/private/pkg/wasm" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/zap" @@ -513,7 +513,6 @@ func TestRunPackageNoImportCycle(t *testing.T) { require.NoError(t, err) return newImage }, - false, bufanalysistesting.NewFileAnnotation(t, "c1.proto", 5, 1, 5, 19, "PACKAGE_NO_IMPORT_CYCLE"), bufanalysistesting.NewFileAnnotation(t, "d1.proto", 5, 1, 5, 19, "PACKAGE_NO_IMPORT_CYCLE"), ) @@ -596,8 +595,7 @@ func TestRunProtovalidate(t *testing.T) { t, "protovalidate", "buf.testing/lint/protovalidate", - nil, // no image modification - false, // no wasm runtime + nil, bufanalysistesting.NewFileAnnotation(t, "bool.proto", 18, 51, 18, 84, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "bool.proto", 19, 31, 19, 69, "PROTOVALIDATE"), bufanalysistesting.NewFileAnnotation(t, "bool.proto", 20, 50, 20, 88, "PROTOVALIDATE"), @@ -1030,8 +1028,7 @@ func TestRunV2WorkspaceIgnores(t *testing.T) { t, "v2/ignores", "ignores1", - nil, // no image modification - false, // no wasm runtime + nil, bufanalysistesting.NewFileAnnotation(t, "bar1/bar.proto", 6, 9, 6, 15, "FIELD_LOWER_SNAKE_CASE"), bufanalysistesting.NewFileAnnotation(t, "bar1/bar.proto", 9, 9, 9, 12, "MESSAGE_PASCAL_CASE"), bufanalysistesting.NewFileAnnotation(t, "bar1/bar.proto", 13, 6, 13, 9, "ENUM_PASCAL_CASE"), @@ -1055,8 +1052,7 @@ func TestRunV2WorkspaceIgnores(t *testing.T) { t, "v2/ignores", "ignores2", - nil, // no image modification - false, // no wasm runtime + nil, bufanalysistesting.NewFileAnnotation(t, "bar2/bar.proto", 6, 9, 6, 15, "FIELD_LOWER_SNAKE_CASE"), bufanalysistesting.NewFileAnnotation(t, "bar2/bar.proto", 9, 9, 9, 12, "MESSAGE_PASCAL_CASE"), bufanalysistesting.NewFileAnnotation(t, "bar2/bar.proto", 13, 6, 13, 9, "ENUM_PASCAL_CASE"), @@ -1068,8 +1064,7 @@ func TestRunV2WorkspaceIgnores(t *testing.T) { t, "v2/ignores", "ignores3", - nil, // no image modification - false, // no wasm runtime + nil, bufanalysistesting.NewFileAnnotation(t, "bar3/bar.proto", 6, 9, 6, 15, "FIELD_LOWER_SNAKE_CASE"), bufanalysistesting.NewFileAnnotation(t, "bar3/bar2.proto", 6, 9, 6, 15, "FIELD_LOWER_SNAKE_CASE"), bufanalysistesting.NewFileAnnotation(t, "bar3/bar2.proto", 9, 9, 9, 13, "MESSAGE_PASCAL_CASE"), @@ -1230,8 +1225,7 @@ func TestRunLintCustomWasmPlugins(t *testing.T) { t, "custom_wasm_plugins", "", - nil, // no image modification - true, // wasm runtime + nil, bufanalysistesting.NewFileAnnotationNoLocation(t, "a.proto", "PACKAGE_DEFINED"), bufanalysistesting.NewFileAnnotation(t, "a.proto", 8, 1, 10, 2, "SERVICE_BANNED_SUFFIXES"), bufanalysistesting.NewFileAnnotation(t, "b.proto", 6, 3, 6, 66, "RPC_BANNED_SUFFIXES"), @@ -1250,7 +1244,6 @@ func testLint( relDirPath, "", nil, - false, expectedFileAnnotations..., ) } @@ -1261,7 +1254,6 @@ func testLintWithOptions( // only set if in workspace moduleFullNameString string, imageModifier func(bufimage.Image) bufimage.Image, - wasmRuntime bool, expectedFileAnnotations ...bufanalysis.FileAnnotation, ) { ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) // Increased timeout for Wasm runtime @@ -1321,17 +1313,12 @@ func testLintWithOptions( lintConfig := workspace.GetLintConfigForOpaqueID(opaqueID) require.NotNil(t, lintConfig) - var runnerProviderOptions []bufpluginrunner.RunnerProviderOption - if wasmRuntime { - wasmRuntime, err := bufwasm.NewRuntime(ctx) - require.NoError(t, err) - t.Cleanup(func() { assert.NoError(t, wasmRuntime.Release(ctx)) }) - runnerProviderOptions = append(runnerProviderOptions, bufpluginrunner.RunnerProviderWithWasmRuntime(wasmRuntime)) - } + wasmRuntime, err := wasm.NewRuntime(ctx) + require.NoError(t, err) client, err := bufcheck.NewClient( zap.NewNop(), tracing.NopTracer, - bufpluginrunner.NewRunnerProvider(command.NewRunner(), runnerProviderOptions...), + bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasmRuntime), ) require.NoError(t, err) err = client.Lint( diff --git a/private/bufpkg/bufcheck/multi_client_test.go b/private/bufpkg/bufcheck/multi_client_test.go index f98f5834bc..2e4f9535d7 100644 --- a/private/bufpkg/bufcheck/multi_client_test.go +++ b/private/bufpkg/bufcheck/multi_client_test.go @@ -24,11 +24,12 @@ import ( "buf.build/go/bufplugin/check/checkutil" "buf.build/go/bufplugin/option" "github.com/bufbuild/buf/private/bufpkg/bufconfig" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrunner" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/stringutil" "github.com/bufbuild/buf/private/pkg/tracing" + "github.com/bufbuild/buf/private/pkg/wasm" "github.com/stretchr/testify/require" "go.uber.org/zap" "go.uber.org/zap/zaptest" @@ -188,7 +189,7 @@ func TestMultiClientCannotHaveOverlappingRulesWithBuiltIn(t *testing.T) { client, err := newClient( zaptest.NewLogger(t), tracing.NopTracer, - bufpluginrunner.NewRunnerProvider(command.NewRunner()), + bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), ) require.NoError(t, err) duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig( @@ -283,7 +284,7 @@ func TestMultiClientCannotHaveOverlappingCategoriesWithBuiltIn(t *testing.T) { client, err := newClient( zaptest.NewLogger(t), tracing.NopTracer, - bufpluginrunner.NewRunnerProvider(command.NewRunner()), + bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), ) require.NoError(t, err) duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig( diff --git a/private/bufpkg/bufpluginrunner/bufpluginrunner.go b/private/bufpkg/bufpluginrpcutil/pluginrpcutil.go similarity index 57% rename from private/bufpkg/bufpluginrunner/bufpluginrunner.go rename to private/bufpkg/bufpluginrpcutil/pluginrpcutil.go index 5c834015f1..a974f281ef 100644 --- a/private/bufpkg/bufpluginrunner/bufpluginrunner.go +++ b/private/bufpkg/bufpluginrpcutil/pluginrpcutil.go @@ -12,37 +12,31 @@ // See the License for the specific language governing permissions and // limitations under the License. -package bufpluginrunner +package bufpluginrpcutil import ( "github.com/bufbuild/buf/private/bufpkg/bufconfig" - "github.com/bufbuild/buf/private/bufpkg/bufwasm" "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/wasm" "pluginrpc.com/pluginrpc" ) +// NewRunner returns a new pluginrpc.Runner for the command.Runner and program name. +func NewRunner(delegate command.Runner, programName string, programArgs ...string) pluginrpc.Runner { + return newRunner(delegate, programName, programArgs...) +} + +// NewWasmRunner returns a new pluginrpc.Runner for the wasm.Runtime and plugin name. +func NewWasmRunner(wasmRuntime wasm.Runtime, pluginName string) pluginrpc.Runner { + return newWasmRunner(wasmRuntime, pluginName) +} + // RunnerProvider provides pluginrpc.Runners for a given plugin config. type RunnerProvider interface { NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) } // NewRunnerProvider returns a new RunnerProvider for the command.Runner. -// -// This implementation should only be used for local applications. -func NewRunnerProvider( - delegate command.Runner, - options ...RunnerProviderOption, -) RunnerProvider { - return newRunnerProvider(delegate, options...) -} - -// RunnerProviderOption is an option for NewRunnerProvider. -type RunnerProviderOption func(*runnerProviderOptions) - -// RunnerProviderWithWasmRuntime returns a new RunnerProviderOption that -// specifies a Wasm runtime. This is required for local Wasm plugins. -func RunnerProviderWithWasmRuntime(wasmRuntime bufwasm.Runtime) RunnerProviderOption { - return func(runnerProviderOptions *runnerProviderOptions) { - runnerProviderOptions.wasmRuntime = wasmRuntime - } +func NewRunnerProvider(delegate command.Runner, wasmRuntime wasm.Runtime) RunnerProvider { + return newRunnerProvider(delegate, wasmRuntime) } diff --git a/private/pkg/pluginrpcutil/runner.go b/private/bufpkg/bufpluginrpcutil/runner.go similarity index 98% rename from private/pkg/pluginrpcutil/runner.go rename to private/bufpkg/bufpluginrpcutil/runner.go index af29b6af70..15f747257f 100644 --- a/private/pkg/pluginrpcutil/runner.go +++ b/private/bufpkg/bufpluginrpcutil/runner.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package pluginrpcutil +package bufpluginrpcutil import ( "context" diff --git a/private/bufpkg/bufpluginrpcutil/runner_provider.go b/private/bufpkg/bufpluginrpcutil/runner_provider.go new file mode 100644 index 0000000000..fdc2d11f4a --- /dev/null +++ b/private/bufpkg/bufpluginrpcutil/runner_provider.go @@ -0,0 +1,55 @@ +// 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 bufpluginrpcutil + +import ( + "github.com/bufbuild/buf/private/bufpkg/bufconfig" + "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/syserror" + "github.com/bufbuild/buf/private/pkg/wasm" + "pluginrpc.com/pluginrpc" +) + +type runnerProvider struct { + delegate command.Runner + wasmRuntime wasm.Runtime +} + +func newRunnerProvider(delegate command.Runner, wasmRuntime wasm.Runtime) *runnerProvider { + return &runnerProvider{ + delegate: delegate, + wasmRuntime: wasmRuntime, + } +} + +func (r *runnerProvider) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) { + switch pluginConfig.Type() { + case bufconfig.PluginConfigTypeLocal: + path := pluginConfig.Path() + return newRunner( + r.delegate, + // We know that Path is of at least length 1. + path[0], + path[1:]..., + ), nil + case bufconfig.PluginConfigTypeLocalWasm: + return newWasmRunner( + r.wasmRuntime, + pluginConfig.Name(), + ), nil + default: + return nil, syserror.Newf("unsupported plugin type: %v", pluginConfig.Type()) + } +} diff --git a/private/pkg/pluginrpcutil/usage.gen.go b/private/bufpkg/bufpluginrpcutil/usage.gen.go similarity index 96% rename from private/pkg/pluginrpcutil/usage.gen.go rename to private/bufpkg/bufpluginrpcutil/usage.gen.go index 75f8cda4af..8cf9480d37 100644 --- a/private/pkg/pluginrpcutil/usage.gen.go +++ b/private/bufpkg/bufpluginrpcutil/usage.gen.go @@ -14,6 +14,6 @@ // Generated. DO NOT EDIT. -package pluginrpcutil +package bufpluginrpcutil import _ "github.com/bufbuild/buf/private/usage" diff --git a/private/bufpkg/bufpluginrunner/provider.go b/private/bufpkg/bufpluginrpcutil/wasm_runner.go similarity index 63% rename from private/bufpkg/bufpluginrunner/provider.go rename to private/bufpkg/bufpluginrpcutil/wasm_runner.go index 916132d27b..4e5f362565 100644 --- a/private/bufpkg/bufpluginrunner/provider.go +++ b/private/bufpkg/bufpluginrpcutil/wasm_runner.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package bufpluginrunner +package bufpluginrpcutil import ( "context" @@ -22,75 +22,22 @@ import ( "os/exec" "sync" - "github.com/bufbuild/buf/private/bufpkg/bufconfig" - "github.com/bufbuild/buf/private/bufpkg/bufwasm" - "github.com/bufbuild/buf/private/pkg/command" - "github.com/bufbuild/buf/private/pkg/pluginrpcutil" - "github.com/bufbuild/buf/private/pkg/syserror" + "github.com/bufbuild/buf/private/pkg/wasm" "pluginrpc.com/pluginrpc" ) -type runnerProvider struct { - delegate command.Runner - wasmRuntime bufwasm.Runtime -} - -func newRunnerProvider(delegate command.Runner, options ...RunnerProviderOption) *runnerProvider { - runnerProviderOptions := newRunnerProviderOptions() - for _, option := range options { - option(runnerProviderOptions) - } - return &runnerProvider{ - delegate: delegate, - wasmRuntime: runnerProviderOptions.wasmRuntime, - } -} - -func (r *runnerProvider) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) { - switch pluginConfig.Type() { - case bufconfig.PluginConfigTypeLocal: - path := pluginConfig.Path() - return pluginrpcutil.NewRunner( - r.delegate, - // We know that Path is of at least length 1. - path[0], - path[1:]..., - ), nil - case bufconfig.PluginConfigTypeLocalWasm: - if r.wasmRuntime == nil { - return nil, syserror.Newf("wasm runtime is required for local wasm plugins") - } - return newWasmRunner( - r.wasmRuntime, - pluginConfig.Name(), - ), nil - default: - return nil, syserror.Newf("unsupported plugin type: %v", pluginConfig.Type()) - } -} - -type runnerProviderOptions struct { - wasmRuntime bufwasm.Runtime -} - -func newRunnerProviderOptions() *runnerProviderOptions { - return &runnerProviderOptions{ - wasmRuntime: bufwasm.UnimplementedRuntime, - } -} - // wasmRunner is a runner that loads a Wasm plugin. type wasmRunner struct { programName string - wasmRuntime bufwasm.Runtime + wasmRuntime wasm.Runtime // Once protects compiledModule and compiledModuleErr. once sync.Once - compiledModule bufwasm.CompiledModule + compiledModule wasm.CompiledModule compiledModuleErr error } // newWasmRunner returns a new pluginrpc.Runner for the Wasm binary on a -// bufwasm.Runtime and program name. This runner is only suitable for use with +// wasm.Runtime and program name. This runner is only suitable for use with // short-lived programs, compiled plugins lifetime is tied to the runtime. // // The program name should be the name of the program as it appears in the @@ -98,7 +45,7 @@ type wasmRunner struct { // exec.LookPath to find the program in the PATH. This is only safe for use in // the CLI, as it is not safe to use in a server environment. func newWasmRunner( - runtime bufwasm.Runtime, + runtime wasm.Runtime, programName string, ) *wasmRunner { return &wasmRunner{ @@ -115,14 +62,14 @@ func (r *wasmRunner) Run(ctx context.Context, env pluginrpc.Env) (retErr error) return compiledModule.Run(ctx, env) } -func (r *wasmRunner) loadCompiledModuleOnce(ctx context.Context) (bufwasm.CompiledModule, error) { +func (r *wasmRunner) loadCompiledModuleOnce(ctx context.Context) (wasm.CompiledModule, error) { r.once.Do(func() { r.compiledModule, r.compiledModuleErr = r.loadCompiledModule(ctx) }) return r.compiledModule, r.compiledModuleErr } -func (r *wasmRunner) loadCompiledModule(ctx context.Context) (bufwasm.CompiledModule, error) { +func (r *wasmRunner) loadCompiledModule(ctx context.Context) (wasm.CompiledModule, error) { // Find the plugin path. We use the same logic as exec.LookPath, but we do // not require the file to be executable. So check the local directory // first before checking the PATH. diff --git a/private/bufpkg/bufpluginrunner/usage.gen.go b/private/bufpkg/bufpluginrunner/usage.gen.go deleted file mode 100644 index e1bfe596f7..0000000000 --- a/private/bufpkg/bufpluginrunner/usage.gen.go +++ /dev/null @@ -1,19 +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. - -// Generated. DO NOT EDIT. - -package bufpluginrunner - -import _ "github.com/bufbuild/buf/private/usage" diff --git a/private/pkg/pluginrpcutil/pluginrpcutil.go b/private/pkg/pluginrpcutil/pluginrpcutil.go deleted file mode 100644 index f984f8b8a9..0000000000 --- a/private/pkg/pluginrpcutil/pluginrpcutil.go +++ /dev/null @@ -1,25 +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 pluginrpcutil - -import ( - "github.com/bufbuild/buf/private/pkg/command" - "pluginrpc.com/pluginrpc" -) - -// NewRunner returns a new pluginrpc.Runner for the command.Runner and program name. -func NewRunner(delegate command.Runner, programName string, programArgs ...string) pluginrpc.Runner { - return newRunner(delegate, programName, programArgs...) -} From 9634e9d265bd7aead75aecdd62dc39dd01cfac06 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 21:51:31 -0400 Subject: [PATCH 14/48] Fixes --- private/buf/buflsp/buflsp.go | 2 +- private/buf/bufmigrate/migrator.go | 2 +- .../bufpluginrpcutil/{pluginrpcutil.go => bufpluginrpcutil.go} | 2 +- private/bufpkg/bufpluginrpcutil/runner_provider.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename private/bufpkg/bufpluginrpcutil/{pluginrpcutil.go => bufpluginrpcutil.go} (95%) diff --git a/private/buf/buflsp/buflsp.go b/private/buf/buflsp/buflsp.go index 8063c11b98..bdac4fec7d 100644 --- a/private/buf/buflsp/buflsp.go +++ b/private/buf/buflsp/buflsp.go @@ -60,6 +60,7 @@ func Serve( if err != nil { return nil, err } + pluginCacheDir, err := bufcli.CreatePluginCacheDir(container) if err != nil { return nil, err @@ -69,7 +70,6 @@ func Serve( return nil, err } defer func() { retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) }() - tracer := tracing.NewTracer(container.Tracer()) checkClient, err := bufcheck.NewClient( container.Logger(), diff --git a/private/buf/bufmigrate/migrator.go b/private/buf/bufmigrate/migrator.go index a8b0bbb837..c991bd4adc 100644 --- a/private/buf/bufmigrate/migrator.go +++ b/private/buf/bufmigrate/migrator.go @@ -713,7 +713,7 @@ func equivalentCheckConfigInV2( checkConfig bufconfig.CheckConfig, ) (bufconfig.CheckConfig, error) { // No need for custom lint/breaking plugins since there's no plugins to migrate from <=v1. - // TODO: If we ever need v3, then we will have to deal with this.w + // TODO: If we ever need v3, then we will have to deal with this. client, err := bufcheck.NewClient(logger, tracer, bufpluginrpcutil.NewRunnerProvider(runner, wasm.UnimplementedRuntime)) if err != nil { return nil, err diff --git a/private/bufpkg/bufpluginrpcutil/pluginrpcutil.go b/private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go similarity index 95% rename from private/bufpkg/bufpluginrpcutil/pluginrpcutil.go rename to private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go index a974f281ef..630ee4feb4 100644 --- a/private/bufpkg/bufpluginrpcutil/pluginrpcutil.go +++ b/private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go @@ -31,7 +31,7 @@ func NewWasmRunner(wasmRuntime wasm.Runtime, pluginName string) pluginrpc.Runner return newWasmRunner(wasmRuntime, pluginName) } -// RunnerProvider provides pluginrpc.Runners for a given plugin config. +// RunnerProvider provides pluginrpc.Runners for a given PluginConfig. type RunnerProvider interface { NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) } diff --git a/private/bufpkg/bufpluginrpcutil/runner_provider.go b/private/bufpkg/bufpluginrpcutil/runner_provider.go index fdc2d11f4a..64fb47079a 100644 --- a/private/bufpkg/bufpluginrpcutil/runner_provider.go +++ b/private/bufpkg/bufpluginrpcutil/runner_provider.go @@ -50,6 +50,6 @@ func (r *runnerProvider) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginr pluginConfig.Name(), ), nil default: - return nil, syserror.Newf("unsupported plugin type: %v", pluginConfig.Type()) + return nil, syserror.Newf("unknown PluginConfigType: %v", pluginConfig.Type()) } } From 34982417f4c2879fbdc313b8b514217ec8ebed07 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 22:02:08 -0400 Subject: [PATCH 15/48] use wasmruntime cache dir --- private/buf/bufcli/cache.go | 17 ++++++++++------- private/buf/buflsp/buflsp.go | 2 +- .../buf/cmd/buf/command/breaking/breaking.go | 2 +- .../cmd/buf/command/config/internal/internal.go | 2 +- private/buf/cmd/buf/command/lint/lint.go | 2 +- 5 files changed, 14 insertions(+), 11 deletions(-) diff --git a/private/buf/bufcli/cache.go b/private/buf/bufcli/cache.go index 17f1900424..3c88725b35 100644 --- a/private/buf/bufcli/cache.go +++ b/private/buf/bufcli/cache.go @@ -103,10 +103,11 @@ var ( // // Normalized. v3CacheModuleLockRelDirPath = normalpath.Join("v3", "modulelocks") - // v3CachePluginsRelDirPath is the relative path to the plugins cache directory in its newest iteration. + // v3CacheWasmRuntimeRelDirPath is the relative path to the Wasm runtime cache directory in its newest iteration. + // This directory is used to store the Wasm runtime cache. This is an implementation specific cache and opaque outside of the runtime. // // Normalized. - v3CachePluginsRelDirPath = normalpath.Join("v3", "plugins") + v3CacheWasmRuntimeRelDirPath = normalpath.Join("v3", "wasmruntime") ) // NewModuleDataProvider returns a new ModuleDataProvider while creating the @@ -139,14 +140,16 @@ func NewCommitProvider(container appext.Container) (bufmodule.CommitProvider, er ) } -// CreatePluginCacheDir creates the cache directory for plugins. +// CreateWasmRuntimeCacheDir creates the cache directory for the Wasm runtime. // -// This is used by the [bufwasm.WithLocalCacheDir] option. -func CreatePluginCacheDir(container appext.Container) (string, error) { - if err := createCacheDir(container.CacheDirPath(), v3CachePluginsRelDirPath); err != nil { +// This is used by the Wasm runtime to cache compiled Wasm plugins. This is an +// implementation specific cache and opaque outside of the runtime. The runtime +// will manage the cache versioning. It is safe to clear this cache directory. +func CreateWasmRuntimeCacheDir(container appext.Container) (string, error) { + if err := createCacheDir(container.CacheDirPath(), v3CacheWasmRuntimeRelDirPath); err != nil { return "", err } - fullCacheDirPath := normalpath.Join(container.CacheDirPath(), v3CachePluginsRelDirPath) + fullCacheDirPath := normalpath.Join(container.CacheDirPath(), v3CacheWasmRuntimeRelDirPath) return fullCacheDirPath, nil } diff --git a/private/buf/buflsp/buflsp.go b/private/buf/buflsp/buflsp.go index bdac4fec7d..2f03ddeda5 100644 --- a/private/buf/buflsp/buflsp.go +++ b/private/buf/buflsp/buflsp.go @@ -61,7 +61,7 @@ func Serve( return nil, err } - pluginCacheDir, err := bufcli.CreatePluginCacheDir(container) + pluginCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return nil, err } diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index 9f84e04aa2..35205681f6 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -209,7 +209,7 @@ func run( len(againstImageWithConfigs), ) } - pluginCacheDir, err := bufcli.CreatePluginCacheDir(container) + pluginCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return err } diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index 3b6d19ac47..9366755da2 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -187,7 +187,7 @@ func lsRun( return err } } - pluginCacheDir, err := bufcli.CreatePluginCacheDir(container) + pluginCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return err } diff --git a/private/buf/cmd/buf/command/lint/lint.go b/private/buf/cmd/buf/command/lint/lint.go index a054e57933..4c44630302 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -134,7 +134,7 @@ func run( if err != nil { return err } - pluginCacheDir, err := bufcli.CreatePluginCacheDir(container) + pluginCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return err } From 767897e61f13e23b933c6f058a17c27c3eed4b71 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 22:09:54 -0400 Subject: [PATCH 16/48] doc RunnerProvider config --- private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go b/private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go index 630ee4feb4..c18e20c883 100644 --- a/private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go +++ b/private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go @@ -32,11 +32,21 @@ func NewWasmRunner(wasmRuntime wasm.Runtime, pluginName string) pluginrpc.Runner } // RunnerProvider provides pluginrpc.Runners for a given PluginConfig. +// +// The PluginConfig provides the configuration for a plugin. The PluginConfig +// must be of type bufconfig.PluginConfigTypeLocal or bufconfig.PluginConfigTypeLocalWasm. +// If the PluginConfig is of type bufconfig.PluginConfigTypeLocal, the path +// must be of length at least 1. If the PluginConfig is of type +// bufconfig.PluginConfigTypeLocalWasm, the name must be non-empty. If the +// PluginConfig is of any other type, an error will be returned. type RunnerProvider interface { NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) } // NewRunnerProvider returns a new RunnerProvider for the command.Runner. +// +// This implementation should only be used for local applications. It is safe to +// use concurrently. func NewRunnerProvider(delegate command.Runner, wasmRuntime wasm.Runtime) RunnerProvider { return newRunnerProvider(delegate, wasmRuntime) } From a69da63d78d714db948022b8720116bf38edb811 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 22:54:37 -0400 Subject: [PATCH 17/48] fix wasm args --- private/buf/bufcli/cache.go | 2 +- private/bufpkg/bufconfig/plugin_config.go | 32 ++++++++++++------- .../bufpluginrpcutil/bufpluginrpcutil.go | 28 +++++++++------- .../bufpluginrpcutil/runner_provider.go | 5 ++- .../bufpkg/bufpluginrpcutil/wasm_runner.go | 20 ++++++------ 5 files changed, 51 insertions(+), 36 deletions(-) diff --git a/private/buf/bufcli/cache.go b/private/buf/bufcli/cache.go index 3c88725b35..568e1104f0 100644 --- a/private/buf/bufcli/cache.go +++ b/private/buf/bufcli/cache.go @@ -144,7 +144,7 @@ func NewCommitProvider(container appext.Container) (bufmodule.CommitProvider, er // // This is used by the Wasm runtime to cache compiled Wasm plugins. This is an // implementation specific cache and opaque outside of the runtime. The runtime -// will manage the cache versioning. It is safe to clear this cache directory. +// will manage the cache versioning itself within this directory. func CreateWasmRuntimeCacheDir(container appext.Container) (string, error) { if err := createCacheDir(container.CacheDirPath(), v3CacheWasmRuntimeRelDirPath); err != nil { return "", err diff --git a/private/bufpkg/bufconfig/plugin_config.go b/private/bufpkg/bufconfig/plugin_config.go index 66fc8be9e3..72107eded3 100644 --- a/private/bufpkg/bufconfig/plugin_config.go +++ b/private/bufpkg/bufconfig/plugin_config.go @@ -69,7 +69,6 @@ func NewLocalPluginConfig( func NewLocalWasmPluginConfig( name string, options map[string]any, - path []string, ) (PluginConfig, error) { return newLocalWasmPluginConfig( name, @@ -109,21 +108,24 @@ func newPluginConfigForExternalV2( if err != nil { return nil, err } + if len(path) == 0 { + return nil, errors.New("must specify a path to the plugin") + } if len(path) == 1 { - name := path[0] // TODO: Parse as a module reference to fail early if we find // a remote plugin reference. This syntax is subject to change. - if _, err := bufmodule.ParseModuleRef(name); err == nil { - return nil, syserror.Newf("remote plugins are not yet supported") - } - // Wasm plugins are suffixed with .wasm. Otherwise, it's a binary. - if strings.HasSuffix(name, ".wasm") { - return newLocalWasmPluginConfig( - name, - options, - ) + if _, err := bufmodule.ParseModuleRef(path[0]); err == nil { + return nil, errors.New("remote plugins are not yet supported") } } + // Wasm plugins are suffixed with .wasm. Otherwise, it's a binary. + if strings.HasSuffix(path[0], ".wasm") { + return newLocalWasmPluginConfig( + strings.Join(path, " "), + options, + path, + ) + } return newLocalPluginConfig( strings.Join(path, " "), options, @@ -150,11 +152,19 @@ func newLocalPluginConfig( func newLocalWasmPluginConfig( name string, options map[string]any, + path []string, ) (*pluginConfig, error) { + if len(path) != 0 { + return nil, errors.New("must specify a path to the plugin") + } + if programName := path[0]; !strings.HasSuffix(programName, ".wasm") { + return nil, errors.New("Wasm plugin name must end with .wasm") + } return &pluginConfig{ pluginConfigType: PluginConfigTypeLocalWasm, name: name, options: options, + path: path, }, nil } diff --git a/private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go b/private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go index c18e20c883..ba70a16928 100644 --- a/private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go +++ b/private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go @@ -26,27 +26,31 @@ func NewRunner(delegate command.Runner, programName string, programArgs ...strin return newRunner(delegate, programName, programArgs...) } -// NewWasmRunner returns a new pluginrpc.Runner for the wasm.Runtime and plugin name. -func NewWasmRunner(wasmRuntime wasm.Runtime, pluginName string) pluginrpc.Runner { - return newWasmRunner(wasmRuntime, pluginName) +// NewWasmRunner returns a new pluginrpc.Runner for the wasm.Runtime and program name. +// +// This runner is used for local Wasm plugins. The program name is the path to the Wasm file. +func NewWasmRunner(wasmRuntime wasm.Runtime, programName string, programArgs ...string) pluginrpc.Runner { + return newWasmRunner(wasmRuntime, programName, programArgs...) } -// RunnerProvider provides pluginrpc.Runners for a given PluginConfig. -// -// The PluginConfig provides the configuration for a plugin. The PluginConfig -// must be of type bufconfig.PluginConfigTypeLocal or bufconfig.PluginConfigTypeLocalWasm. -// If the PluginConfig is of type bufconfig.PluginConfigTypeLocal, the path -// must be of length at least 1. If the PluginConfig is of type -// bufconfig.PluginConfigTypeLocalWasm, the name must be non-empty. If the -// PluginConfig is of any other type, an error will be returned. +// RunnerProvider provides pluginrpc.Runners for the PluginConfig. +// +// RunnerProvider selects the correct runner based on the PluginConfig.Type. type RunnerProvider interface { NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) } -// NewRunnerProvider returns a new RunnerProvider for the command.Runner. +// NewRunnerProvider returns a new RunnerProvider for the command.Runner and wasm.Runtime. // // This implementation should only be used for local applications. It is safe to // use concurrently. +// +// The RunnerProvider selects the correct runner based on the PluginConfig.Type. +// The supported types are: +// - bufconfig.PluginConfigTypeLocal +// - bufconfig.PluginConfigTypeLocalWasm +// +// If the PluginConfig.Type is not supported, an error is returned. func NewRunnerProvider(delegate command.Runner, wasmRuntime wasm.Runtime) RunnerProvider { return newRunnerProvider(delegate, wasmRuntime) } diff --git a/private/bufpkg/bufpluginrpcutil/runner_provider.go b/private/bufpkg/bufpluginrpcutil/runner_provider.go index 64fb47079a..84a7c18006 100644 --- a/private/bufpkg/bufpluginrpcutil/runner_provider.go +++ b/private/bufpkg/bufpluginrpcutil/runner_provider.go @@ -45,9 +45,12 @@ func (r *runnerProvider) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginr path[1:]..., ), nil case bufconfig.PluginConfigTypeLocalWasm: + path := pluginConfig.Path() return newWasmRunner( r.wasmRuntime, - pluginConfig.Name(), + // We know that Path is of at least length 1. + path[0], + path[1:]..., ), nil default: return nil, syserror.Newf("unknown PluginConfigType: %v", pluginConfig.Type()) diff --git a/private/bufpkg/bufpluginrpcutil/wasm_runner.go b/private/bufpkg/bufpluginrpcutil/wasm_runner.go index 4e5f362565..bb51eaf123 100644 --- a/private/bufpkg/bufpluginrpcutil/wasm_runner.go +++ b/private/bufpkg/bufpluginrpcutil/wasm_runner.go @@ -20,37 +20,32 @@ import ( "fmt" "os" "os/exec" + "slices" "sync" "github.com/bufbuild/buf/private/pkg/wasm" "pluginrpc.com/pluginrpc" ) -// wasmRunner is a runner that loads a Wasm plugin. type wasmRunner struct { - programName string wasmRuntime wasm.Runtime + programName string + programArgs []string // Once protects compiledModule and compiledModuleErr. once sync.Once compiledModule wasm.CompiledModule compiledModuleErr error } -// newWasmRunner returns a new pluginrpc.Runner for the Wasm binary on a -// wasm.Runtime and program name. This runner is only suitable for use with -// short-lived programs, compiled plugins lifetime is tied to the runtime. -// -// The program name should be the name of the program as it appears in the -// plugin config. The runner will check the current directory and fallback to -// exec.LookPath to find the program in the PATH. This is only safe for use in -// the CLI, as it is not safe to use in a server environment. func newWasmRunner( runtime wasm.Runtime, programName string, + programArgs ...string, ) *wasmRunner { return &wasmRunner{ - programName: programName, wasmRuntime: runtime, + programName: programName, + programArgs: programArgs, } } @@ -59,6 +54,9 @@ func (r *wasmRunner) Run(ctx context.Context, env pluginrpc.Env) (retErr error) if err != nil { return err } + if len(r.programArgs) > 0 { + env.Args = append(slices.Clone(r.programArgs), env.Args...) + } return compiledModule.Run(ctx, env) } From 87676c06c0bcad284abad9e47dafe24d0925d928 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 23:07:43 -0400 Subject: [PATCH 18/48] cleanup wasm module docs --- private/bufpkg/bufconfig/plugin_config.go | 14 ++----------- .../wasm/{plugin.go => compiled_module.go} | 10 +++------ private/pkg/wasm/runtime.go | 8 +++---- private/pkg/wasm/wasm.go | 21 +++++++++---------- 4 files changed, 19 insertions(+), 34 deletions(-) rename private/pkg/wasm/{plugin.go => compiled_module.go} (90%) diff --git a/private/bufpkg/bufconfig/plugin_config.go b/private/bufpkg/bufconfig/plugin_config.go index 72107eded3..dbac35b11d 100644 --- a/private/bufpkg/bufconfig/plugin_config.go +++ b/private/bufpkg/bufconfig/plugin_config.go @@ -99,11 +99,8 @@ func newPluginConfigForExternalV2( } options[key] = value } - // Differentiate between local and remote plugins. - // Local plugins are specified as a path to a binary or a Wasm file. - // Remote plugins are specified as a module reference. - // Paths with more than one element are assumed to be local plugin commands. - // This heuristic is based on the buffetch heuristics for inputs. + // TODO: differentiate between local and remote in the future + // Use the same heuristic that we do for dir vs module in buffetch path, err := encoding.InterfaceSliceOrStringToStringSlice(externalConfig.Plugin) if err != nil { return nil, err @@ -111,13 +108,6 @@ func newPluginConfigForExternalV2( if len(path) == 0 { return nil, errors.New("must specify a path to the plugin") } - if len(path) == 1 { - // TODO: Parse as a module reference to fail early if we find - // a remote plugin reference. This syntax is subject to change. - if _, err := bufmodule.ParseModuleRef(path[0]); err == nil { - return nil, errors.New("remote plugins are not yet supported") - } - } // Wasm plugins are suffixed with .wasm. Otherwise, it's a binary. if strings.HasSuffix(path[0], ".wasm") { return newLocalWasmPluginConfig( diff --git a/private/pkg/wasm/plugin.go b/private/pkg/wasm/compiled_module.go similarity index 90% rename from private/pkg/wasm/plugin.go rename to private/pkg/wasm/compiled_module.go index 3eec12a1a8..16dd536173 100644 --- a/private/pkg/wasm/plugin.go +++ b/private/pkg/wasm/compiled_module.go @@ -23,17 +23,13 @@ import ( ) type compiledModule struct { - pluginName string + moduleName string runtime wazero.Runtime compiledModule wazero.CompiledModule } var _ CompiledModule = (*compiledModule)(nil) -func (p *compiledModule) PluginName() string { - return p.pluginName -} - func (p *compiledModule) Run(ctx context.Context, env pluginrpc.Env) error { // Create a new module config with the given environment. config := wazero.NewModuleConfig(). @@ -47,9 +43,9 @@ func (p *compiledModule) Run(ctx context.Context, env pluginrpc.Env) error { ctx, p.compiledModule, // Use an empty name to allow for multiple instances of the same module. - // See [wazero.ModuleConfig.WithName]. + // See wazero.ModuleConfig.WithName. config.WithName("").WithArgs( - append([]string{p.pluginName}, env.Args...)..., + append([]string{p.moduleName}, env.Args...)..., ), ) if err != nil { diff --git a/private/pkg/wasm/runtime.go b/private/pkg/wasm/runtime.go index b54ba491b1..d10abe785f 100644 --- a/private/pkg/wasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -69,8 +69,8 @@ func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) }, nil } -func (r *runtime) Compile(ctx context.Context, pluginName string, pluginWasm []byte) (CompiledModule, error) { - if pluginName == "" { +func (r *runtime) Compile(ctx context.Context, moduleName string, moduleWasm []byte) (CompiledModule, error) { + if moduleName == "" { // The plugin is required to be named. We cannot use the name // from the Wasm binary as this is not guaranteed to be set and // may conflict with the provided name. @@ -78,12 +78,12 @@ func (r *runtime) Compile(ctx context.Context, pluginName string, pluginWasm []b } // Compile the WebAssembly. This operation is hashed on pluginWasm // bytes by the wazero runtime. - compiledModulePlugin, err := r.runtime.CompileModule(ctx, pluginWasm) + compiledModulePlugin, err := r.runtime.CompileModule(ctx, moduleWasm) if err != nil { return nil, err } return &compiledModule{ - pluginName: pluginName, + moduleName: moduleName, runtime: r.runtime, compiledModule: compiledModulePlugin, }, nil diff --git a/private/pkg/wasm/wasm.go b/private/pkg/wasm/wasm.go index 00244b59b1..65f2c2dc61 100644 --- a/private/pkg/wasm/wasm.go +++ b/private/pkg/wasm/wasm.go @@ -23,27 +23,26 @@ import ( // CompiledModule is a Wasm module ready to be run. // -// It is safe to use this module concurrently. Ensure that you call [Release] -// to free resources associated with the module. +// It is safe to use this module concurrently. When done, call Release to free +// resources held by the CompiledModule. All CompiledModules created by the +// same Runtime will be invalidated when the Runtime is released. // // Memory is limited by the runtime. To restrict CPU usage, cancel the context. type CompiledModule interface { pluginrpc.Runner - // PluginName returns the name of the plugin. - PluginName() string // Release releases all resources held by the compiled module. Release(ctx context.Context) error } // Runtime is a Wasm runtime. // -// It is safe to use this runtime concurrently. Ensure that you call [Release] -// when you are done with the runtime. All plugins created by this runtime will -// be invalidated when [Release] is called. +// It is safe to use this runtime concurrently. Release must be called when done +// with the runtime to free resources. All CompiledModules created by the same +// Runtime will be invalidated when the Runtime is released. type Runtime interface { - // Compile compiles the given Wasm module bytes into a [CompiledModule]. - Compile(ctx context.Context, pluginName string, pluginWasm []byte) (CompiledModule, error) - // Release releases all resources held by the runtime. + // Compile compiles the given Wasm module bytes into a CompiledModule. + Compile(ctx context.Context, moduleName string, moduleWasm []byte) (CompiledModule, error) + // Release releases all resources held by the Runtime. Release(ctx context.Context) error } @@ -52,7 +51,7 @@ func NewRuntime(ctx context.Context, options ...RuntimeOption) (Runtime, error) return newRuntime(ctx, options...) } -// RuntimeOption is an option for [NewRuntime]. +// RuntimeOption is an option for NewRuntime. type RuntimeOption interface { apply(*runtimeConfig) } From e42f48dd693f170493d684dcecc4cf826649aaae Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 23:17:01 -0400 Subject: [PATCH 19/48] fix bufconfig --- private/bufpkg/bufconfig/plugin_config.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/private/bufpkg/bufconfig/plugin_config.go b/private/bufpkg/bufconfig/plugin_config.go index dbac35b11d..0f015f593e 100644 --- a/private/bufpkg/bufconfig/plugin_config.go +++ b/private/bufpkg/bufconfig/plugin_config.go @@ -16,9 +16,9 @@ package bufconfig import ( "errors" + "fmt" "strings" - "github.com/bufbuild/buf/private/bufpkg/bufmodule" "github.com/bufbuild/buf/private/pkg/encoding" "github.com/bufbuild/buf/private/pkg/syserror" ) @@ -69,10 +69,12 @@ func NewLocalPluginConfig( func NewLocalWasmPluginConfig( name string, options map[string]any, + path []string, ) (PluginConfig, error) { return newLocalWasmPluginConfig( name, options, + path, ) } @@ -144,11 +146,11 @@ func newLocalWasmPluginConfig( options map[string]any, path []string, ) (*pluginConfig, error) { - if len(path) != 0 { + if len(path) == 0 { return nil, errors.New("must specify a path to the plugin") } - if programName := path[0]; !strings.HasSuffix(programName, ".wasm") { - return nil, errors.New("Wasm plugin name must end with .wasm") + if !strings.HasSuffix(path[0], ".wasm") { + return nil, fmt.Errorf("must specify a path to the plugin, and the first path argument must end with .wasm") } return &pluginConfig{ pluginConfigType: PluginConfigTypeLocalWasm, From 88ffcb7bbacd73425c1204c9b24b3953bde09389 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 23:30:27 -0400 Subject: [PATCH 20/48] fix naming --- private/buf/buflsp/buflsp.go | 4 ++-- private/buf/cmd/buf/command/breaking/breaking.go | 4 ++-- private/buf/cmd/buf/command/config/internal/internal.go | 4 ++-- private/buf/cmd/buf/command/lint/lint.go | 4 ++-- private/bufpkg/bufconfig/plugin_config.go | 4 ++++ private/bufpkg/bufpluginrpcutil/wasm_runner.go | 4 ++-- private/pkg/wasm/runtime.go | 6 ++++-- 7 files changed, 18 insertions(+), 12 deletions(-) diff --git a/private/buf/buflsp/buflsp.go b/private/buf/buflsp/buflsp.go index 2f03ddeda5..75fa541f0d 100644 --- a/private/buf/buflsp/buflsp.go +++ b/private/buf/buflsp/buflsp.go @@ -61,11 +61,11 @@ func Serve( return nil, err } - pluginCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) + wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return nil, err } - wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(pluginCacheDir)) + wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(wasmRuntimeCacheDir)) if err != nil { return nil, err } diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index 35205681f6..d447c68d74 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -209,11 +209,11 @@ func run( len(againstImageWithConfigs), ) } - pluginCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) + wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return err } - wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(pluginCacheDir)) + wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(wasmRuntimeCacheDir)) if err != nil { return err } diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index 9366755da2..53f731135a 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -187,11 +187,11 @@ func lsRun( return err } } - pluginCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) + wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return err } - wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(pluginCacheDir)) + wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(wasmRuntimeCacheDir)) if err != nil { return err } diff --git a/private/buf/cmd/buf/command/lint/lint.go b/private/buf/cmd/buf/command/lint/lint.go index 4c44630302..21c2a7770d 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -134,11 +134,11 @@ func run( if err != nil { return err } - pluginCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) + wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return err } - wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(pluginCacheDir)) + wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(wasmRuntimeCacheDir)) if err != nil { return err } diff --git a/private/bufpkg/bufconfig/plugin_config.go b/private/bufpkg/bufconfig/plugin_config.go index 0f015f593e..fd9f8e39b2 100644 --- a/private/bufpkg/bufconfig/plugin_config.go +++ b/private/bufpkg/bufconfig/plugin_config.go @@ -66,6 +66,10 @@ func NewLocalPluginConfig( } // NewLocalWasmPluginConfig returns a new PluginConfig for a local Wasm plugin. +// +// The first path argument is the path to the Wasm plugin and must end with .wasm. +// The remaining path arguments are the arguments to the Wasm plugin. These are passed +// to the Wasm plugin as command line arguments. func NewLocalWasmPluginConfig( name string, options map[string]any, diff --git a/private/bufpkg/bufpluginrpcutil/wasm_runner.go b/private/bufpkg/bufpluginrpcutil/wasm_runner.go index bb51eaf123..4fa0fb70b1 100644 --- a/private/bufpkg/bufpluginrpcutil/wasm_runner.go +++ b/private/bufpkg/bufpluginrpcutil/wasm_runner.go @@ -81,12 +81,12 @@ func (r *wasmRunner) loadCompiledModule(ctx context.Context) (wasm.CompiledModul return nil, fmt.Errorf("could not find plugin %q in PATH: %v", r.programName, err) } } - wasmModule, err := os.ReadFile(path) + moduleWasm, err := os.ReadFile(path) if err != nil { return nil, err } // Compile and run, releasing the compiledModule at the end. - compiledModule, err := r.wasmRuntime.Compile(ctx, r.programName, wasmModule) + compiledModule, err := r.wasmRuntime.Compile(ctx, r.programName, moduleWasm) if err != nil { return nil, err } diff --git a/private/pkg/wasm/runtime.go b/private/pkg/wasm/runtime.go index d10abe785f..b97809b026 100644 --- a/private/pkg/wasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -76,8 +76,10 @@ func (r *runtime) Compile(ctx context.Context, moduleName string, moduleWasm []b // may conflict with the provided name. return nil, fmt.Errorf("name is empty") } - // Compile the WebAssembly. This operation is hashed on pluginWasm - // bytes by the wazero runtime. + // Compile the WebAssembly. This operation is hashed on the module + // bytes and the runtime configuration. The compiled module is + // cached in memory and on disk if an optional cache directory is + // provided. compiledModulePlugin, err := r.runtime.CompileModule(ctx, moduleWasm) if err != nil { return nil, err From 85c7fbc7adad04404863f87ca8c2922c941064b5 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 1 Oct 2024 23:41:55 -0400 Subject: [PATCH 21/48] fix banimports --- private/buf/buflsp/buflsp.go | 28 ++--------------- private/buf/cmd/buf/command/beta/lsp/lsp.go | 31 +++++++++++++++++-- .../buf/cmd/buf/command/breaking/breaking.go | 4 ++- .../buf/command/config/internal/internal.go | 4 ++- private/buf/cmd/buf/command/lint/lint.go | 4 ++- private/bufpkg/bufcheck/lint_test.go | 3 ++ 6 files changed, 43 insertions(+), 31 deletions(-) diff --git a/private/buf/buflsp/buflsp.go b/private/buf/buflsp/buflsp.go index 75fa541f0d..96560847ad 100644 --- a/private/buf/buflsp/buflsp.go +++ b/private/buf/buflsp/buflsp.go @@ -22,21 +22,16 @@ import ( "fmt" "sync/atomic" - "github.com/bufbuild/buf/private/buf/bufcli" "github.com/bufbuild/buf/private/buf/bufctl" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/app/appext" - "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/tracing" - "github.com/bufbuild/buf/private/pkg/wasm" "go.lsp.dev/jsonrpc2" "go.lsp.dev/protocol" "go.opentelemetry.io/otel/attribute" - "go.uber.org/multierr" "go.uber.org/zap" ) @@ -47,6 +42,7 @@ func Serve( ctx context.Context, container appext.Container, controller bufctl.Controller, + checkClient bufcheck.Client, stream jsonrpc2.Stream, ) (_ jsonrpc2.Conn, retErr error) { // The LSP protocol deals with absolute filesystem paths. This requires us to @@ -61,26 +57,6 @@ func Serve( return nil, err } - wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) - if err != nil { - return nil, err - } - wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(wasmRuntimeCacheDir)) - if err != nil { - return nil, err - } - defer func() { retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) }() - tracer := tracing.NewTracer(container.Tracer()) - checkClient, err := bufcheck.NewClient( - container.Logger(), - tracer, - bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasmRuntime), - bufcheck.ClientWithStderr(container.Stderr()), - ) - if err != nil { - return nil, err - } - conn := jsonrpc2.NewConn(stream) lsp := &lsp{ conn: conn, @@ -89,7 +65,7 @@ func Serve( zap.NewNop(), // The logging from protocol itself isn't very good, we've replaced it with connAdapter here. ), logger: container.Logger(), - tracer: tracer, + tracer: tracing.NewTracer(container.Tracer()), controller: controller, checkClient: checkClient, rootBucket: bucket, diff --git a/private/buf/cmd/buf/command/beta/lsp/lsp.go b/private/buf/cmd/buf/command/beta/lsp/lsp.go index e20dcf229d..35a701d659 100644 --- a/private/buf/cmd/buf/command/beta/lsp/lsp.go +++ b/private/buf/cmd/buf/command/beta/lsp/lsp.go @@ -25,11 +25,17 @@ import ( "github.com/bufbuild/buf/private/buf/bufcli" "github.com/bufbuild/buf/private/buf/buflsp" + "github.com/bufbuild/buf/private/bufpkg/bufcheck" + "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" + "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/ioext" + "github.com/bufbuild/buf/private/pkg/tracing" + "github.com/bufbuild/buf/private/pkg/wasm" "github.com/spf13/pflag" "go.lsp.dev/jsonrpc2" + "go.uber.org/multierr" ) const ( @@ -77,7 +83,7 @@ func run( ctx context.Context, container appext.Container, flags *flags, -) error { +) (retErr error) { bufcli.WarnBetaCommand(ctx, container) transport, err := dial(container, flags) @@ -90,7 +96,28 @@ func run( return err } - conn, err := buflsp.Serve(ctx, container, controller, jsonrpc2.NewStream(transport)) + wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) + if err != nil { + return err + } + wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(wasmRuntimeCacheDir)) + if err != nil { + return err + } + defer func() { + retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) + }() + checkClient, err := bufcheck.NewClient( + container.Logger(), + tracing.NewTracer(container.Tracer()), + bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasmRuntime), + bufcheck.ClientWithStderr(container.Stderr()), + ) + if err != nil { + return err + } + + conn, err := buflsp.Serve(ctx, container, controller, checkClient, jsonrpc2.NewStream(transport)) if err != nil { return err } diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index d447c68d74..30c1a67fec 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -217,7 +217,9 @@ func run( if err != nil { return err } - defer func() { retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) }() + defer func() { + retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) + }() tracer := tracing.NewTracer(container.Tracer()) var allFileAnnotations []bufanalysis.FileAnnotation for i, imageWithConfig := range imageWithConfigs { diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index 53f731135a..72c8eff48a 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -195,7 +195,9 @@ func lsRun( if err != nil { return err } - defer func() { retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) }() + defer func() { + retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) + }() tracer := tracing.NewTracer(container.Tracer()) client, err := bufcheck.NewClient( container.Logger(), diff --git a/private/buf/cmd/buf/command/lint/lint.go b/private/buf/cmd/buf/command/lint/lint.go index 21c2a7770d..e9d55a440c 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -142,7 +142,9 @@ func run( if err != nil { return err } - defer func() { retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) }() + defer func() { + retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) + }() tracer := tracing.NewTracer(container.Tracer()) var allFileAnnotations []bufanalysis.FileAnnotation for _, imageWithConfig := range imageWithConfigs { diff --git a/private/bufpkg/bufcheck/lint_test.go b/private/bufpkg/bufcheck/lint_test.go index 43a95f74b5..f6d4087e94 100644 --- a/private/bufpkg/bufcheck/lint_test.go +++ b/private/bufpkg/bufcheck/lint_test.go @@ -1315,6 +1315,9 @@ func testLintWithOptions( require.NotNil(t, lintConfig) wasmRuntime, err := wasm.NewRuntime(ctx) require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, wasmRuntime.Release(ctx)) + }) client, err := bufcheck.NewClient( zap.NewNop(), tracing.NopTracer, From 41f812761a1b510f1e489599fe656c14874ce1d1 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 00:02:22 -0400 Subject: [PATCH 22/48] fix pluginrpcutil --- private/buf/buflsp/buflsp.go | 2 +- private/buf/bufmigrate/migrator.go | 3 +-- private/buf/cmd/buf/buf_test.go | 3 +-- private/buf/cmd/buf/command/beta/lsp/lsp.go | 3 +-- .../buf/cmd/buf/command/breaking/breaking.go | 3 +-- .../buf/command/config/internal/internal.go | 3 +-- private/buf/cmd/buf/command/lint/lint.go | 3 +-- .../cmd/buf/command/mod/internal/internal.go | 3 +-- .../cmd/protoc-gen-buf-breaking/breaking.go | 3 +-- private/buf/cmd/protoc-gen-buf-lint/lint.go | 3 +-- private/bufpkg/bufcheck/breaking_test.go | 3 +-- private/bufpkg/bufcheck/bufcheck.go | 19 ++++++++++++++ private/bufpkg/bufcheck/lint_test.go | 3 +-- private/bufpkg/bufcheck/multi_client_test.go | 5 ++-- .../runner_provider.go | 7 +++--- .../pluginrpcutil/pluginrpcutil.go} | 25 +------------------ .../pluginrpcutil}/runner.go | 2 +- .../pluginrpcutil}/usage.gen.go | 2 +- .../pluginrpcutil}/wasm_runner.go | 2 +- 19 files changed, 41 insertions(+), 56 deletions(-) rename private/bufpkg/{bufpluginrpcutil => bufcheck}/runner_provider.go (92%) rename private/{bufpkg/bufpluginrpcutil/bufpluginrpcutil.go => pkg/pluginrpcutil/pluginrpcutil.go} (58%) rename private/{bufpkg/bufpluginrpcutil => pkg/pluginrpcutil}/runner.go (98%) rename private/{bufpkg/bufpluginrpcutil => pkg/pluginrpcutil}/usage.gen.go (96%) rename private/{bufpkg/bufpluginrpcutil => pkg/pluginrpcutil}/wasm_runner.go (99%) diff --git a/private/buf/buflsp/buflsp.go b/private/buf/buflsp/buflsp.go index 96560847ad..06c3cc51e9 100644 --- a/private/buf/buflsp/buflsp.go +++ b/private/buf/buflsp/buflsp.go @@ -44,7 +44,7 @@ func Serve( controller bufctl.Controller, checkClient bufcheck.Client, stream jsonrpc2.Stream, -) (_ jsonrpc2.Conn, retErr error) { +) (jsonrpc2.Conn, error) { // The LSP protocol deals with absolute filesystem paths. This requires us to // bypass the bucket API completely, so we create a bucket pointing at the filesystem // root. diff --git a/private/buf/bufmigrate/migrator.go b/private/buf/bufmigrate/migrator.go index 2a05928d03..a014ecd0ea 100644 --- a/private/buf/bufmigrate/migrator.go +++ b/private/buf/bufmigrate/migrator.go @@ -26,7 +26,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufmodule" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/normalpath" "github.com/bufbuild/buf/private/pkg/slicesext" @@ -714,7 +713,7 @@ func equivalentCheckConfigInV2( ) (bufconfig.CheckConfig, error) { // No need for custom lint/breaking plugins since there's no plugins to migrate from <=v1. // TODO: If we ever need v3, then we will have to deal with this. - client, err := bufcheck.NewClient(logger, tracer, bufpluginrpcutil.NewRunnerProvider(runner, wasm.UnimplementedRuntime)) + client, err := bufcheck.NewClient(logger, tracer, bufcheck.NewRunnerProvider(runner, wasm.UnimplementedRuntime)) if err != nil { return nil, err } diff --git a/private/buf/cmd/buf/buf_test.go b/private/buf/cmd/buf/buf_test.go index 093011471c..fc6a57efdf 100644 --- a/private/buf/cmd/buf/buf_test.go +++ b/private/buf/cmd/buf/buf_test.go @@ -35,7 +35,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufimage" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" imagev1 "github.com/bufbuild/buf/private/gen/proto/go/buf/alpha/image/v1" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appcmd/appcmdtesting" @@ -1351,7 +1350,7 @@ func TestCheckLsBreakingRulesFromConfigExceptDeprecated(t *testing.T) { t.Run(version.String(), func(t *testing.T) { t.Parallel() // Do not need any custom lint/breaking plugins here. - client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime)) + client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufcheck.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime)) require.NoError(t, err) allRules, err := client.AllRules(context.Background(), check.RuleTypeBreaking, version) require.NoError(t, err) diff --git a/private/buf/cmd/buf/command/beta/lsp/lsp.go b/private/buf/cmd/buf/command/beta/lsp/lsp.go index 35a701d659..855b7f7a57 100644 --- a/private/buf/cmd/buf/command/beta/lsp/lsp.go +++ b/private/buf/cmd/buf/command/beta/lsp/lsp.go @@ -26,7 +26,6 @@ import ( "github.com/bufbuild/buf/private/buf/bufcli" "github.com/bufbuild/buf/private/buf/buflsp" "github.com/bufbuild/buf/private/bufpkg/bufcheck" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" @@ -110,7 +109,7 @@ func run( checkClient, err := bufcheck.NewClient( container.Logger(), tracing.NewTracer(container.Tracer()), - bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasmRuntime), + bufcheck.NewRunnerProvider(command.NewRunner(), wasmRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index 30c1a67fec..dd23632303 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -25,7 +25,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" @@ -226,7 +225,7 @@ func run( client, err := bufcheck.NewClient( container.Logger(), tracer, - bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasmRuntime), + bufcheck.NewRunnerProvider(command.NewRunner(), wasmRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index 72c8eff48a..692159faeb 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -24,7 +24,6 @@ import ( "github.com/bufbuild/buf/private/buf/bufcli" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufconfig" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" @@ -202,7 +201,7 @@ func lsRun( client, err := bufcheck.NewClient( container.Logger(), tracer, - bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasmRuntime), + bufcheck.NewRunnerProvider(command.NewRunner(), wasmRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/lint/lint.go b/private/buf/cmd/buf/command/lint/lint.go index e9d55a440c..4f9893fb0e 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -23,7 +23,6 @@ import ( "github.com/bufbuild/buf/private/buf/bufctl" "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" @@ -151,7 +150,7 @@ func run( client, err := bufcheck.NewClient( container.Logger(), tracer, - bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasmRuntime), + bufcheck.NewRunnerProvider(command.NewRunner(), wasmRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/mod/internal/internal.go b/private/buf/cmd/buf/command/mod/internal/internal.go index 4a21b1a646..d7f7c71ba1 100644 --- a/private/buf/cmd/buf/command/mod/internal/internal.go +++ b/private/buf/cmd/buf/command/mod/internal/internal.go @@ -24,7 +24,6 @@ import ( "github.com/bufbuild/buf/private/buf/bufcli" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufconfig" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/command" @@ -179,7 +178,7 @@ func lsRun( tracer := tracing.NewTracer(container.Tracer()) client, err := bufcheck.NewClient( container.Logger(), tracer, - bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), + bufcheck.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go index 4b5f7a19b5..522274a1a5 100644 --- a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go +++ b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go @@ -28,7 +28,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/encoding" "github.com/bufbuild/buf/private/pkg/protodescriptor" @@ -130,7 +129,7 @@ func handle( client, err := bufcheck.NewClient( container.Logger(), tracer, - bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), + bufcheck.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), bufcheck.ClientWithStderr(pluginEnv.Stderr), ) if err != nil { diff --git a/private/buf/cmd/protoc-gen-buf-lint/lint.go b/private/buf/cmd/protoc-gen-buf-lint/lint.go index a70437b178..b61a13edf2 100644 --- a/private/buf/cmd/protoc-gen-buf-lint/lint.go +++ b/private/buf/cmd/protoc-gen-buf-lint/lint.go @@ -27,7 +27,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/encoding" "github.com/bufbuild/buf/private/pkg/protodescriptor" @@ -105,7 +104,7 @@ func handle( client, err := bufcheck.NewClient( container.Logger(), tracer, - bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), + bufcheck.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), bufcheck.ClientWithStderr(pluginEnv.Stderr), ) if err != nil { diff --git a/private/bufpkg/bufcheck/breaking_test.go b/private/bufpkg/bufcheck/breaking_test.go index 65ee05b783..3d53d63afa 100644 --- a/private/bufpkg/bufcheck/breaking_test.go +++ b/private/bufpkg/bufcheck/breaking_test.go @@ -30,7 +30,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/bufpkg/bufmodule" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/tracing" @@ -1349,7 +1348,7 @@ func testBreaking( require.NoError(t, err) breakingConfig := workspace.GetBreakingConfigForOpaqueID(opaqueID) require.NotNil(t, breakingConfig) - client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime)) + client, err := bufcheck.NewClient(zap.NewNop(), tracing.NopTracer, bufcheck.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime)) require.NoError(t, err) err = client.Breaking( ctx, diff --git a/private/bufpkg/bufcheck/bufcheck.go b/private/bufpkg/bufcheck/bufcheck.go index 91aaf0cc89..450d0f94d9 100644 --- a/private/bufpkg/bufcheck/bufcheck.go +++ b/private/bufpkg/bufcheck/bufcheck.go @@ -21,9 +21,11 @@ import ( "buf.build/go/bufplugin/check" "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/bufbuild/buf/private/pkg/tracing" + "github.com/bufbuild/buf/private/pkg/wasm" "go.uber.org/zap" "pluginrpc.com/pluginrpc" ) @@ -163,10 +165,27 @@ type RunnerProvider interface { type RunnerProviderFunc func(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) // NewRunner implements RunnerProvider. +// +// RunnerProvider selects the correct runner based on the PluginConfig.Type. func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) { return r(pluginConfig) } +// NewRunnerProvider returns a new RunnerProvider for the command.Runner and wasm.Runtime. +// +// This implementation should only be used for local applications. It is safe to +// use concurrently. +// +// The RunnerProvider selects the correct runner based on the PluginConfig.Type. +// The supported types are: +// - bufconfig.PluginConfigTypeLocal +// - bufconfig.PluginConfigTypeLocalWasm +// +// If the PluginConfig.Type is not supported, an error is returned. +func NewRunnerProvider(delegate command.Runner, wasmRuntime wasm.Runtime) RunnerProvider { + return newRunnerProvider(delegate, wasmRuntime) +} + // NewClient returns a new Client. func NewClient( logger *zap.Logger, diff --git a/private/bufpkg/bufcheck/lint_test.go b/private/bufpkg/bufcheck/lint_test.go index f6d4087e94..c4e0b7f756 100644 --- a/private/bufpkg/bufcheck/lint_test.go +++ b/private/bufpkg/bufcheck/lint_test.go @@ -27,7 +27,6 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/bufpkg/bufmodule" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/tracing" @@ -1321,7 +1320,7 @@ func testLintWithOptions( client, err := bufcheck.NewClient( zap.NewNop(), tracing.NopTracer, - bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasmRuntime), + bufcheck.NewRunnerProvider(command.NewRunner(), wasmRuntime), ) require.NoError(t, err) err = client.Lint( diff --git a/private/bufpkg/bufcheck/multi_client_test.go b/private/bufpkg/bufcheck/multi_client_test.go index e7a911c8c8..4561996031 100644 --- a/private/bufpkg/bufcheck/multi_client_test.go +++ b/private/bufpkg/bufcheck/multi_client_test.go @@ -24,7 +24,6 @@ import ( "buf.build/go/bufplugin/check/checkutil" "buf.build/go/bufplugin/option" "github.com/bufbuild/buf/private/bufpkg/bufconfig" - "github.com/bufbuild/buf/private/bufpkg/bufpluginrpcutil" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/stringutil" @@ -189,7 +188,7 @@ func TestMultiClientCannotHaveOverlappingRulesWithBuiltIn(t *testing.T) { client, err := newClient( zaptest.NewLogger(t), tracing.NopTracer, - bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), + NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), ) require.NoError(t, err) duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig( @@ -284,7 +283,7 @@ func TestMultiClientCannotHaveOverlappingCategoriesWithBuiltIn(t *testing.T) { client, err := newClient( zaptest.NewLogger(t), tracing.NopTracer, - bufpluginrpcutil.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), + NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), ) require.NoError(t, err) duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig( diff --git a/private/bufpkg/bufpluginrpcutil/runner_provider.go b/private/bufpkg/bufcheck/runner_provider.go similarity index 92% rename from private/bufpkg/bufpluginrpcutil/runner_provider.go rename to private/bufpkg/bufcheck/runner_provider.go index 84a7c18006..6f77a826ef 100644 --- a/private/bufpkg/bufpluginrpcutil/runner_provider.go +++ b/private/bufpkg/bufcheck/runner_provider.go @@ -12,11 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -package bufpluginrpcutil +package bufcheck import ( "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/pkg/command" + "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/bufbuild/buf/private/pkg/wasm" "pluginrpc.com/pluginrpc" @@ -38,7 +39,7 @@ func (r *runnerProvider) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginr switch pluginConfig.Type() { case bufconfig.PluginConfigTypeLocal: path := pluginConfig.Path() - return newRunner( + return pluginrpcutil.NewRunner( r.delegate, // We know that Path is of at least length 1. path[0], @@ -46,7 +47,7 @@ func (r *runnerProvider) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginr ), nil case bufconfig.PluginConfigTypeLocalWasm: path := pluginConfig.Path() - return newWasmRunner( + return pluginrpcutil.NewWasmRunner( r.wasmRuntime, // We know that Path is of at least length 1. path[0], diff --git a/private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go b/private/pkg/pluginrpcutil/pluginrpcutil.go similarity index 58% rename from private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go rename to private/pkg/pluginrpcutil/pluginrpcutil.go index ba70a16928..99647188f9 100644 --- a/private/bufpkg/bufpluginrpcutil/bufpluginrpcutil.go +++ b/private/pkg/pluginrpcutil/pluginrpcutil.go @@ -12,10 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -package bufpluginrpcutil +package pluginrpcutil import ( - "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/pkg/command" "github.com/bufbuild/buf/private/pkg/wasm" "pluginrpc.com/pluginrpc" @@ -32,25 +31,3 @@ func NewRunner(delegate command.Runner, programName string, programArgs ...strin func NewWasmRunner(wasmRuntime wasm.Runtime, programName string, programArgs ...string) pluginrpc.Runner { return newWasmRunner(wasmRuntime, programName, programArgs...) } - -// RunnerProvider provides pluginrpc.Runners for the PluginConfig. -// -// RunnerProvider selects the correct runner based on the PluginConfig.Type. -type RunnerProvider interface { - NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) -} - -// NewRunnerProvider returns a new RunnerProvider for the command.Runner and wasm.Runtime. -// -// This implementation should only be used for local applications. It is safe to -// use concurrently. -// -// The RunnerProvider selects the correct runner based on the PluginConfig.Type. -// The supported types are: -// - bufconfig.PluginConfigTypeLocal -// - bufconfig.PluginConfigTypeLocalWasm -// -// If the PluginConfig.Type is not supported, an error is returned. -func NewRunnerProvider(delegate command.Runner, wasmRuntime wasm.Runtime) RunnerProvider { - return newRunnerProvider(delegate, wasmRuntime) -} diff --git a/private/bufpkg/bufpluginrpcutil/runner.go b/private/pkg/pluginrpcutil/runner.go similarity index 98% rename from private/bufpkg/bufpluginrpcutil/runner.go rename to private/pkg/pluginrpcutil/runner.go index 15f747257f..af29b6af70 100644 --- a/private/bufpkg/bufpluginrpcutil/runner.go +++ b/private/pkg/pluginrpcutil/runner.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package bufpluginrpcutil +package pluginrpcutil import ( "context" diff --git a/private/bufpkg/bufpluginrpcutil/usage.gen.go b/private/pkg/pluginrpcutil/usage.gen.go similarity index 96% rename from private/bufpkg/bufpluginrpcutil/usage.gen.go rename to private/pkg/pluginrpcutil/usage.gen.go index 8cf9480d37..75f8cda4af 100644 --- a/private/bufpkg/bufpluginrpcutil/usage.gen.go +++ b/private/pkg/pluginrpcutil/usage.gen.go @@ -14,6 +14,6 @@ // Generated. DO NOT EDIT. -package bufpluginrpcutil +package pluginrpcutil import _ "github.com/bufbuild/buf/private/usage" diff --git a/private/bufpkg/bufpluginrpcutil/wasm_runner.go b/private/pkg/pluginrpcutil/wasm_runner.go similarity index 99% rename from private/bufpkg/bufpluginrpcutil/wasm_runner.go rename to private/pkg/pluginrpcutil/wasm_runner.go index 4fa0fb70b1..8620528497 100644 --- a/private/bufpkg/bufpluginrpcutil/wasm_runner.go +++ b/private/pkg/pluginrpcutil/wasm_runner.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package bufpluginrpcutil +package pluginrpcutil import ( "context" From 2be37aacac0744a2cb4eaa99dedf44e19b1d7888 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 00:07:26 -0400 Subject: [PATCH 23/48] fix doc --- private/pkg/pluginrpcutil/wasm_runner.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/private/pkg/pluginrpcutil/wasm_runner.go b/private/pkg/pluginrpcutil/wasm_runner.go index 8620528497..cca9e103f2 100644 --- a/private/pkg/pluginrpcutil/wasm_runner.go +++ b/private/pkg/pluginrpcutil/wasm_runner.go @@ -85,14 +85,13 @@ func (r *wasmRunner) loadCompiledModule(ctx context.Context) (wasm.CompiledModul if err != nil { return nil, err } - // Compile and run, releasing the compiledModule at the end. + // Compile the module. This plugin is never released, so subsequent + // calls to this function will benefit from the cached plugin. This is + // only safe as the runner is limited to the CLI. compiledModule, err := r.wasmRuntime.Compile(ctx, r.programName, moduleWasm) if err != nil { return nil, err } - // This plugin is never released, so subsequent calls to this function - // will benefit from the cached plugin. This is only safe as the - // runner is limited to the CLI. return compiledModule, nil } From 0aacd547275bd03c3569630f0a9434e53a9d9520 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 08:16:20 -0400 Subject: [PATCH 24/48] fix once --- private/pkg/pluginrpcutil/wasm_runner.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/private/pkg/pluginrpcutil/wasm_runner.go b/private/pkg/pluginrpcutil/wasm_runner.go index cca9e103f2..54803b1a57 100644 --- a/private/pkg/pluginrpcutil/wasm_runner.go +++ b/private/pkg/pluginrpcutil/wasm_runner.go @@ -31,8 +31,10 @@ type wasmRunner struct { wasmRuntime wasm.Runtime programName string programArgs []string - // Once protects compiledModule and compiledModuleErr. - once sync.Once + // lock protects compiledModule and compiledModuleErr. Store called as + // a boolean to avoid nil comparison. + lock sync.RWMutex + called bool compiledModule wasm.CompiledModule compiledModuleErr error } @@ -61,9 +63,18 @@ func (r *wasmRunner) Run(ctx context.Context, env pluginrpc.Env) (retErr error) } func (r *wasmRunner) loadCompiledModuleOnce(ctx context.Context) (wasm.CompiledModule, error) { - r.once.Do(func() { + r.lock.RLock() + if r.called { + r.lock.RUnlock() + return r.compiledModule, r.compiledModuleErr + } + r.lock.RUnlock() + r.lock.Lock() + defer r.lock.Unlock() + if !r.called { r.compiledModule, r.compiledModuleErr = r.loadCompiledModule(ctx) - }) + r.called = true + } return r.compiledModule, r.compiledModuleErr } From a073aedbd9091a9134bd466876c351148d35fd74 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 08:16:27 -0400 Subject: [PATCH 25/48] add changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3145874e9f..4043ba8716 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## [Unreleased] -- No changes yet. +- Add support for custom WebAssembly plugins for lint and breaking changes plugins. Use the `.wasm` file extension to specify a WebAssembly plugin. ## [v1.43.0] - 2024-09-30 From e3c329d5c486ff6166f06ae81a29b49f37ecba45 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 08:37:53 -0400 Subject: [PATCH 26/48] fix docs --- private/pkg/wasm/wasm.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/private/pkg/wasm/wasm.go b/private/pkg/wasm/wasm.go index 65f2c2dc61..443437740e 100644 --- a/private/pkg/wasm/wasm.go +++ b/private/pkg/wasm/wasm.go @@ -36,8 +36,8 @@ type CompiledModule interface { // Runtime is a Wasm runtime. // -// It is safe to use this runtime concurrently. Release must be called when done -// with the runtime to free resources. All CompiledModules created by the same +// It is safe to use the Runtime concurrently. Release must be called when done +// with the Runtime to free resources. All CompiledModules created by the same // Runtime will be invalidated when the Runtime is released. type Runtime interface { // Compile compiles the given Wasm module bytes into a CompiledModule. @@ -72,5 +72,5 @@ func WithLocalCacheDir(cacheDir string) RuntimeOption { }) } -// UnimplementedRuntime returns a new unimplemented Runtime. +// UnimplementedRuntime is an unimplemented Runtime. var UnimplementedRuntime = unimplementedRuntime{} From e1c589bab11b368af7978ed4c74a3d6919ac1af6 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 08:45:49 -0400 Subject: [PATCH 27/48] doc wasm cache --- private/pkg/wasm/wasm.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/private/pkg/wasm/wasm.go b/private/pkg/wasm/wasm.go index 443437740e..5ac1e7bab5 100644 --- a/private/pkg/wasm/wasm.go +++ b/private/pkg/wasm/wasm.go @@ -65,6 +65,10 @@ func WithMaxMemoryBytes(maxMemoryBytes uint32) RuntimeOption { // WithLocalCacheDir sets the local cache directory. // +// The cache directory is used to store compiled Wasm modules. This can be used +// to speed up subsequent runs of the same module. The internal cache structure +// and versioning is handled by the runtime. +// // This option is only safe use in CLI environments. func WithLocalCacheDir(cacheDir string) RuntimeOption { return runtimeOptionFunc(func(cfg *runtimeConfig) { From f0354021a358cd80c6028a085f935812e06d9c8b Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 13:25:11 -0400 Subject: [PATCH 28/48] Fix runtime options --- private/bufpkg/bufcheck/bufcheck.go | 4 +-- private/pkg/wasm/runtime.go | 42 ++++++++++++++--------------- private/pkg/wasm/wasm.go | 18 ++++++------- 3 files changed, 30 insertions(+), 34 deletions(-) diff --git a/private/bufpkg/bufcheck/bufcheck.go b/private/bufpkg/bufcheck/bufcheck.go index 450d0f94d9..7def1b2e5e 100644 --- a/private/bufpkg/bufcheck/bufcheck.go +++ b/private/bufpkg/bufcheck/bufcheck.go @@ -166,7 +166,7 @@ type RunnerProviderFunc func(pluginConfig bufconfig.PluginConfig) (pluginrpc.Run // NewRunner implements RunnerProvider. // -// RunnerProvider selects the correct runner based on the PluginConfig.Type. +// RunnerProvider selects the correct Runner based on the PluginConfig.Type. func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) { return r(pluginConfig) } @@ -176,7 +176,7 @@ func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (plug // This implementation should only be used for local applications. It is safe to // use concurrently. // -// The RunnerProvider selects the correct runner based on the PluginConfig.Type. +// The RunnerProvider selects the correct Runner based on the PluginConfig.Type. // The supported types are: // - bufconfig.PluginConfigTypeLocal // - bufconfig.PluginConfigTypeLocalWasm diff --git a/private/pkg/wasm/runtime.go b/private/pkg/wasm/runtime.go index b97809b026..bf39a6c93c 100644 --- a/private/pkg/wasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -40,31 +40,35 @@ type runtime struct { var _ Runtime = (*runtime)(nil) func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) { - var cfg runtimeConfig - for _, opt := range options { - opt.apply(&cfg) + runtimeOptions := newRuntimeOptions() + for _, option := range options { + option(runtimeOptions) } // Create the runtime config with enforceable limits. runtimeConfig := wazero.NewRuntimeConfig(). WithCoreFeatures(api.CoreFeaturesV2). WithCloseOnContextDone(true). - WithMemoryLimitPages(cfg.getMaxMemoryBytes() / wasmPageSize) + WithMemoryLimitPages(runtimeOptions.getMaxMemoryBytes() / wasmPageSize) var cache wazero.CompilationCache - if cfg.cacheDir != "" { + if runtimeOptions.cacheDir != "" { var err error - cache, err = wazero.NewCompilationCacheWithDir(cfg.cacheDir) + cache, err = wazero.NewCompilationCacheWithDir(runtimeOptions.cacheDir) if err != nil { return nil, fmt.Errorf("failed to create compilation cache: %w", err) } runtimeConfig = runtimeConfig.WithCompilationCache(cache) } - r := wazero.NewRuntimeWithConfig(ctx, runtimeConfig) + wazeroRuntime := wazero.NewRuntimeWithConfig(ctx, runtimeConfig) - // Init wasi. - wasi_snapshot_preview1.MustInstantiate(ctx, r) + // Init WASI preview1 APIs. This is required to support the pluginrpc + // protocol. The closer method is not required as the instantiated + // module is never required to be unloaded. + if _, err := wasi_snapshot_preview1.Instantiate(ctx, wazeroRuntime); err != nil { + return nil, fmt.Errorf("failed to instantiate WASI snapshot preview1: %w", err) + } return &runtime{ - runtime: r, + runtime: wazeroRuntime, cache: cache, }, nil } @@ -99,30 +103,24 @@ func (r *runtime) Release(ctx context.Context) error { return err } -type runtimeConfig struct { +type runtimeOptions struct { maxMemoryBytes uint32 cacheDir string } -func (r *runtimeConfig) getMaxMemoryBytes() uint32 { +func newRuntimeOptions() *runtimeOptions { + return &runtimeOptions{} +} + +func (r *runtimeOptions) getMaxMemoryBytes() uint32 { if r.maxMemoryBytes == 0 { return defaultMaxMemoryBytes } return r.maxMemoryBytes } -type runtimeOptionFunc func(*runtimeConfig) - -func (f runtimeOptionFunc) apply(cfg *runtimeConfig) { - f(cfg) -} - -var _ RuntimeOption = runtimeOptionFunc(nil) - type unimplementedRuntime struct{} -var _ Runtime = unimplementedRuntime{} - func (unimplementedRuntime) Compile(ctx context.Context, name string, moduleBytes []byte) (CompiledModule, error) { return nil, syserror.Newf("not implemented") } diff --git a/private/pkg/wasm/wasm.go b/private/pkg/wasm/wasm.go index 5ac1e7bab5..9325ab07e7 100644 --- a/private/pkg/wasm/wasm.go +++ b/private/pkg/wasm/wasm.go @@ -51,16 +51,14 @@ func NewRuntime(ctx context.Context, options ...RuntimeOption) (Runtime, error) return newRuntime(ctx, options...) } -// RuntimeOption is an option for NewRuntime. -type RuntimeOption interface { - apply(*runtimeConfig) -} +// RuntimeOption is an option for Runtime. +type RuntimeOption func(*runtimeOptions) // WithMaxMemoryBytes sets the maximum memory size in bytes. func WithMaxMemoryBytes(maxMemoryBytes uint32) RuntimeOption { - return runtimeOptionFunc(func(cfg *runtimeConfig) { - cfg.maxMemoryBytes = maxMemoryBytes - }) + return func(runtimeOptions *runtimeOptions) { + runtimeOptions.maxMemoryBytes = maxMemoryBytes + } } // WithLocalCacheDir sets the local cache directory. @@ -71,9 +69,9 @@ func WithMaxMemoryBytes(maxMemoryBytes uint32) RuntimeOption { // // This option is only safe use in CLI environments. func WithLocalCacheDir(cacheDir string) RuntimeOption { - return runtimeOptionFunc(func(cfg *runtimeConfig) { - cfg.cacheDir = cacheDir - }) + return func(runtimeOptions *runtimeOptions) { + runtimeOptions.cacheDir = cacheDir + } } // UnimplementedRuntime is an unimplemented Runtime. From 4e161a6fd9c295ac46387a460d520eba7af69549 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 13:29:59 -0400 Subject: [PATCH 29/48] fix naming to Close --- private/buf/cmd/buf/command/beta/lsp/lsp.go | 2 +- private/buf/cmd/buf/command/breaking/breaking.go | 2 +- .../buf/cmd/buf/command/config/internal/internal.go | 2 +- private/buf/cmd/buf/command/lint/lint.go | 2 +- private/bufpkg/bufcheck/lint_test.go | 2 +- private/pkg/wasm/compiled_module.go | 2 +- private/pkg/wasm/runtime.go | 2 +- private/pkg/wasm/wasm.go | 12 ++++++------ 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/private/buf/cmd/buf/command/beta/lsp/lsp.go b/private/buf/cmd/buf/command/beta/lsp/lsp.go index 855b7f7a57..7ff61d8c5e 100644 --- a/private/buf/cmd/buf/command/beta/lsp/lsp.go +++ b/private/buf/cmd/buf/command/beta/lsp/lsp.go @@ -104,7 +104,7 @@ func run( return err } defer func() { - retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) + retErr = multierr.Append(retErr, wasmRuntime.Close(ctx)) }() checkClient, err := bufcheck.NewClient( container.Logger(), diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index dd23632303..38e042005a 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -217,7 +217,7 @@ func run( return err } defer func() { - retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) + retErr = multierr.Append(retErr, wasmRuntime.Close(ctx)) }() tracer := tracing.NewTracer(container.Tracer()) var allFileAnnotations []bufanalysis.FileAnnotation diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index 692159faeb..897440f3ac 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -195,7 +195,7 @@ func lsRun( return err } defer func() { - retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) + retErr = multierr.Append(retErr, wasmRuntime.Close(ctx)) }() tracer := tracing.NewTracer(container.Tracer()) client, err := bufcheck.NewClient( diff --git a/private/buf/cmd/buf/command/lint/lint.go b/private/buf/cmd/buf/command/lint/lint.go index 4f9893fb0e..371b6e43de 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -142,7 +142,7 @@ func run( return err } defer func() { - retErr = multierr.Append(retErr, wasmRuntime.Release(ctx)) + retErr = multierr.Append(retErr, wasmRuntime.Close(ctx)) }() tracer := tracing.NewTracer(container.Tracer()) var allFileAnnotations []bufanalysis.FileAnnotation diff --git a/private/bufpkg/bufcheck/lint_test.go b/private/bufpkg/bufcheck/lint_test.go index c4e0b7f756..1808e3c896 100644 --- a/private/bufpkg/bufcheck/lint_test.go +++ b/private/bufpkg/bufcheck/lint_test.go @@ -1315,7 +1315,7 @@ func testLintWithOptions( wasmRuntime, err := wasm.NewRuntime(ctx) require.NoError(t, err) t.Cleanup(func() { - require.NoError(t, wasmRuntime.Release(ctx)) + require.NoError(t, wasmRuntime.Close(ctx)) }) client, err := bufcheck.NewClient( zap.NewNop(), diff --git a/private/pkg/wasm/compiled_module.go b/private/pkg/wasm/compiled_module.go index 16dd536173..bc2569b1df 100644 --- a/private/pkg/wasm/compiled_module.go +++ b/private/pkg/wasm/compiled_module.go @@ -57,6 +57,6 @@ func (p *compiledModule) Run(ctx context.Context, env pluginrpc.Env) error { return nil } -func (p *compiledModule) Release(ctx context.Context) error { +func (p *compiledModule) Close(ctx context.Context) error { return p.compiledModule.Close(ctx) } diff --git a/private/pkg/wasm/runtime.go b/private/pkg/wasm/runtime.go index bf39a6c93c..3ae9fa5a5c 100644 --- a/private/pkg/wasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -95,7 +95,7 @@ func (r *runtime) Compile(ctx context.Context, moduleName string, moduleWasm []b }, nil } -func (r *runtime) Release(ctx context.Context) error { +func (r *runtime) Close(ctx context.Context) error { err := r.runtime.Close(ctx) if r.cache != nil { err = multierr.Append(err, r.cache.Close(ctx)) diff --git a/private/pkg/wasm/wasm.go b/private/pkg/wasm/wasm.go index 9325ab07e7..c9996441c6 100644 --- a/private/pkg/wasm/wasm.go +++ b/private/pkg/wasm/wasm.go @@ -23,27 +23,27 @@ import ( // CompiledModule is a Wasm module ready to be run. // -// It is safe to use this module concurrently. When done, call Release to free +// It is safe to use this module concurrently. When done, call Close to free // resources held by the CompiledModule. All CompiledModules created by the // same Runtime will be invalidated when the Runtime is released. // // Memory is limited by the runtime. To restrict CPU usage, cancel the context. type CompiledModule interface { pluginrpc.Runner - // Release releases all resources held by the compiled module. - Release(ctx context.Context) error + // Close releases all resources held by the compiled module. + Close(ctx context.Context) error } // Runtime is a Wasm runtime. // -// It is safe to use the Runtime concurrently. Release must be called when done +// It is safe to use the Runtime concurrently. Close must be called when done // with the Runtime to free resources. All CompiledModules created by the same // Runtime will be invalidated when the Runtime is released. type Runtime interface { // Compile compiles the given Wasm module bytes into a CompiledModule. Compile(ctx context.Context, moduleName string, moduleWasm []byte) (CompiledModule, error) - // Release releases all resources held by the Runtime. - Release(ctx context.Context) error + // Close releases all resources held by the Runtime. + Close(ctx context.Context) error } // NewRuntime creates a new Wasm runtime. From cec3d6df903453202954696e2778edcb9954ae59 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 13:34:39 -0400 Subject: [PATCH 30/48] fix var names --- private/pkg/wasm/compiled_module.go | 10 +++++----- private/pkg/wasm/runtime.go | 23 +++++++++++------------ 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/private/pkg/wasm/compiled_module.go b/private/pkg/wasm/compiled_module.go index bc2569b1df..e6b78e8d81 100644 --- a/private/pkg/wasm/compiled_module.go +++ b/private/pkg/wasm/compiled_module.go @@ -31,27 +31,27 @@ type compiledModule struct { var _ CompiledModule = (*compiledModule)(nil) func (p *compiledModule) Run(ctx context.Context, env pluginrpc.Env) error { - // Create a new module config with the given environment. - config := wazero.NewModuleConfig(). + // Create a new module moduleConfig with the given environment. + moduleConfig := wazero.NewModuleConfig(). WithStdin(env.Stdin). WithStdout(env.Stdout). WithStderr(env.Stderr) // Instantiate the guest wasm module into the same runtime. // See: https://github.com/tetratelabs/wazero/issues/985 - mod, err := p.runtime.InstantiateModule( + wazeroModule, err := p.runtime.InstantiateModule( ctx, p.compiledModule, // Use an empty name to allow for multiple instances of the same module. // See wazero.ModuleConfig.WithName. - config.WithName("").WithArgs( + moduleConfig.WithName("").WithArgs( append([]string{p.moduleName}, env.Args...)..., ), ) if err != nil { return fmt.Errorf("failed to instantiate module: %w", err) } - if err := mod.Close(ctx); err != nil { + if err := wazeroModule.Close(ctx); err != nil { return fmt.Errorf("failed to close module: %w", err) } return nil diff --git a/private/pkg/wasm/runtime.go b/private/pkg/wasm/runtime.go index 3ae9fa5a5c..f765472910 100644 --- a/private/pkg/wasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -33,8 +33,8 @@ const ( ) type runtime struct { - runtime wazero.Runtime - cache wazero.CompilationCache + runtime wazero.Runtime + compilationCache wazero.CompilationCache } var _ Runtime = (*runtime)(nil) @@ -49,27 +49,26 @@ func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) WithCoreFeatures(api.CoreFeaturesV2). WithCloseOnContextDone(true). WithMemoryLimitPages(runtimeOptions.getMaxMemoryBytes() / wasmPageSize) - var cache wazero.CompilationCache + var compilationCache wazero.CompilationCache if runtimeOptions.cacheDir != "" { var err error - cache, err = wazero.NewCompilationCacheWithDir(runtimeOptions.cacheDir) + compilationCache, err = wazero.NewCompilationCacheWithDir(runtimeOptions.cacheDir) if err != nil { return nil, fmt.Errorf("failed to create compilation cache: %w", err) } - runtimeConfig = runtimeConfig.WithCompilationCache(cache) + runtimeConfig = runtimeConfig.WithCompilationCache(compilationCache) } wazeroRuntime := wazero.NewRuntimeWithConfig(ctx, runtimeConfig) // Init WASI preview1 APIs. This is required to support the pluginrpc - // protocol. The closer method is not required as the instantiated - // module is never required to be unloaded. + // protocol. The returned closer method is discarded as the + // instantiated module is never required to be unloaded. if _, err := wasi_snapshot_preview1.Instantiate(ctx, wazeroRuntime); err != nil { return nil, fmt.Errorf("failed to instantiate WASI snapshot preview1: %w", err) } - return &runtime{ - runtime: wazeroRuntime, - cache: cache, + runtime: wazeroRuntime, + compilationCache: compilationCache, }, nil } @@ -97,8 +96,8 @@ func (r *runtime) Compile(ctx context.Context, moduleName string, moduleWasm []b func (r *runtime) Close(ctx context.Context) error { err := r.runtime.Close(ctx) - if r.cache != nil { - err = multierr.Append(err, r.cache.Close(ctx)) + if r.compilationCache != nil { + err = multierr.Append(err, r.compilationCache.Close(ctx)) } return err } From 64764ccf2ddc41f1bb99249cf4dc251ea5b85f7c Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 13:42:25 -0400 Subject: [PATCH 31/48] fix unimplemented --- private/pkg/wasm/compiled_module.go | 2 -- private/pkg/wasm/runtime.go | 8 +++----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/private/pkg/wasm/compiled_module.go b/private/pkg/wasm/compiled_module.go index e6b78e8d81..071944b422 100644 --- a/private/pkg/wasm/compiled_module.go +++ b/private/pkg/wasm/compiled_module.go @@ -28,8 +28,6 @@ type compiledModule struct { compiledModule wazero.CompiledModule } -var _ CompiledModule = (*compiledModule)(nil) - func (p *compiledModule) Run(ctx context.Context, env pluginrpc.Env) error { // Create a new module moduleConfig with the given environment. moduleConfig := wazero.NewModuleConfig(). diff --git a/private/pkg/wasm/runtime.go b/private/pkg/wasm/runtime.go index f765472910..aebe71b914 100644 --- a/private/pkg/wasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -37,8 +37,6 @@ type runtime struct { compilationCache wazero.CompilationCache } -var _ Runtime = (*runtime)(nil) - func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) { runtimeOptions := newRuntimeOptions() for _, option := range options { @@ -83,14 +81,14 @@ func (r *runtime) Compile(ctx context.Context, moduleName string, moduleWasm []b // bytes and the runtime configuration. The compiled module is // cached in memory and on disk if an optional cache directory is // provided. - compiledModulePlugin, err := r.runtime.CompileModule(ctx, moduleWasm) + wazeroCompiledModule, err := r.runtime.CompileModule(ctx, moduleWasm) if err != nil { return nil, err } return &compiledModule{ moduleName: moduleName, runtime: r.runtime, - compiledModule: compiledModulePlugin, + compiledModule: wazeroCompiledModule, }, nil } @@ -123,6 +121,6 @@ type unimplementedRuntime struct{} func (unimplementedRuntime) Compile(ctx context.Context, name string, moduleBytes []byte) (CompiledModule, error) { return nil, syserror.Newf("not implemented") } -func (unimplementedRuntime) Release(ctx context.Context) error { +func (unimplementedRuntime) Close(ctx context.Context) error { return nil } From 75fe87e21d5afe96badc39691d2285c7c9fbdde2 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 13:42:35 -0400 Subject: [PATCH 32/48] fix changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4043ba8716..937dc9f37e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,8 @@ ## [Unreleased] -- Add support for custom WebAssembly plugins for lint and breaking changes plugins. Use the `.wasm` file extension to specify a WebAssembly plugin. +- Add support for a WebAssembly (Wasm) runtime for custom lint and breaking changes plugins. Use the + `.wasm` file extension to specify a path to a Wasm plugin. ## [v1.43.0] - 2024-09-30 From a5e1c4329dd776cded6e9c07d5e65d60f8e08e68 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 14:23:37 -0400 Subject: [PATCH 33/48] fix commandRunner naming --- private/bufpkg/bufcheck/bufcheck.go | 4 ++-- private/bufpkg/bufcheck/runner_provider.go | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/private/bufpkg/bufcheck/bufcheck.go b/private/bufpkg/bufcheck/bufcheck.go index 7def1b2e5e..3ffdbac282 100644 --- a/private/bufpkg/bufcheck/bufcheck.go +++ b/private/bufpkg/bufcheck/bufcheck.go @@ -182,8 +182,8 @@ func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (plug // - bufconfig.PluginConfigTypeLocalWasm // // If the PluginConfig.Type is not supported, an error is returned. -func NewRunnerProvider(delegate command.Runner, wasmRuntime wasm.Runtime) RunnerProvider { - return newRunnerProvider(delegate, wasmRuntime) +func NewRunnerProvider(commandRunner command.Runner, wasmRuntime wasm.Runtime) RunnerProvider { + return newRunnerProvider(commandRunner, wasmRuntime) } // NewClient returns a new Client. diff --git a/private/bufpkg/bufcheck/runner_provider.go b/private/bufpkg/bufcheck/runner_provider.go index 6f77a826ef..e5788c9017 100644 --- a/private/bufpkg/bufcheck/runner_provider.go +++ b/private/bufpkg/bufcheck/runner_provider.go @@ -24,14 +24,14 @@ import ( ) type runnerProvider struct { - delegate command.Runner - wasmRuntime wasm.Runtime + commandRunner command.Runner + wasmRuntime wasm.Runtime } -func newRunnerProvider(delegate command.Runner, wasmRuntime wasm.Runtime) *runnerProvider { +func newRunnerProvider(commandRunner command.Runner, wasmRuntime wasm.Runtime) *runnerProvider { return &runnerProvider{ - delegate: delegate, - wasmRuntime: wasmRuntime, + commandRunner: commandRunner, + wasmRuntime: wasmRuntime, } } @@ -40,7 +40,7 @@ func (r *runnerProvider) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginr case bufconfig.PluginConfigTypeLocal: path := pluginConfig.Path() return pluginrpcutil.NewRunner( - r.delegate, + r.commandRunner, // We know that Path is of at least length 1. path[0], path[1:]..., From 799658a1218e3a1ca71696c248e76a2faaccc026 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 14:30:12 -0400 Subject: [PATCH 34/48] fix delegate --- private/buf/cmd/buf/command/mod/internal/internal.go | 3 ++- private/pkg/pluginrpcutil/pluginrpcutil.go | 4 ++-- private/pkg/pluginrpcutil/wasm_runner.go | 10 +++++----- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/private/buf/cmd/buf/command/mod/internal/internal.go b/private/buf/cmd/buf/command/mod/internal/internal.go index d7f7c71ba1..1247554d19 100644 --- a/private/buf/cmd/buf/command/mod/internal/internal.go +++ b/private/buf/cmd/buf/command/mod/internal/internal.go @@ -177,7 +177,8 @@ func lsRun( // BufYAMLFiles <=v1 never had plugins. tracer := tracing.NewTracer(container.Tracer()) client, err := bufcheck.NewClient( - container.Logger(), tracer, + container.Logger(), + tracer, bufcheck.NewRunnerProvider(command.NewRunner(), wasm.UnimplementedRuntime), bufcheck.ClientWithStderr(container.Stderr()), ) diff --git a/private/pkg/pluginrpcutil/pluginrpcutil.go b/private/pkg/pluginrpcutil/pluginrpcutil.go index 99647188f9..ef50a20146 100644 --- a/private/pkg/pluginrpcutil/pluginrpcutil.go +++ b/private/pkg/pluginrpcutil/pluginrpcutil.go @@ -28,6 +28,6 @@ func NewRunner(delegate command.Runner, programName string, programArgs ...strin // NewWasmRunner returns a new pluginrpc.Runner for the wasm.Runtime and program name. // // This runner is used for local Wasm plugins. The program name is the path to the Wasm file. -func NewWasmRunner(wasmRuntime wasm.Runtime, programName string, programArgs ...string) pluginrpc.Runner { - return newWasmRunner(wasmRuntime, programName, programArgs...) +func NewWasmRunner(delegate wasm.Runtime, programName string, programArgs ...string) pluginrpc.Runner { + return newWasmRunner(delegate, programName, programArgs...) } diff --git a/private/pkg/pluginrpcutil/wasm_runner.go b/private/pkg/pluginrpcutil/wasm_runner.go index 54803b1a57..5fb8fdbb1d 100644 --- a/private/pkg/pluginrpcutil/wasm_runner.go +++ b/private/pkg/pluginrpcutil/wasm_runner.go @@ -28,7 +28,7 @@ import ( ) type wasmRunner struct { - wasmRuntime wasm.Runtime + delegate wasm.Runtime programName string programArgs []string // lock protects compiledModule and compiledModuleErr. Store called as @@ -40,12 +40,12 @@ type wasmRunner struct { } func newWasmRunner( - runtime wasm.Runtime, + delegate wasm.Runtime, programName string, programArgs ...string, ) *wasmRunner { return &wasmRunner{ - wasmRuntime: runtime, + delegate: delegate, programName: programName, programArgs: programArgs, } @@ -94,12 +94,12 @@ func (r *wasmRunner) loadCompiledModule(ctx context.Context) (wasm.CompiledModul } moduleWasm, err := os.ReadFile(path) if err != nil { - return nil, err + return nil, fmt.Errorf("could not read plugin %q: %v", r.programName, err) } // Compile the module. This plugin is never released, so subsequent // calls to this function will benefit from the cached plugin. This is // only safe as the runner is limited to the CLI. - compiledModule, err := r.wasmRuntime.Compile(ctx, r.programName, moduleWasm) + compiledModule, err := r.delegate.Compile(ctx, r.programName, moduleWasm) if err != nil { return nil, err } From 41705f97b0557aef7c373cb0c67d38fe348eaf1f Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 14:59:26 -0400 Subject: [PATCH 35/48] clarify args --- private/pkg/wasm/compiled_module.go | 18 ++++++++++-------- private/pkg/wasm/runtime.go | 12 ++++++------ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/private/pkg/wasm/compiled_module.go b/private/pkg/wasm/compiled_module.go index 071944b422..babadea21d 100644 --- a/private/pkg/wasm/compiled_module.go +++ b/private/pkg/wasm/compiled_module.go @@ -29,22 +29,24 @@ type compiledModule struct { } func (p *compiledModule) Run(ctx context.Context, env pluginrpc.Env) error { - // Create a new module moduleConfig with the given environment. - moduleConfig := wazero.NewModuleConfig(). + // Create a new module wazeroModuleConfig with the given environment. + wazeroModuleConfig := wazero.NewModuleConfig(). WithStdin(env.Stdin). WithStdout(env.Stdout). - WithStderr(env.Stderr) + WithStderr(env.Stderr). + // Use an empty name to allow for multiple instances of the same module. + // See wazero.ModuleConfig.WithName. + WithName(""). + // Use the program name as the first argument to replicate the + // behavior of os.Exec. See wazero.ModuleConfig.WithArgs. + WithArgs(append([]string{p.moduleName}, env.Args...)...) // Instantiate the guest wasm module into the same runtime. // See: https://github.com/tetratelabs/wazero/issues/985 wazeroModule, err := p.runtime.InstantiateModule( ctx, p.compiledModule, - // Use an empty name to allow for multiple instances of the same module. - // See wazero.ModuleConfig.WithName. - moduleConfig.WithName("").WithArgs( - append([]string{p.moduleName}, env.Args...)..., - ), + wazeroModuleConfig, ) if err != nil { return fmt.Errorf("failed to instantiate module: %w", err) diff --git a/private/pkg/wasm/runtime.go b/private/pkg/wasm/runtime.go index aebe71b914..7c5f4d889c 100644 --- a/private/pkg/wasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -43,20 +43,20 @@ func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) option(runtimeOptions) } // Create the runtime config with enforceable limits. - runtimeConfig := wazero.NewRuntimeConfig(). + wazeroRuntimeConfig := wazero.NewRuntimeConfig(). WithCoreFeatures(api.CoreFeaturesV2). WithCloseOnContextDone(true). WithMemoryLimitPages(runtimeOptions.getMaxMemoryBytes() / wasmPageSize) - var compilationCache wazero.CompilationCache + var wazeroCompilationCache wazero.CompilationCache if runtimeOptions.cacheDir != "" { var err error - compilationCache, err = wazero.NewCompilationCacheWithDir(runtimeOptions.cacheDir) + wazeroCompilationCache, err = wazero.NewCompilationCacheWithDir(runtimeOptions.cacheDir) if err != nil { return nil, fmt.Errorf("failed to create compilation cache: %w", err) } - runtimeConfig = runtimeConfig.WithCompilationCache(compilationCache) + wazeroRuntimeConfig = wazeroRuntimeConfig.WithCompilationCache(wazeroCompilationCache) } - wazeroRuntime := wazero.NewRuntimeWithConfig(ctx, runtimeConfig) + wazeroRuntime := wazero.NewRuntimeWithConfig(ctx, wazeroRuntimeConfig) // Init WASI preview1 APIs. This is required to support the pluginrpc // protocol. The returned closer method is discarded as the @@ -66,7 +66,7 @@ func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) } return &runtime{ runtime: wazeroRuntime, - compilationCache: compilationCache, + compilationCache: wazeroCompilationCache, }, nil } From 4bfc10c4ec3b214c50494091c29f6dac5e71d75b Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 15:10:00 -0400 Subject: [PATCH 36/48] use filepath.Ext --- private/bufpkg/bufconfig/plugin_config.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/private/bufpkg/bufconfig/plugin_config.go b/private/bufpkg/bufconfig/plugin_config.go index fd9f8e39b2..8e8394c605 100644 --- a/private/bufpkg/bufconfig/plugin_config.go +++ b/private/bufpkg/bufconfig/plugin_config.go @@ -17,6 +17,7 @@ package bufconfig import ( "errors" "fmt" + "path/filepath" "strings" "github.com/bufbuild/buf/private/pkg/encoding" @@ -115,7 +116,7 @@ func newPluginConfigForExternalV2( return nil, errors.New("must specify a path to the plugin") } // Wasm plugins are suffixed with .wasm. Otherwise, it's a binary. - if strings.HasSuffix(path[0], ".wasm") { + if filepath.Ext(path[0]) == ".wasm" { return newLocalWasmPluginConfig( strings.Join(path, " "), options, From d09ddbfa09316b89c3c82160b5836a6da134c4bd Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 15:15:04 -0400 Subject: [PATCH 37/48] expand doc on limits --- private/pkg/wasm/runtime.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/private/pkg/wasm/runtime.go b/private/pkg/wasm/runtime.go index 7c5f4d889c..b968a77e67 100644 --- a/private/pkg/wasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -42,7 +42,12 @@ func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) for _, option := range options { option(runtimeOptions) } - // Create the runtime config with enforceable limits. + // Create the wazero.RuntimeConfig with enforceable limits. Limits are + // enforced through the two mechanisms: + // - Memory limit: The maximum memory size in pages. + // - Close on context done: The runtime is closed when the context is + // done. + // See wazero.NewRuntimeConfig for more details. wazeroRuntimeConfig := wazero.NewRuntimeConfig(). WithCoreFeatures(api.CoreFeaturesV2). WithCloseOnContextDone(true). From 2fc5bf690eb0874a49d8d4de715fdd62d5b2ec1b Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 15:27:01 -0400 Subject: [PATCH 38/48] doc instantiation --- private/pkg/wasm/compiled_module.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/private/pkg/wasm/compiled_module.go b/private/pkg/wasm/compiled_module.go index babadea21d..43f6641e6d 100644 --- a/private/pkg/wasm/compiled_module.go +++ b/private/pkg/wasm/compiled_module.go @@ -41,7 +41,9 @@ func (p *compiledModule) Run(ctx context.Context, env pluginrpc.Env) error { // behavior of os.Exec. See wazero.ModuleConfig.WithArgs. WithArgs(append([]string{p.moduleName}, env.Args...)...) - // Instantiate the guest wasm module into the same runtime. + // Instantiate the Wasm module into the runtime. This effectively runs + // the Wasm module. Only the effect of instantiating the module is used, + // the module is closed immediately after running to free up resources. // See: https://github.com/tetratelabs/wazero/issues/985 wazeroModule, err := p.runtime.InstantiateModule( ctx, From 1415eea0ff529e077af62d9d8823229b48eafab2 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 15:38:15 -0400 Subject: [PATCH 39/48] fix see docs --- private/bufpkg/bufconfig/plugin_config.go | 2 +- private/pkg/wasm/compiled_module.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/private/bufpkg/bufconfig/plugin_config.go b/private/bufpkg/bufconfig/plugin_config.go index 8e8394c605..388bb15b50 100644 --- a/private/bufpkg/bufconfig/plugin_config.go +++ b/private/bufpkg/bufconfig/plugin_config.go @@ -154,7 +154,7 @@ func newLocalWasmPluginConfig( if len(path) == 0 { return nil, errors.New("must specify a path to the plugin") } - if !strings.HasSuffix(path[0], ".wasm") { + if filepath.Ext(path[0]) != ".wasm" { return nil, fmt.Errorf("must specify a path to the plugin, and the first path argument must end with .wasm") } return &pluginConfig{ diff --git a/private/pkg/wasm/compiled_module.go b/private/pkg/wasm/compiled_module.go index 43f6641e6d..84766013df 100644 --- a/private/pkg/wasm/compiled_module.go +++ b/private/pkg/wasm/compiled_module.go @@ -38,13 +38,14 @@ func (p *compiledModule) Run(ctx context.Context, env pluginrpc.Env) error { // See wazero.ModuleConfig.WithName. WithName(""). // Use the program name as the first argument to replicate the - // behavior of os.Exec. See wazero.ModuleConfig.WithArgs. + // behavior of os.Exec. + // See wazero.ModuleConfig.WithArgs. WithArgs(append([]string{p.moduleName}, env.Args...)...) // Instantiate the Wasm module into the runtime. This effectively runs // the Wasm module. Only the effect of instantiating the module is used, // the module is closed immediately after running to free up resources. - // See: https://github.com/tetratelabs/wazero/issues/985 + // See https://github.com/tetratelabs/wazero/issues/985. wazeroModule, err := p.runtime.InstantiateModule( ctx, p.compiledModule, From dc11fe5fc3d0ac9d5cf1d3a22a897529431986c1 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 16:52:42 -0400 Subject: [PATCH 40/48] doc memory page limits --- private/pkg/wasm/runtime.go | 22 ++++++++++++++-------- private/pkg/wasm/wasm.go | 3 +++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/private/pkg/wasm/runtime.go b/private/pkg/wasm/runtime.go index b968a77e67..cda4cdfce9 100644 --- a/private/pkg/wasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -42,6 +42,17 @@ func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) for _, option := range options { option(runtimeOptions) } + if runtimeOptions.maxMemoryBytes == 0 { + return nil, fmt.Errorf("Wasm max memory bytes must be greater than 0") + } + // The maximum memory size is limited to 4 GiB. Sizes less than the page + // size (64 KiB) are truncated. memoryLimitPages is guaranteed to be + // below 2^16 as the maxium uint32 value is 2^32 - 1. + memoryLimitPages := runtimeOptions.maxMemoryBytes / wasmPageSize + if memoryLimitPages == 0 { + return nil, fmt.Errorf("Wasm max memory bytes %d is too small", runtimeOptions.maxMemoryBytes) + } + // Create the wazero.RuntimeConfig with enforceable limits. Limits are // enforced through the two mechanisms: // - Memory limit: The maximum memory size in pages. @@ -51,7 +62,7 @@ func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) wazeroRuntimeConfig := wazero.NewRuntimeConfig(). WithCoreFeatures(api.CoreFeaturesV2). WithCloseOnContextDone(true). - WithMemoryLimitPages(runtimeOptions.getMaxMemoryBytes() / wasmPageSize) + WithMemoryLimitPages(memoryLimitPages) var wazeroCompilationCache wazero.CompilationCache if runtimeOptions.cacheDir != "" { var err error @@ -111,14 +122,9 @@ type runtimeOptions struct { } func newRuntimeOptions() *runtimeOptions { - return &runtimeOptions{} -} - -func (r *runtimeOptions) getMaxMemoryBytes() uint32 { - if r.maxMemoryBytes == 0 { - return defaultMaxMemoryBytes + return &runtimeOptions{ + maxMemoryBytes: defaultMaxMemoryBytes, } - return r.maxMemoryBytes } type unimplementedRuntime struct{} diff --git a/private/pkg/wasm/wasm.go b/private/pkg/wasm/wasm.go index c9996441c6..7dd44abd24 100644 --- a/private/pkg/wasm/wasm.go +++ b/private/pkg/wasm/wasm.go @@ -55,6 +55,9 @@ func NewRuntime(ctx context.Context, options ...RuntimeOption) (Runtime, error) type RuntimeOption func(*runtimeOptions) // WithMaxMemoryBytes sets the maximum memory size in bytes. +// +// The maximuim memory size is limited to 4 GiB. The default is 512 MiB. Sizes +// less then the page size (64 KiB) are truncated. func WithMaxMemoryBytes(maxMemoryBytes uint32) RuntimeOption { return func(runtimeOptions *runtimeOptions) { runtimeOptions.maxMemoryBytes = maxMemoryBytes From 086a6ce820e14de03d261923640068dd01a9335c Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 16:57:59 -0400 Subject: [PATCH 41/48] note about size restriction --- private/pkg/wasm/runtime.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/private/pkg/wasm/runtime.go b/private/pkg/wasm/runtime.go index cda4cdfce9..8b69b37363 100644 --- a/private/pkg/wasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -48,6 +48,10 @@ func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) // The maximum memory size is limited to 4 GiB. Sizes less than the page // size (64 KiB) are truncated. memoryLimitPages is guaranteed to be // below 2^16 as the maxium uint32 value is 2^32 - 1. + // NOTE: The option represented as a uint32 bytes value restricts the + // max number of pages to 2^16-1, one less the the actual max value of + // 2^16. But this is a nicer API then specifying the max number of + // pages directly. memoryLimitPages := runtimeOptions.maxMemoryBytes / wasmPageSize if memoryLimitPages == 0 { return nil, fmt.Errorf("Wasm max memory bytes %d is too small", runtimeOptions.maxMemoryBytes) From 6ab5dcb05ca1ed9caeaea895c139938355a4a32b Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 16:59:00 -0400 Subject: [PATCH 42/48] fix --- private/pkg/wasm/runtime.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/private/pkg/wasm/runtime.go b/private/pkg/wasm/runtime.go index 8b69b37363..5f6d99aa2c 100644 --- a/private/pkg/wasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -48,10 +48,9 @@ func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) // The maximum memory size is limited to 4 GiB. Sizes less than the page // size (64 KiB) are truncated. memoryLimitPages is guaranteed to be // below 2^16 as the maxium uint32 value is 2^32 - 1. - // NOTE: The option represented as a uint32 bytes value restricts the - // max number of pages to 2^16-1, one less the the actual max value of - // 2^16. But this is a nicer API then specifying the max number of - // pages directly. + // NOTE: The option represented as a uint32 restricts the max number of + // pages to 2^16-1, one less the the actual max value of 2^16. But this + // is a nicer API then specifying the max number of pages directly. memoryLimitPages := runtimeOptions.maxMemoryBytes / wasmPageSize if memoryLimitPages == 0 { return nil, fmt.Errorf("Wasm max memory bytes %d is too small", runtimeOptions.maxMemoryBytes) From 5e8e9d9aeaa90073fae76c7139af22e55b66272a Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 17:11:04 -0400 Subject: [PATCH 43/48] doc limits --- private/pkg/wasm/runtime.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/private/pkg/wasm/runtime.go b/private/pkg/wasm/runtime.go index 5f6d99aa2c..0f740b436d 100644 --- a/private/pkg/wasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -57,10 +57,11 @@ func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) } // Create the wazero.RuntimeConfig with enforceable limits. Limits are - // enforced through the two mechanisms: + // enforced through the Wasm sandbox. The following limits are set: // - Memory limit: The maximum memory size in pages. - // - Close on context done: The runtime is closed when the context is - // done. + // - CPU limit: The runtime stops work on context done. + // - Access limit: All system interfaces are stubbed. No network, + // disk, etc. // See wazero.NewRuntimeConfig for more details. wazeroRuntimeConfig := wazero.NewRuntimeConfig(). WithCoreFeatures(api.CoreFeaturesV2). From 1866ce1774695c3beb7ba9550e6bb172a9c650d0 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 17:19:12 -0400 Subject: [PATCH 44/48] fix Runtime --- private/pkg/wasm/wasm.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/private/pkg/wasm/wasm.go b/private/pkg/wasm/wasm.go index 7dd44abd24..fb3765a534 100644 --- a/private/pkg/wasm/wasm.go +++ b/private/pkg/wasm/wasm.go @@ -27,7 +27,7 @@ import ( // resources held by the CompiledModule. All CompiledModules created by the // same Runtime will be invalidated when the Runtime is released. // -// Memory is limited by the runtime. To restrict CPU usage, cancel the context. +// Memory is limited by the Runtime. To restrict CPU usage, cancel the context. type CompiledModule interface { pluginrpc.Runner // Close releases all resources held by the compiled module. @@ -46,7 +46,7 @@ type Runtime interface { Close(ctx context.Context) error } -// NewRuntime creates a new Wasm runtime. +// NewRuntime creates a new Wasm Runtime. func NewRuntime(ctx context.Context, options ...RuntimeOption) (Runtime, error) { return newRuntime(ctx, options...) } @@ -68,7 +68,7 @@ func WithMaxMemoryBytes(maxMemoryBytes uint32) RuntimeOption { // // The cache directory is used to store compiled Wasm modules. This can be used // to speed up subsequent runs of the same module. The internal cache structure -// and versioning is handled by the runtime. +// and versioning is handled by the Runtime. // // This option is only safe use in CLI environments. func WithLocalCacheDir(cacheDir string) RuntimeOption { From ef0d8a80f923840b6b22adad1f236556008f2d60 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 17:20:26 -0400 Subject: [PATCH 45/48] fix closed --- private/pkg/wasm/wasm.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/private/pkg/wasm/wasm.go b/private/pkg/wasm/wasm.go index fb3765a534..f33c0a6b4c 100644 --- a/private/pkg/wasm/wasm.go +++ b/private/pkg/wasm/wasm.go @@ -25,7 +25,7 @@ import ( // // It is safe to use this module concurrently. When done, call Close to free // resources held by the CompiledModule. All CompiledModules created by the -// same Runtime will be invalidated when the Runtime is released. +// same Runtime will be invalidated when the Runtime is closed. // // Memory is limited by the Runtime. To restrict CPU usage, cancel the context. type CompiledModule interface { @@ -38,7 +38,7 @@ type CompiledModule interface { // // It is safe to use the Runtime concurrently. Close must be called when done // with the Runtime to free resources. All CompiledModules created by the same -// Runtime will be invalidated when the Runtime is released. +// Runtime will be invalidated when the Runtime is closed. type Runtime interface { // Compile compiles the given Wasm module bytes into a CompiledModule. Compile(ctx context.Context, moduleName string, moduleWasm []byte) (CompiledModule, error) From 6c4030ae210bf7048526b5e8422f6c26212d8676 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 2 Oct 2024 17:36:54 -0400 Subject: [PATCH 46/48] fix doc --- private/bufpkg/bufcheck/bufcheck.go | 6 +++--- private/pkg/pluginrpcutil/wasm_runner.go | 6 +++--- private/pkg/wasm/runtime.go | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/private/bufpkg/bufcheck/bufcheck.go b/private/bufpkg/bufcheck/bufcheck.go index 3ffdbac282..7bfe732dde 100644 --- a/private/bufpkg/bufcheck/bufcheck.go +++ b/private/bufpkg/bufcheck/bufcheck.go @@ -166,7 +166,7 @@ type RunnerProviderFunc func(pluginConfig bufconfig.PluginConfig) (pluginrpc.Run // NewRunner implements RunnerProvider. // -// RunnerProvider selects the correct Runner based on the PluginConfig.Type. +// RunnerProvider selects the correct Runner based on the type of pluginConfig. func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) { return r(pluginConfig) } @@ -176,12 +176,12 @@ func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (plug // This implementation should only be used for local applications. It is safe to // use concurrently. // -// The RunnerProvider selects the correct Runner based on the PluginConfig.Type. +// The RunnerProvider selects the correct Runner based on the PluginConfigType. // The supported types are: // - bufconfig.PluginConfigTypeLocal // - bufconfig.PluginConfigTypeLocalWasm // -// If the PluginConfig.Type is not supported, an error is returned. +// If the PluginConfigType is not supported, an error is returned. func NewRunnerProvider(commandRunner command.Runner, wasmRuntime wasm.Runtime) RunnerProvider { return newRunnerProvider(commandRunner, wasmRuntime) } diff --git a/private/pkg/pluginrpcutil/wasm_runner.go b/private/pkg/pluginrpcutil/wasm_runner.go index 5fb8fdbb1d..0c7640ab41 100644 --- a/private/pkg/pluginrpcutil/wasm_runner.go +++ b/private/pkg/pluginrpcutil/wasm_runner.go @@ -96,9 +96,9 @@ func (r *wasmRunner) loadCompiledModule(ctx context.Context) (wasm.CompiledModul if err != nil { return nil, fmt.Errorf("could not read plugin %q: %v", r.programName, err) } - // Compile the module. This plugin is never released, so subsequent - // calls to this function will benefit from the cached plugin. This is - // only safe as the runner is limited to the CLI. + // Compile the module. This CompiledModule is never released, so + // subsequent calls to this function will benefit from the cached + // module. This is only safe as the runner is limited to the CLI. compiledModule, err := r.delegate.Compile(ctx, r.programName, moduleWasm) if err != nil { return nil, err diff --git a/private/pkg/wasm/runtime.go b/private/pkg/wasm/runtime.go index 0f740b436d..b5eb0dd052 100644 --- a/private/pkg/wasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -61,7 +61,7 @@ func newRuntime(ctx context.Context, options ...RuntimeOption) (*runtime, error) // - Memory limit: The maximum memory size in pages. // - CPU limit: The runtime stops work on context done. // - Access limit: All system interfaces are stubbed. No network, - // disk, etc. + // disk, clock, etc. // See wazero.NewRuntimeConfig for more details. wazeroRuntimeConfig := wazero.NewRuntimeConfig(). WithCoreFeatures(api.CoreFeaturesV2). From bf634a5c568b6e7dccd61cb7e87e24db64652346 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Thu, 3 Oct 2024 10:20:56 -0400 Subject: [PATCH 47/48] move unimplemented runtime --- private/pkg/wasm/runtime.go | 10 -------- private/pkg/wasm/unimplemented_runtime.go | 31 +++++++++++++++++++++++ 2 files changed, 31 insertions(+), 10 deletions(-) create mode 100644 private/pkg/wasm/unimplemented_runtime.go diff --git a/private/pkg/wasm/runtime.go b/private/pkg/wasm/runtime.go index b5eb0dd052..d80178847f 100644 --- a/private/pkg/wasm/runtime.go +++ b/private/pkg/wasm/runtime.go @@ -18,7 +18,6 @@ import ( "context" "fmt" - "github.com/bufbuild/buf/private/pkg/syserror" "github.com/tetratelabs/wazero" "github.com/tetratelabs/wazero/api" "github.com/tetratelabs/wazero/imports/wasi_snapshot_preview1" @@ -130,12 +129,3 @@ func newRuntimeOptions() *runtimeOptions { maxMemoryBytes: defaultMaxMemoryBytes, } } - -type unimplementedRuntime struct{} - -func (unimplementedRuntime) Compile(ctx context.Context, name string, moduleBytes []byte) (CompiledModule, error) { - return nil, syserror.Newf("not implemented") -} -func (unimplementedRuntime) Close(ctx context.Context) error { - return nil -} diff --git a/private/pkg/wasm/unimplemented_runtime.go b/private/pkg/wasm/unimplemented_runtime.go new file mode 100644 index 0000000000..14be4975f2 --- /dev/null +++ b/private/pkg/wasm/unimplemented_runtime.go @@ -0,0 +1,31 @@ +// 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 wasm + +import ( + "context" + + "github.com/bufbuild/buf/private/pkg/syserror" +) + +type unimplementedRuntime struct{} + +func (unimplementedRuntime) Compile(ctx context.Context, name string, moduleBytes []byte) (CompiledModule, error) { + return nil, syserror.Newf("not implemented") +} + +func (unimplementedRuntime) Close(ctx context.Context) error { + return nil +} From 423e0444fe08cd56a96b425f7f357b16d2981eed Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Thu, 3 Oct 2024 10:40:40 -0400 Subject: [PATCH 48/48] fix global var position --- private/pkg/wasm/wasm.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/private/pkg/wasm/wasm.go b/private/pkg/wasm/wasm.go index f33c0a6b4c..5275a0a432 100644 --- a/private/pkg/wasm/wasm.go +++ b/private/pkg/wasm/wasm.go @@ -21,6 +21,9 @@ import ( "pluginrpc.com/pluginrpc" ) +// UnimplementedRuntime is an unimplemented Runtime. +var UnimplementedRuntime = unimplementedRuntime{} + // CompiledModule is a Wasm module ready to be run. // // It is safe to use this module concurrently. When done, call Close to free @@ -76,6 +79,3 @@ func WithLocalCacheDir(cacheDir string) RuntimeOption { runtimeOptions.cacheDir = cacheDir } } - -// UnimplementedRuntime is an unimplemented Runtime. -var UnimplementedRuntime = unimplementedRuntime{}