Change ModuleConfigReader to return an interface with a single Worksp…

…ace (#2233)

private/buf/bufwire/bufwire.go

@@ -75,30 +75,36 @@ func NewImageConfigReader(
 	)
 }
 
-// ModuleConfig is an module and configuration.
+// ModuleConfig is a Module and configuration.
 type ModuleConfig interface {
 	Module() bufmodule.Module
 	Config() *bufconfig.Config
+}
+
+// ModuleConfigSet is a set of ModuleConfigs with a potentially associated Workspace.
+type ModuleConfigSet interface {
+	ModuleConfigs() []ModuleConfig
+	// Optional. May be nil.
 	Workspace() bufmodule.Workspace
 }
 
 // ModuleConfigReader is a ModuleConfig reader.
 type ModuleConfigReader interface {
-	// GetModuleConfigs gets the ModuleConfig for the fetch value.
+	// GetModuleConfigSet gets the ModuleConfigSet for the fetch value.
 	//
 	// If externalDirOrFilePaths is empty, this builds all files under Buf control.
 	//
-	// Note that as opposed to ModuleReader, this will return a Module for either
+	// Note that as opposed to ModuleReader, this will return Modules for either
 	// a source or module reference, not just a module reference.
-	GetModuleConfigs(
+	GetModuleConfigSet(
 		ctx context.Context,
 		container app.EnvStdinContainer,
 		sourceOrModuleRef buffetch.SourceOrModuleRef,
 		configOverride string,
 		externalDirOrFilePaths []string,
 		externalExcludeDirOrFilePaths []string,
 		externalDirOrFilePathsAllowNotExist bool,
-	) ([]ModuleConfig, error)
+	) (ModuleConfigSet, error)
 }
 
 // NewModuleConfigReader returns a new ModuleConfigReader

private/buf/bufwire/image_config_reader.go

