Workspace directory

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ## [Unreleased]
 
-- No changes yet.
+- Fix issue in v1.27.0 where `--path` did not work with workspaces under certain scenarios.
 
 ## [v1.27.0] - 2023-10-04
 

private/buf/bufwire/module_config_reader.go

@@ -611,6 +611,7 @@ func (m *moduleConfigReader) getSourceModuleConfig(
 			}
 		}
 	}
+	buildOptions = append(buildOptions, bufmodulebuild.WithWorkspaceDirectory(subDirPath))
 	if len(externalExcludeDirOrFilePaths) > 0 {
 		bucketRelPaths := make([]string, len(externalExcludeDirOrFilePaths))
 		for i, excludeDirOrFilePath := range externalExcludeDirOrFilePaths {

private/buf/bufwork/workspace_builder.go

@@ -136,6 +136,7 @@ func (w *workspaceBuilder) BuildWorkspace(
 		if err != nil {
 			return nil, err
 		}
+		buildOptions = append(buildOptions, bufmodulebuild.WithWorkspaceDirectory(directory))
 		module, err := bufmodulebuild.NewModuleBucketBuilder().BuildForBucket(
 			ctx,
 			readBucketForDirectory,

private/buf/cmd/buf/testdata/workspace/success/breaking/a/proto/a/v1/a.proto

@@ -0,0 +1,5 @@
+syntax = "proto3";
+
+package a.v1;
+
+message A {}

private/buf/cmd/buf/testdata/workspace/success/breaking/a/proto/buf.yaml

@@ -0,0 +1 @@
+version: v1

private/buf/cmd/buf/testdata/workspace/success/breaking/buf.work.yaml

@@ -1,4 +1,5 @@
 version: v1
 directories:
+  - a/proto
   - other/proto
   - proto

private/buf/cmd/buf/testdata/workspace/success/dir/a/proto/a/v1/a.proto

@@ -0,0 +1,5 @@
+syntax = "proto3";
+
+package a.v1;
+
+message A {}

private/buf/cmd/buf/testdata/workspace/success/dir/a/proto/buf.yaml

@@ -0,0 +1 @@
+version: v1

private/buf/cmd/buf/testdata/workspace/success/dir/buf.work.yaml

@@ -1,4 +1,5 @@
 version: v1
 directories:
+  - a/proto
   - other/proto
   - proto

private/buf/cmd/buf/testdata/workspace/success/dir_buf_work/a/proto/a/v1/a.proto

@@ -0,0 +1,5 @@
+syntax = "proto3";
+
+package a.v1;
+
+message A {}

private/buf/cmd/buf/testdata/workspace/success/dir_buf_work/a/proto/buf.yaml

@@ -0,0 +1 @@
+version: v1

private/buf/cmd/buf/testdata/workspace/success/dir_buf_work/buf.work

@@ -1,4 +1,5 @@
 version: v1
 directories:
+  - a/proto
   - other/proto
   - proto

private/buf/cmd/buf/workspace_test.go

@@ -116,7 +116,8 @@ func TestWorkspaceDir(t *testing.T) {
 			t,
 			nil,
 			0,
-			filepath.FromSlash(`testdata/workspace/success/`+baseDirPath+`/other/proto/request.proto
+			filepath.FromSlash(`testdata/workspace/success/`+baseDirPath+`/a/proto/a/v1/a.proto
+		    testdata/workspace/success/`+baseDirPath+`/other/proto/request.proto
 		    testdata/workspace/success/`+baseDirPath+`/proto/rpc.proto`),
 			"ls-files",
 			filepath.Join("testdata", "workspace", "success", baseDirPath),
@@ -144,7 +145,8 @@ func TestWorkspaceDir(t *testing.T) {
 			t,
 			nil,
 			0,
-			filepath.FromSlash(`testdata/workspace/success/breaking/other/proto/request.proto
+			filepath.FromSlash(`testdata/workspace/success/breaking/a/proto/a/v1/a.proto
+		    testdata/workspace/success/breaking/other/proto/request.proto
 		    testdata/workspace/success/breaking/proto/rpc.proto`),
 			"ls-files",
 			filepath.Join("testdata", "workspace", "success", "breaking"),
@@ -302,48 +304,6 @@ func TestWorkspaceNestedArchive(t *testing.T) {
 	)
 }
 
-func TestWorkspaceGit(t *testing.T) {
-	t.Skip("skip until the move to private/buf is merged")
-	// Directory paths specified as a git reference within a workspace.
-	t.Parallel()
-	testRunStdout(
-		t,
-		nil,
-		0,
-		``,
-		"build",
-		"../../../../.git#ref=HEAD,subdir=private/buf/cmd/buf/testdata/workspace/success/dir/proto",
-	)
-	testRunStdout(
-		t,
-		nil,
-		0,
-		filepath.FromSlash(`private/buf/cmd/buf/testdata/workspace/success/dir/proto/rpc.proto`),
-		"ls-files",
-		"../../../../.git#ref=HEAD,subdir=private/buf/cmd/buf/testdata/workspace/success/dir/proto",
-	)
-	testRunStdout(
-		t,
-		nil,
-		bufcli.ExitCodeFileAnnotation,
-		filepath.FromSlash(`private/buf/cmd/buf/testdata/workspace/success/dir/proto/rpc.proto:3:1:Files with package "example" must be within a directory "example" relative to root but were in directory ".".
-        private/buf/cmd/buf/testdata/workspace/success/dir/proto/rpc.proto:3:1:Package name "example" should be suffixed with a correctly formed version, such as "example.v1".`),
-		"lint",
-		"../../../../.git#ref=HEAD,subdir=private/buf/cmd/buf/testdata/workspace/success/dir/proto",
-	)
-	testRunStdout(
-		t,
-		nil,
-		bufcli.ExitCodeFileAnnotation,
-		filepath.FromSlash(`private/buf/cmd/buf/testdata/workspace/success/dir/proto/rpc.proto:3:1:Files with package "example" must be within a directory "example" relative to root but were in directory ".".
-        private/buf/cmd/buf/testdata/workspace/success/dir/proto/rpc.proto:3:1:Package name "example" should be suffixed with a correctly formed version, such as "example.v1".`),
-		"lint",
-		"../../../../.git#ref=HEAD,subdir=private/buf/cmd/buf/testdata/workspace/success/dir/proto",
-		"--path",
-		filepath.Join("internal", "buf", "cmd", "buf", "testdata", "workspace", "success", "dir", "proto", "rpc.proto"),
-	)
-}
-
 func TestWorkspaceDetached(t *testing.T) {
 	// The workspace doesn't include the 'proto' directory, so
 	// its contents aren't included in the workspace.
@@ -900,7 +860,7 @@ func TestWorkspaceBreakingFail(t *testing.T) {
 		nil,
 		1,
 		``,
-		`Failure: input contained 1 images, whereas against contained 2 images`,
+		`Failure: input contained 1 images, whereas against contained 3 images`,
 		"breaking",
 		filepath.Join("testdata", "workspace", "fail", "breaking"),
 		"--against",

private/buf/cmd/buf/workspace_unix_test.go

@@ -87,3 +87,58 @@ func TestWorkspaceAbsoluteFail(t *testing.T) {
 		filepath.Join("testdata", "workspace", "fail", "absolute"),
 	)
 }
+
+// TODO: Move this back to workspace_test.go. after resolving the issue where git
+// clone failed with "unable to create file filename too long" on Windows CI.
+// Workflow run: https://github.com/bufbuild/buf/actions/runs/6510804063/job/17685247791.
+// Potential fix: https://stackoverflow.com/questions/22575662/filename-too-long-in-git-for-windows.
+func TestWorkspaceGit(t *testing.T) {
+	// Directory paths specified as a git reference within a workspace.
+	t.Parallel()
+	testRunStdout(
+		t,
+		nil,
+		0,
+		``,
+		"build",
+		"../../../../.git#ref=HEAD,subdir=private/buf/cmd/buf/testdata/workspace/success/dir/proto",
+	)
+	testRunStdout(
+		t,
+		nil,
+		0,
+		``,
+		"build",
+		"../../../../.git#ref=HEAD,subdir=private/buf/cmd/buf/testdata/workspace/success/dir/proto",
+		"--path",
+		filepath.Join("private", "buf", "cmd", "buf", "testdata", "workspace", "success", "dir", "proto", "rpc.proto"),
+	)
+	testRunStdout(
+		t,
+		nil,
+		0,
+		filepath.FromSlash(`private/buf/cmd/buf/testdata/workspace/success/dir/proto/rpc.proto`),
+		"ls-files",
+		"../../../../.git#ref=HEAD,subdir=private/buf/cmd/buf/testdata/workspace/success/dir/proto",
+	)
+	testRunStdout(
+		t,
+		nil,
+		bufcli.ExitCodeFileAnnotation,
+		filepath.FromSlash(`private/buf/cmd/buf/testdata/workspace/success/dir/proto/rpc.proto:3:1:Files with package "example" must be within a directory "example" relative to root but were in directory ".".
+        private/buf/cmd/buf/testdata/workspace/success/dir/proto/rpc.proto:3:1:Package name "example" should be suffixed with a correctly formed version, such as "example.v1".`),
+		"lint",
+		"../../../../.git#ref=HEAD,subdir=private/buf/cmd/buf/testdata/workspace/success/dir/proto",
+	)
+	testRunStdout(
+		t,
+		nil,
+		bufcli.ExitCodeFileAnnotation,
+		filepath.FromSlash(`private/buf/cmd/buf/testdata/workspace/success/dir/proto/rpc.proto:3:1:Files with package "example" must be within a directory "example" relative to root but were in directory ".".
+        private/buf/cmd/buf/testdata/workspace/success/dir/proto/rpc.proto:3:1:Package name "example" should be suffixed with a correctly formed version, such as "example.v1".`),
+		"lint",
+		"../../../../.git#ref=HEAD,subdir=private/buf/cmd/buf/testdata/workspace/success/dir/proto",
+		"--path",
+		filepath.Join("private", "buf", "cmd", "buf", "testdata", "workspace", "success", "dir", "proto", "rpc.proto"),
+	)
+}

private/bufpkg/bufgraph/bufgraph_test.go

@@ -105,6 +105,9 @@ func testBuildWorkspace(ctx context.Context, workspacePath string) (bufmodule.Wo
 			bufmodulebuild.WithModuleIdentity(
 				moduleConfig.ModuleIdentity,
 			),
+			bufmodulebuild.WithWorkspaceDirectory(
+				directory,
+			),
 		)
 		if err != nil {
 			return nil, err

private/bufpkg/bufmodule/bufmodule.go

@@ -148,8 +148,6 @@ type Module interface {
 	// of every file in the module, which is useful for caching or recreating a module's
 	// original files.
 	BlobSet() *manifest.BlobSet
-
-	getSourceReadBucket() storage.ReadBucket
 	// ModuleIdentity returns the ModuleIdentity for the Module, if it was
 	// provided at construction time via ModuleWithModuleIdentity or ModuleWithModuleIdentityAndCommit.
 	//
@@ -165,6 +163,17 @@ type Module interface {
 	// even if ModuleIdentity is set, that is commit is optional information
 	// even if we know what module this file came from.
 	Commit() string
+	// WorkspaceDirectory returns the directory within the workspace that this Module was constructed from,
+	// if this Module was constructed from a Workspace. If the Module was not constructed from a Workspace,
+	// This value will be empty.
+	//
+	// This is needed for now because we need to determine if the input for ModuleFileSetBuilder is equal
+	// to any Modules within the given optional Workspace. This is terrible. Once we have refactored
+	// the CLI to have Workspaces as a first-class citizen, where the typical case is a Workspace with
+	// a single Module, we will no longer need to do this type of check, and this can be removed.
+	WorkspaceDirectory() string
+
+	getSourceReadBucket() storage.ReadBucket
 	isModule()
 }
 
@@ -189,6 +198,15 @@ func ModuleWithModuleIdentityAndCommit(moduleIdentity bufmoduleref.ModuleIdentit
 	}
 }
 
+// ModuleWithWorkspaceDirectory returns a new ModuleOption that sets the workspace directory.
+//
+// See the comment on Module.WorkspaceDirectory() for more details.
+func ModuleWithWorkspaceDirectory(workspaceDirectory string) ModuleOption {
+	return func(module *module) {
+		module.workspaceDirectory = workspaceDirectory
+	}
+}
+
 // NewModuleForBucket returns a new Module. It attempts to read dependencies
 // from a lock file in the read bucket.
 func NewModuleForBucket(

private/bufpkg/bufmodule/bufmodulebuild/bufmodulebuild.go

@@ -16,7 +16,6 @@ package bufmodulebuild
 
 import (
 	"context"
-	"errors"
 
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule"
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleconfig"
@@ -26,10 +25,6 @@ import (
 	"go.uber.org/zap"
 )
 
-// ErrDuplicateDependency is the error returned if a two modules have the same set of Protobuf file
-// paths.
-var ErrDuplicateDependency = errors.New("module declared in DependencyModulePins but not in workspace was already added to the dependency Module set")
-
 // ModuleFileSetBuilder builds ModuleFileSets from Modules.
 type ModuleFileSetBuilder interface {
 	Build(
@@ -173,3 +168,12 @@ func WithExcludePathsAllowNotExist(excludePaths []string) BuildOption {
 		buildOptions.pathsAllowNotExist = true
 	}
 }
+
+// WithWorkspaceDirectory returns a new BuildOption that specifies the workspace directory.
+//
+// See the comment on Module.WorkspaceDirectory() for more details.
+func WithWorkspaceDirectory(workspaceDirectory string) BuildOption {
+	return func(buildOptions *buildOptions) {
+		buildOptions.workspaceDirectory = workspaceDirectory
+	}
+}

private/bufpkg/bufmodule/bufmodulebuild/module_bucket_builder.go

@@ -141,6 +141,9 @@ func (b *moduleBucketBuilder) buildForBucket(
 		bufmodule.ModuleWithModuleIdentity(
 			buildOptions.moduleIdentity, // This may be nil
 		),
+		bufmodule.ModuleWithWorkspaceDirectory(
+			buildOptions.workspaceDirectory,
+		),
 	)
 	if err != nil {
 		return nil, err