From b16eccacb29898d591ae32455ce3f20ef89da453 Mon Sep 17 00:00:00 2001 From: bufdev <4228796+bufdev@users.noreply.github.com> Date: Thu, 29 Jun 2023 11:34:36 -0400 Subject: [PATCH] Delete bufmodulebuild.BuildForBucket (#2237) --- private/buf/bufcli/bufcli.go | 3 +- private/buf/bufsync/syncer.go | 2 +- private/buf/bufwire/file_lister.go | 2 +- private/buf/bufwire/module_config_reader.go | 2 +- private/buf/bufwork/workspace_builder.go | 2 +- private/buf/cmd/buf/command/push/push.go | 2 +- .../bufcheck/bufbreaking/bufbreaking_test.go | 4 +- .../bufpkg/bufcheck/buflint/buflint_test.go | 2 +- .../bufimagebuildtesting.go | 2 +- .../bufimage/bufimagebuild/builder_test.go | 2 +- .../bufmodulebuild/bufmodulebuild.go | 31 +++++++++-- .../bufmodulebuild/module_bucket_builder.go | 52 ++++++------------- .../module_bucket_builder_test.go | 14 ++--- 13 files changed, 60 insertions(+), 60 deletions(-) diff --git a/private/buf/bufcli/bufcli.go b/private/buf/bufcli/bufcli.go index 3c22bb9e4d..b53e5d5ea8 100644 --- a/private/buf/bufcli/bufcli.go +++ b/private/buf/bufcli/bufcli.go @@ -812,7 +812,8 @@ func WellKnownTypeImage(ctx context.Context, logger *zap.Logger, wellKnownType s if err != nil { return nil, err } - module, err := bufmodulebuild.BuildForBucket( + + module, err := bufmodulebuild.NewModuleBucketBuilder().BuildForBucket( ctx, datawkt.ReadBucket, sourceConfig.Build, diff --git a/private/buf/bufsync/syncer.go b/private/buf/bufsync/syncer.go index adc7da2865..1390e2b8f5 100644 --- a/private/buf/bufsync/syncer.go +++ b/private/buf/bufsync/syncer.go @@ -213,7 +213,7 @@ func (s *syncer) visitCommit( ) return nil } - builtModule, err := bufmodulebuild.BuildForBucket( + builtModule, err := bufmodulebuild.NewModuleBucketBuilder().BuildForBucket( ctx, sourceBucket, sourceConfig.Build, diff --git a/private/buf/bufwire/file_lister.go b/private/buf/bufwire/file_lister.go index 288e848d27..7e8cee376b 100644 --- a/private/buf/bufwire/file_lister.go +++ b/private/buf/bufwire/file_lister.go @@ -256,7 +256,7 @@ func (e *fileLister) sourceFileInfosForDirectory( if err != nil { return nil, err } - module, err := bufmodulebuild.BuildForBucket( + module, err := bufmodulebuild.NewModuleBucketBuilder().BuildForBucket( ctx, mappedReadBucket, config.Build, diff --git a/private/buf/bufwire/module_config_reader.go b/private/buf/bufwire/module_config_reader.go index 89f9cc8dee..014fbd80da 100644 --- a/private/buf/bufwire/module_config_reader.go +++ b/private/buf/bufwire/module_config_reader.go @@ -618,7 +618,7 @@ func (m *moduleConfigReader) getSourceModuleConfig( } buildOptions = append(buildOptions, bufmodulebuild.WithExcludePaths(bucketRelPaths)) } - module, err := bufmodulebuild.BuildForBucket( + module, err := bufmodulebuild.NewModuleBucketBuilder().BuildForBucket( ctx, mappedReadBucket, moduleConfig.Build, diff --git a/private/buf/bufwork/workspace_builder.go b/private/buf/bufwork/workspace_builder.go index 6539ab09bc..2b0fcdf66f 100644 --- a/private/buf/bufwork/workspace_builder.go +++ b/private/buf/bufwork/workspace_builder.go @@ -136,7 +136,7 @@ func (w *workspaceBuilder) BuildWorkspace( if err != nil { return nil, err } - module, err := bufmodulebuild.BuildForBucket( + module, err := bufmodulebuild.NewModuleBucketBuilder().BuildForBucket( ctx, readBucketForDirectory, moduleConfig.Build, diff --git a/private/buf/cmd/buf/command/push/push.go b/private/buf/cmd/buf/command/push/push.go index 75e261b882..151102d540 100644 --- a/private/buf/cmd/buf/command/push/push.go +++ b/private/buf/cmd/buf/command/push/push.go @@ -181,7 +181,7 @@ func run( return err } moduleIdentity := sourceConfig.ModuleIdentity - builtModule, err := bufmodulebuild.BuildForBucket( + builtModule, err := bufmodulebuild.NewModuleBucketBuilder().BuildForBucket( ctx, sourceBucket, sourceConfig.Build, diff --git a/private/bufpkg/bufcheck/bufbreaking/bufbreaking_test.go b/private/bufpkg/bufcheck/bufbreaking/bufbreaking_test.go index b15d7bc066..27b0f03b12 100644 --- a/private/bufpkg/bufcheck/bufbreaking/bufbreaking_test.go +++ b/private/bufpkg/bufcheck/bufbreaking/bufbreaking_test.go @@ -737,7 +737,7 @@ func testBreaking( previousConfig := testGetConfig(t, previousReadWriteBucket) config := testGetConfig(t, readWriteBucket) - previousModule, err := bufmodulebuild.BuildForBucket( + previousModule, err := bufmodulebuild.NewModuleBucketBuilder().BuildForBucket( context.Background(), previousReadWriteBucket, previousConfig.Build, @@ -755,7 +755,7 @@ func testBreaking( require.Empty(t, previousFileAnnotations) previousImage = bufimage.ImageWithoutImports(previousImage) - module, err := bufmodulebuild.BuildForBucket( + module, err := bufmodulebuild.NewModuleBucketBuilder().BuildForBucket( context.Background(), readWriteBucket, config.Build, diff --git a/private/bufpkg/bufcheck/buflint/buflint_test.go b/private/bufpkg/bufcheck/buflint/buflint_test.go index 624ac5d65d..51c63144f7 100644 --- a/private/bufpkg/bufcheck/buflint/buflint_test.go +++ b/private/bufpkg/bufcheck/buflint/buflint_test.go @@ -925,7 +925,7 @@ func testLintConfigModifier( configModifier(config) } - module, err := bufmodulebuild.BuildForBucket( + module, err := bufmodulebuild.NewModuleBucketBuilder().BuildForBucket( context.Background(), readWriteBucket, config.Build, diff --git a/private/bufpkg/bufimage/bufimagebuild/bufimagebuildtesting/bufimagebuildtesting.go b/private/bufpkg/bufimage/bufimagebuild/bufimagebuildtesting/bufimagebuildtesting.go index 463f740bfb..10647c9fcf 100644 --- a/private/bufpkg/bufimage/bufimagebuild/bufimagebuildtesting/bufimagebuildtesting.go +++ b/private/bufpkg/bufimage/bufimagebuild/bufimagebuildtesting/bufimagebuildtesting.go @@ -115,7 +115,7 @@ func fuzzGetModule(ctx context.Context, dirPath string) (bufmodule.Module, error if err != nil { return nil, err } - return bufmodulebuild.BuildForBucket( + return bufmodulebuild.NewModuleBucketBuilder().BuildForBucket( ctx, readWriteBucket, config, diff --git a/private/bufpkg/bufimage/bufimagebuild/builder_test.go b/private/bufpkg/bufimage/bufimagebuild/builder_test.go index 0733892ecc..4d447dabb5 100644 --- a/private/bufpkg/bufimage/bufimagebuild/builder_test.go +++ b/private/bufpkg/bufimage/bufimagebuild/builder_test.go @@ -356,7 +356,7 @@ func testGetModule(t *testing.T, dirPath string) bufmodule.Module { require.NoError(t, err) config, err := bufmoduleconfig.NewConfigV1(bufmoduleconfig.ExternalConfigV1{}) require.NoError(t, err) - module, err := bufmodulebuild.BuildForBucket( + module, err := bufmodulebuild.NewModuleBucketBuilder().BuildForBucket( context.Background(), readWriteBucket, config, diff --git a/private/bufpkg/bufmodule/bufmodulebuild/bufmodulebuild.go b/private/bufpkg/bufmodule/bufmodulebuild/bufmodulebuild.go index e13de64178..4386589726 100644 --- a/private/bufpkg/bufmodule/bufmodulebuild/bufmodulebuild.go +++ b/private/bufpkg/bufmodule/bufmodulebuild/bufmodulebuild.go @@ -18,7 +18,9 @@ import ( "context" "github.com/bufbuild/buf/private/bufpkg/bufmodule" + "github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleconfig" "github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleref" + "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storageos" "go.uber.org/zap" ) @@ -50,14 +52,33 @@ func WithWorkspace(workspace bufmodule.Workspace) BuildModuleFileSetOption { } } +// BuiltModule ties a bufmodule.Module with the configuration and a bucket +// containing just the files required to build it. +type BuiltModule struct { + bufmodule.Module + Bucket storage.ReadBucket +} + // ModuleBucketBuilder builds modules for buckets. -type ModuleBucketBuilder = *moduleBucketBuilder +type ModuleBucketBuilder interface { + // BuildForBucket constructs a minimal bucket from the passed readBucket and + // builds a module from it. + // + // config's value is used even if the bucket contains configuration (buf.yaml). + // This means the module is built differently than described in storage, which + // may cause building to fail or succeed when it shouldn't. For your own + // sanity, you should pass a config value read from the provided bucket. + BuildForBucket( + ctx context.Context, + readBucket storage.ReadBucket, + config *bufmoduleconfig.Config, + options ...BuildOption, + ) (*BuiltModule, error) +} // NewModuleBucketBuilder returns a new BucketBuilder. -func NewModuleBucketBuilder( - options ...BuildOption, -) ModuleBucketBuilder { - return newModuleBucketBuilder(options...) +func NewModuleBucketBuilder() ModuleBucketBuilder { + return newModuleBucketBuilder() } // ModuleIncludeBuilder builds modules for includes. diff --git a/private/bufpkg/bufmodule/bufmodulebuild/module_bucket_builder.go b/private/bufpkg/bufmodule/bufmodulebuild/module_bucket_builder.go index ecdf074440..ee8a4812c9 100644 --- a/private/bufpkg/bufmodule/bufmodulebuild/module_bucket_builder.go +++ b/private/bufpkg/bufmodule/bufmodulebuild/module_bucket_builder.go @@ -26,58 +26,36 @@ import ( "github.com/bufbuild/buf/private/pkg/storage/storagemem" ) -// BuiltModule ties a bufmodule.Module with the configuration and a bucket -// containing just the files required to build it. -type BuiltModule struct { - bufmodule.Module - Bucket storage.ReadBucket -} - type moduleBucketBuilder struct { - opt buildOptions } -func newModuleBucketBuilder( - options ...BuildOption, -) *moduleBucketBuilder { - opt := buildOptions{} - for _, option := range options { - option(&opt) - } - return &moduleBucketBuilder{opt: opt} +func newModuleBucketBuilder() *moduleBucketBuilder { + return &moduleBucketBuilder{} } -// BuildForBucket is an alternative constructor of NewModuleBucketBuilder -// followed by calling the BuildForBucket method. -func BuildForBucket( +func (b *moduleBucketBuilder) BuildForBucket( ctx context.Context, readBucket storage.ReadBucket, config *bufmoduleconfig.Config, options ...BuildOption, ) (*BuiltModule, error) { - builder := newModuleBucketBuilder(options...) - bm, err := builder.BuildForBucket( + buildOptions := &buildOptions{} + for _, option := range options { + option(buildOptions) + } + return b.buildForBucket( ctx, readBucket, config, + buildOptions, ) - if err != nil { - return nil, err - } - return bm, nil } -// BuildForBucket constructs a minimal bucket from the passed readBucket and -// builds a module from it. -// -// config's value is used even if the bucket contains configuration (buf.yaml). -// This means the module is built differently than described in storage, which -// may cause building to fail or succeed when it shouldn't. For your own -// sanity, you should pass a config value read from the provided bucket. -func (b *moduleBucketBuilder) BuildForBucket( +func (b *moduleBucketBuilder) buildForBucket( ctx context.Context, readBucket storage.ReadBucket, config *bufmoduleconfig.Config, + buildOptions *buildOptions, ) (*BuiltModule, error) { // proxy plain files externalPaths := []string{ @@ -145,7 +123,7 @@ func (b *moduleBucketBuilder) BuildForBucket( ctx, bucket, bufmodule.ModuleWithModuleIdentity( - b.opt.moduleIdentity, // This may be nil + buildOptions.moduleIdentity, // This may be nil ), ) if err != nil { @@ -154,9 +132,9 @@ func (b *moduleBucketBuilder) BuildForBucket( appliedModule, err := applyModulePaths( module, roots, - b.opt.paths, - b.opt.excludePaths, - b.opt.pathsAllowNotExist, + buildOptions.paths, + buildOptions.excludePaths, + buildOptions.pathsAllowNotExist, normalpath.Relative, ) if err != nil { diff --git a/private/bufpkg/bufmodule/bufmodulebuild/module_bucket_builder_test.go b/private/bufpkg/bufmodule/bufmodulebuild/module_bucket_builder_test.go index 1c29f2b659..a39fa8906f 100644 --- a/private/bufpkg/bufmodule/bufmodulebuild/module_bucket_builder_test.go +++ b/private/bufpkg/bufmodule/bufmodulebuild/module_bucket_builder_test.go @@ -353,7 +353,7 @@ lint: bufmoduleconfig.ExternalConfigV1{}, ) require.NoError(t, err) - module, err := BuildForBucket( + module, err := NewModuleBucketBuilder().BuildForBucket( ctx, bucket, config, @@ -406,7 +406,7 @@ func testBucketGetFileInfos( storageos.ReadWriteBucketWithSymlinksIfSupported(), ) require.NoError(t, err) - module, err := BuildForBucket( + module, err := NewModuleBucketBuilder().BuildForBucket( context.Background(), readWriteBucket, config, @@ -429,7 +429,7 @@ func testBucketGetFileInfos( require.NoError(t, err) bucketRelPaths[i] = bucketRelPath } - module, err := BuildForBucket( + module, err := NewModuleBucketBuilder().BuildForBucket( context.Background(), readWriteBucket, config, @@ -458,7 +458,7 @@ func testBucketGetAllFileInfosError( storageos.ReadWriteBucketWithSymlinksIfSupported(), ) require.NoError(t, err) - module, err := BuildForBucket( + module, err := NewModuleBucketBuilder().BuildForBucket( context.Background(), readWriteBucket, config, @@ -492,7 +492,7 @@ func testBucketGetFileInfosForExternalPathsError( require.NoError(t, err) bucketRelPaths[i] = bucketRelPath } - _, err = BuildForBucket( + _, err = NewModuleBucketBuilder().BuildForBucket( context.Background(), readWriteBucket, config, @@ -517,7 +517,7 @@ func testDocumentationBucket( bufmoduleconfig.ExternalConfigV1{}, ) require.NoError(t, err) - module, err := BuildForBucket( + module, err := NewModuleBucketBuilder().BuildForBucket( context.Background(), readWriteBucket, config, @@ -551,7 +551,7 @@ func testLicenseBucket( bufmoduleconfig.ExternalConfigV1{}, ) require.NoError(t, err) - module, err := BuildForBucket( + module, err := NewModuleBucketBuilder().BuildForBucket( context.Background(), readWriteBucket, config,