Delete bufmodulebuild.BuildForBucket (#2237)

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,

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,

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,

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,

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,

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,

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,

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,

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,

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,

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.

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 {

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,