From cbbaf024e4a41f753777f21b61e2f344f5390c65 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 17 Mar 2026 21:07:09 +0100 Subject: [PATCH] Use overlay bucket on buf.lock updates --- .../command/dep/depupdate/depupdate.go | 82 +++++++++++++++---- .../plugin/pluginupdate/pluginupdate.go | 21 +---- .../policy/policyupdate/policyupdate.go | 23 +----- private/buf/bufctl/controller.go | 7 +- private/buf/bufctl/option.go | 13 +++ 5 files changed, 89 insertions(+), 57 deletions(-) diff --git a/cmd/buf/internal/command/dep/depupdate/depupdate.go b/cmd/buf/internal/command/dep/depupdate/depupdate.go index 20dc2c428f..bdd8ed2c72 100644 --- a/cmd/buf/internal/command/dep/depupdate/depupdate.go +++ b/cmd/buf/internal/command/dep/depupdate/depupdate.go @@ -16,7 +16,6 @@ package depupdate import ( "context" - "errors" "fmt" "log/slog" @@ -26,7 +25,11 @@ import ( "github.com/bufbuild/buf/cmd/buf/internal/command/dep/internal" "github.com/bufbuild/buf/private/buf/bufcli" "github.com/bufbuild/buf/private/buf/bufctl" + "github.com/bufbuild/buf/private/bufpkg/bufconfig" "github.com/bufbuild/buf/private/bufpkg/bufmodule" + "github.com/bufbuild/buf/private/bufpkg/bufplugin" + "github.com/bufbuild/buf/private/bufpkg/bufpolicy" + "github.com/bufbuild/buf/private/pkg/storage/storagemem" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/spf13/pflag" ) @@ -87,7 +90,7 @@ func run( ctx context.Context, container appext.Container, flags *flags, -) (retErr error) { +) error { dirPath := "." if container.NumArgs() > 0 { dirPath = container.Arg(0) @@ -125,7 +128,6 @@ func run( slog.Any("deps", xslices.Map(configuredDepModuleKeys, bufmodule.ModuleKey.String)), ) - // Store the existing buf.lock data. existingDepModuleKeys, err := workspaceDepManager.ExistingBufLockFileDepModuleKeys(ctx) if err != nil { return err @@ -152,22 +154,29 @@ func run( return err } - // We're about to edit the buf.lock file on disk. If we have a subsequent error, - // attempt to revert the buf.lock file. - // - // TODO FUTURE: We should be able to update the buf.lock file in an in-memory bucket, then do the rebuild, - // and if the rebuild is successful, then actually write to disk. It shouldn't even be that much work - just - // overlay the new buf.lock file in a union bucket. - defer func() { - if retErr != nil { - retErr = errors.Join(retErr, workspaceDepManager.UpdateBufLockFile(ctx, existingDepModuleKeys, existingRemotePluginKeys, existingRemotePolicyKeys, existingPolicyNameToRemotePluginKeys)) - } - }() - // Edit the buf.lock file with the unpruned dependencies. - if err := workspaceDepManager.UpdateBufLockFile(ctx, configuredDepModuleKeys, existingRemotePluginKeys, existingRemotePolicyKeys, existingPolicyNameToRemotePluginKeys); err != nil { + // Write the updated buf.lock to an in-memory bucket and overlay it on top of + // the workspace bucket for validation. Only persist to disk after the workspace + // builds successfully. + overlayBucket := storagemem.NewReadWriteBucket() + bufLockFile, err := newBufLockFile( + workspaceDepManager.BufLockFileDigestType(), + configuredDepModuleKeys, + existingRemotePluginKeys, + existingRemotePolicyKeys, + existingPolicyNameToRemotePluginKeys, + ) + if err != nil { return err } - workspace, err := controller.GetWorkspace(ctx, dirPath, bufctl.WithIgnoreAndDisallowV1BufWorkYAMLs()) + if err := bufconfig.PutBufLockFileForPrefix(ctx, overlayBucket, ".", bufLockFile); err != nil { + return err + } + workspace, err := controller.GetWorkspace( + ctx, + dirPath, + bufctl.WithIgnoreAndDisallowV1BufWorkYAMLs(), + bufctl.WithBucketOverlay(overlayBucket), + ) if err != nil { return err } @@ -181,6 +190,45 @@ func run( ); err != nil { return err } + // Build succeeded, persist the buf.lock to disk. + if err := workspaceDepManager.UpdateBufLockFile( + ctx, + configuredDepModuleKeys, + existingRemotePluginKeys, + existingRemotePolicyKeys, + existingPolicyNameToRemotePluginKeys, + ); err != nil { + return err + } // Log warnings for users on unused configured deps. return internal.LogUnusedConfiguredDepsForWorkspace(workspace, logger) } + +// newBufLockFile creates a BufLockFile for the given digest type and keys. +func newBufLockFile( + digestType bufmodule.DigestType, + depModuleKeys []bufmodule.ModuleKey, + remotePluginKeys []bufplugin.PluginKey, + remotePolicyKeys []bufpolicy.PolicyKey, + policyNameToRemotePluginKeys map[string][]bufplugin.PluginKey, +) (bufconfig.BufLockFile, error) { + switch digestType { + case bufmodule.DigestTypeB5: + return bufconfig.NewBufLockFile( + bufconfig.FileVersionV2, + depModuleKeys, + remotePluginKeys, + remotePolicyKeys, + policyNameToRemotePluginKeys, + ) + default: + // For v1beta1/v1 workspaces, plugins and policies are not supported. + return bufconfig.NewBufLockFile( + bufconfig.FileVersionV1, + depModuleKeys, + nil, + nil, + nil, + ) + } +} diff --git a/cmd/buf/internal/command/plugin/pluginupdate/pluginupdate.go b/cmd/buf/internal/command/plugin/pluginupdate/pluginupdate.go index 31df686a7e..b4298ba861 100644 --- a/cmd/buf/internal/command/plugin/pluginupdate/pluginupdate.go +++ b/cmd/buf/internal/command/plugin/pluginupdate/pluginupdate.go @@ -16,7 +16,6 @@ package pluginupdate import ( "context" - "errors" "fmt" "buf.build/go/app/appcmd" @@ -77,7 +76,7 @@ func run( ctx context.Context, container appext.Container, flags *flags, -) (retErr error) { +) error { dirPath := "." if container.NumArgs() > 0 { dirPath = container.Arg(0) @@ -140,22 +139,6 @@ func run( return err } - // We're about to edit the buf.lock file on disk. If we have a subsequent error, - // attempt to revert the buf.lock file. - // - // TODO FUTURE: We should be able to update the buf.lock file in an in-memory bucket, then do the rebuild, - // and if the rebuild is successful, then actually write to disk. It shouldn't even be that much work - just - // overlay the new buf.lock file in a union bucket. - defer func() { - if retErr != nil { - retErr = errors.Join(retErr, workspaceDepManager.UpdateBufLockFile( - ctx, existingDepModuleKeys, existingRemotePluginKeys, existingRemotePolicyKeys, existingPolicyNameToRemotePluginKeys, - )) - } - }() // Edit the buf.lock file with the updated remote plugins. - if err := workspaceDepManager.UpdateBufLockFile(ctx, existingDepModuleKeys, configuredRemotePluginKeys, existingRemotePolicyKeys, existingPolicyNameToRemotePluginKeys); err != nil { - return err - } - return nil + return workspaceDepManager.UpdateBufLockFile(ctx, existingDepModuleKeys, configuredRemotePluginKeys, existingRemotePolicyKeys, existingPolicyNameToRemotePluginKeys) } diff --git a/cmd/buf/internal/command/policy/policyupdate/policyupdate.go b/cmd/buf/internal/command/policy/policyupdate/policyupdate.go index d4d730c1f3..08d991d93a 100644 --- a/cmd/buf/internal/command/policy/policyupdate/policyupdate.go +++ b/cmd/buf/internal/command/policy/policyupdate/policyupdate.go @@ -16,7 +16,6 @@ package policyupdate import ( "context" - "errors" "fmt" "maps" @@ -81,7 +80,7 @@ func run( ctx context.Context, container appext.Container, flags *flags, -) (retErr error) { +) error { dirPath := "." if container.NumArgs() > 0 { dirPath = container.Arg(0) @@ -170,24 +169,8 @@ func run( if err != nil { return err } - // We're about to edit the buf.lock file on disk. If we have a subsequent error, - // attempt to revert the buf.lock file. - // - // TODO FUTURE: We should be able to update the buf.lock file in an in-memory bucket, then do the rebuild, - // and if the rebuild is successful, then actually write to disk. It shouldn't even be that much work - just - // overlay the new buf.lock file in a union bucket. - defer func() { - if retErr != nil { - retErr = errors.Join(retErr, workspaceDepManager.UpdateBufLockFile( - ctx, existingDepModuleKeys, existingRemotePluginKeys, existingRemotePolicyKeys, existingPolicyNameToRemotePluginKeys, - )) - } - }() - // Edit the buf.lock file with the updated remote plugins. - if err := workspaceDepManager.UpdateBufLockFile(ctx, existingDepModuleKeys, existingRemotePluginKeys, configuredRemotePolicyKeys, configuredPolicyNameToRemotePluginKeys); err != nil { - return err - } - return nil + // Edit the buf.lock file with the updated remote policies. + return workspaceDepManager.UpdateBufLockFile(ctx, existingDepModuleKeys, existingRemotePluginKeys, configuredRemotePolicyKeys, configuredPolicyNameToRemotePluginKeys) } func getPolicyKeyPluginKeysForPolicyKeys( diff --git a/private/buf/bufctl/controller.go b/private/buf/bufctl/controller.go index 126fea3df4..f7889ba336 100644 --- a/private/buf/bufctl/controller.go +++ b/private/buf/bufctl/controller.go @@ -50,6 +50,7 @@ import ( "github.com/bufbuild/buf/private/pkg/httpauth" "github.com/bufbuild/buf/private/pkg/normalpath" "github.com/bufbuild/buf/private/pkg/protoencoding" + "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/bufbuild/buf/private/pkg/wasm" @@ -1046,6 +1047,10 @@ func (c *controller) getWorkspaceForSourceRef( defer func() { retErr = errors.Join(retErr, readBucketCloser.Close()) }() + var readBucket storage.ReadBucket = readBucketCloser + if functionOptions.bucketOverlay != nil { + readBucket = storage.OverlayReadBucket(functionOptions.bucketOverlay, readBucket) + } options := []bufworkspace.WorkspaceBucketOption{ bufworkspace.WithConfigOverride( functionOptions.configOverride, @@ -1059,7 +1064,7 @@ func (c *controller) getWorkspaceForSourceRef( } return c.workspaceProvider.GetWorkspaceForBucket( ctx, - readBucketCloser, + readBucket, bucketTargeting, options..., ) diff --git a/private/buf/bufctl/option.go b/private/buf/bufctl/option.go index 74760444df..7d2d230156 100644 --- a/private/buf/bufctl/option.go +++ b/private/buf/bufctl/option.go @@ -16,6 +16,7 @@ package bufctl import ( "github.com/bufbuild/buf/private/buf/buffetch" + "github.com/bufbuild/buf/private/pkg/storage" ) // ControllerOption is a controller option. @@ -143,6 +144,17 @@ func WithMessageValidation() FunctionOption { } } +// WithBucketOverlay returns a new FunctionOption that overlays the given ReadBucket +// on top of the source bucket when building a workspace. +// +// Paths in the overlay take precedence over paths in the underlying bucket. This +// is used to validate changes (such as an updated buf.lock) before persisting to disk. +func WithBucketOverlay(overlay storage.ReadBucket) FunctionOption { + return func(functionOptions *functionOptions) { + functionOptions.bucketOverlay = overlay + } +} + // *** PRIVATE *** type functionOptions struct { @@ -158,6 +170,7 @@ type functionOptions struct { configOverride string ignoreAndDisallowV1BufWorkYAMLs bool messageValidation bool + bucketOverlay storage.ReadBucket } func newFunctionOptions(controller *controller) *functionOptions {