@@ -127,7 +127,7 @@ func (i *imageConfigReader) getSourceOrModuleImageConfigs(
 	externalDirOrFilePathsAllowNotExist bool,
 	excludeSourceCodeInfo bool,
 ) ([]ImageConfig, []bufanalysis.FileAnnotation, error) {
-	moduleConfigs, err := i.moduleConfigReader.GetModuleConfigs(
+	moduleConfigSet, err := i.moduleConfigReader.GetModuleConfigSet(
 		ctx,
 		container,
 		sourceOrModuleRef,
@@ -139,6 +139,7 @@ func (i *imageConfigReader) getSourceOrModuleImageConfigs(
 	if err != nil {
 		return nil, nil, err
 	}
+	moduleConfigs := moduleConfigSet.ModuleConfigs()
 	imageConfigs := make([]ImageConfig, 0, len(moduleConfigs))
 	var allFileAnnotations []bufanalysis.FileAnnotation
 	for _, moduleConfig := range moduleConfigs {
@@ -153,7 +154,7 @@ func (i *imageConfigReader) getSourceOrModuleImageConfigs(
 		}
 		buildOpts := []bufimagebuild.BuildOption{
 			bufimagebuild.WithExpectedDirectDependencies(moduleConfig.Module().DeclaredDirectDependencies()),
-			bufimagebuild.WithWorkspace(moduleConfig.Workspace()),
+			bufimagebuild.WithWorkspace(moduleConfigSet.Workspace()),
 		}
 		if excludeSourceCodeInfo {
 			buildOpts = append(buildOpts, bufimagebuild.WithExcludeSourceCodeInfo())

private/buf/bufwire/module_config.go

@@ -20,16 +20,14 @@ import (
 )
 
 type moduleConfig struct {
-	module    bufmodule.Module
-	config    *bufconfig.Config
-	workspace bufmodule.Workspace
+	module bufmodule.Module
+	config *bufconfig.Config
 }
 
-func newModuleConfig(module bufmodule.Module, config *bufconfig.Config, workspace bufmodule.Workspace) *moduleConfig {
+func newModuleConfig(module bufmodule.Module, config *bufconfig.Config) *moduleConfig {
 	return &moduleConfig{
-		module:    module,
-		config:    config,
-		workspace: workspace,
+		module: module,
+		config: config,
 	}
 }
 
@@ -40,7 +38,3 @@ func (m *moduleConfig) Module() bufmodule.Module {
 func (m *moduleConfig) Config() *bufconfig.Config {
 	return m.config
 }
-
-func (m *moduleConfig) Workspace() bufmodule.Workspace {
-	return m.workspace
-}

private/buf/bufwire/module_config_reader.go

@@ -61,15 +61,15 @@ func newModuleConfigReader(
 	}
 }
 
-func (m *moduleConfigReader) GetModuleConfigs(
+func (m *moduleConfigReader) GetModuleConfigSet(
 	ctx context.Context,
 	container app.EnvStdinContainer,
 	sourceOrModuleRef buffetch.SourceOrModuleRef,
 	configOverride string,
 	externalDirOrFilePaths []string,
 	externalExcludeDirOrFilePaths []string,
 	externalDirOrFilePathsAllowNotExist bool,
-) (_ []ModuleConfig, retErr error) {
+) (_ ModuleConfigSet, retErr error) {
 	ctx, span := m.tracer.Start(ctx, "get_module_config")
 	defer span.End()
 	defer func() {
@@ -82,7 +82,7 @@ func (m *moduleConfigReader) GetModuleConfigs(
 	workspaceBuilder := bufwork.NewWorkspaceBuilder()
 	switch t := sourceOrModuleRef.(type) {
 	case buffetch.ProtoFileRef:
-		return m.getProtoFileModuleSourceConfigs(
+		return m.getProtoFileModuleSourceConfigSet(
 			ctx,
 			container,
 			t,
@@ -93,7 +93,7 @@ func (m *moduleConfigReader) GetModuleConfigs(
 			externalDirOrFilePathsAllowNotExist,
 		)
 	case buffetch.SourceRef:
-		return m.getSourceModuleConfigs(
+		return m.getSourceModuleConfigSet(
 			ctx,
 			container,
 			t,
@@ -116,15 +116,18 @@ func (m *moduleConfigReader) GetModuleConfigs(
 		if err != nil {
 			return nil, err
 		}
-		return []ModuleConfig{
-			moduleConfig,
-		}, nil
+		return newModuleConfigSet(
+			[]ModuleConfig{
+				moduleConfig,
+			},
+			nil,
+		), nil
 	default:
 		return nil, fmt.Errorf("invalid ref: %T", sourceOrModuleRef)
 	}
 }
 
-func (m *moduleConfigReader) getSourceModuleConfigs(
+func (m *moduleConfigReader) getSourceModuleConfigSet(
 	ctx context.Context,
 	container app.EnvStdinContainer,
 	sourceRef buffetch.SourceRef,
@@ -133,7 +136,7 @@ func (m *moduleConfigReader) getSourceModuleConfigs(
 	externalDirOrFilePaths []string,
 	externalExcludeDirOrFilePaths []string,
 	externalDirOrFilePathsAllowNotExist bool,
-) (_ []ModuleConfig, retErr error) {
+) (_ ModuleConfigSet, retErr error) {
 	readBucketCloser, err := m.fetchReader.GetSourceBucket(ctx, container, sourceRef)
 	if err != nil {
 		return nil, err
@@ -146,7 +149,7 @@ func (m *moduleConfigReader) getSourceModuleConfigs(
 		return nil, err
 	}
 	if existingConfigFilePath != "" {
-		return m.getWorkspaceModuleConfigs(
+		return m.getWorkspaceModuleConfigSet(
 			ctx,
 			sourceRef,
 			workspaceBuilder,
@@ -176,9 +179,12 @@ func (m *moduleConfigReader) getSourceModuleConfigs(
 	if err != nil {
 		return nil, err
 	}
-	return []ModuleConfig{
-		moduleConfig,
-	}, nil
+	return newModuleConfigSet(
+		[]ModuleConfig{
+			moduleConfig,
+		},
+		nil,
+	), nil
 }
 
 func (m *moduleConfigReader) getModuleModuleConfig(
@@ -240,10 +246,10 @@ func (m *moduleConfigReader) getModuleModuleConfig(
 	if err != nil {
 		return nil, err
 	}
-	return newModuleConfig(module, config, nil /* Workspaces aren't supported for ModuleRefs */), nil
+	return newModuleConfig(module, config), nil
 }
 
-func (m *moduleConfigReader) getProtoFileModuleSourceConfigs(
+func (m *moduleConfigReader) getProtoFileModuleSourceConfigSet(
 	ctx context.Context,
 	container app.EnvStdinContainer,
 	protoFileRef buffetch.ProtoFileRef,
@@ -252,7 +258,7 @@ func (m *moduleConfigReader) getProtoFileModuleSourceConfigs(
 	externalDirOrFilePaths []string,
 	externalExcludeDirOrFilePaths []string,
 	externalDirOrFilePathsAllowNotExist bool,
-) (_ []ModuleConfig, retErr error) {
+) (_ ModuleConfigSet, retErr error) {
 	readBucketCloser, err := m.fetchReader.GetSourceBucket(ctx, container, protoFileRef)
 	if err != nil {
 		return nil, err
@@ -296,7 +302,7 @@ func (m *moduleConfigReader) getProtoFileModuleSourceConfigs(
 				}
 			}
 		}
-		return m.getWorkspaceModuleConfigs(
+		return m.getWorkspaceModuleConfigSet(
 			ctx,
 			protoFileRef,
 			workspaceBuilder,
@@ -326,12 +332,15 @@ func (m *moduleConfigReader) getProtoFileModuleSourceConfigs(
 	if err != nil {
 		return nil, err
 	}
-	return []ModuleConfig{
-		moduleConfig,
-	}, nil
+	return newModuleConfigSet(
+		[]ModuleConfig{
+			moduleConfig,
+		},
+		nil,
+	), nil
 }
 
-func (m *moduleConfigReader) getWorkspaceModuleConfigs(
+func (m *moduleConfigReader) getWorkspaceModuleConfigSet(
 	ctx context.Context,
 	sourceRef buffetch.SourceRef,
 	workspaceBuilder bufwork.WorkspaceBuilder,
@@ -342,28 +351,26 @@ func (m *moduleConfigReader) getWorkspaceModuleConfigs(
 	externalDirOrFilePaths []string,
 	externalExcludeDirOrFilePaths []string,
 	externalDirOrFilePathsAllowNotExist bool,
-) ([]ModuleConfig, error) {
+) (ModuleConfigSet, error) {
 	workspaceConfig, err := bufwork.GetConfigForBucket(ctx, readBucket, relativeRootPath)
 	if err != nil {
 		return nil, err
 	}
+	workspace, err := workspaceBuilder.BuildWorkspace(
+		ctx,
+		workspaceConfig,
+		readBucket,
+		relativeRootPath,
+		subDirPath, // this is used to only apply the config override to this directory
+		configOverride,
+		externalDirOrFilePaths,
+		externalExcludeDirOrFilePaths,
+		externalDirOrFilePathsAllowNotExist,
+	)
+	if err != nil {
+		return nil, err
+	}
 	if subDirPath != "." {
-		// There's only a single ModuleConfig based on the subDirPath,
-		// so we only need to create a single workspace.
-		workspace, err := workspaceBuilder.BuildWorkspace(
-			ctx,
-			workspaceConfig,
-			readBucket,
-			relativeRootPath,
-			subDirPath,
-			configOverride,
-			externalDirOrFilePaths,
-			externalExcludeDirOrFilePaths,
-			externalDirOrFilePathsAllowNotExist,
-		)
-		if err != nil {
-			return nil, err
-		}
 		moduleConfig, err := m.getSourceModuleConfig(
 			ctx,
 			sourceRef,
@@ -381,9 +388,12 @@ func (m *moduleConfigReader) getWorkspaceModuleConfigs(
 		if err != nil {
 			return nil, err
 		}
-		return []ModuleConfig{
-			moduleConfig,
-		}, nil
+		return newModuleConfigSet(
+			[]ModuleConfig{
+				moduleConfig,
+			},
+			workspace,
+		), nil
 	}
 	if configOverride != "" {
 		return nil, errors.New("the --config flag is not compatible with workspaces")
@@ -427,20 +437,6 @@ func (m *moduleConfigReader) getWorkspaceModuleConfigs(
 		for excludeFileOrDirPath, subDirRelExcludePath := range externalExcludeToSubDirRelExcludePaths {
 			externalExcludePathToRelPaths[excludeFileOrDirPath] = subDirRelExcludePath
 		}
-		workspace, err := workspaceBuilder.BuildWorkspace(
-			ctx,
-			workspaceConfig,
-			readBucket,
-			relativeRootPath,
-			directory,
-			configOverride,
-			externalDirOrFilePaths,
-			externalExcludeDirOrFilePaths,
-			externalDirOrFilePathsAllowNotExist,
-		)
-		if err != nil {
-			return nil, err
-		}
 		moduleConfig, err := m.getSourceModuleConfig(
 			ctx,
 			sourceRef,
@@ -473,7 +469,7 @@ func (m *moduleConfigReader) getWorkspaceModuleConfigs(
 			}
 		}
 	}
-	return moduleConfigs, nil
+	return newModuleConfigSet(moduleConfigs, workspace), nil
 }
 
 func (m *moduleConfigReader) getSourceModuleConfig(
@@ -505,7 +501,7 @@ func (m *moduleConfigReader) getSourceModuleConfig(
 				}
 			}
 		}
-		return newModuleConfig(module, moduleConfig, workspace), nil
+		return newModuleConfig(module, moduleConfig), nil
 	}
 	mappedReadBucket := readBucket
 	if subDirPath != "." {
@@ -643,7 +639,7 @@ func (m *moduleConfigReader) getSourceModuleConfig(
 		}
 		m.logger.Warn(builder.String())
 	}
-	return newModuleConfig(module, moduleConfig, workspace), nil
+	return newModuleConfig(module, moduleConfig), nil
 }
 
 func workspaceDirectoryEqualsOrContainsSubDirPath(workspaceConfig *bufwork.Config, subDirPath string) bool {

private/buf/bufwire/module_config_set.go

@@ -0,0 +1,39 @@
+// Copyright 2020-2023 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 bufwire
+
+import (
+	"github.com/bufbuild/buf/private/bufpkg/bufmodule"
+)
+
+type moduleConfigSet struct {
+	moduleConfigs []ModuleConfig
+	workspace     bufmodule.Workspace
+}
+
+func newModuleConfigSet(moduleConfigs []ModuleConfig, workspace bufmodule.Workspace) *moduleConfigSet {
+	return &moduleConfigSet{
+		moduleConfigs: moduleConfigs,
+		workspace:     workspace,
+	}
+}
+
+func (m *moduleConfigSet) ModuleConfigs() []ModuleConfig {
+	return m.moduleConfigs
+}
+
+func (m *moduleConfigSet) Workspace() bufmodule.Workspace {
+	return m.workspace
+}