Repository: bufbuild/buf
Pull request #944 (2022-02-15 16:47:30)

Disable config override for workspaces
The behaviour of the `--config` flag for workspaces is not immediately apparent to users. If a config contains [build configurations](https://docs.buf.build/configuration/v1/buf-yaml#build), for example, then that does not apply seamlessly to all modules across the workspace, and the build will error. Since this behaviour is very brittle, we have made the decision to disable the `--config` flag altogether for workspaces.

doriable: > We might not need to keep around _all_ the test cases from before, but it doesn't hurt to leave them around either. Looks good to me.

Yeah, I wasn't sure about that, since on one hand, they all end up testing the same code path (lol), but on the other hand, they're tests I don't want to miss if we bring back compatibility with the flag. But thinking through that a bit, I think it might still make sense to cull down on the tests (as you say, maybe not _entirely_), since if we were to re-implement this, it would be rewritten anyway, and we'd need to go through and carefully reassess the test cases.
amckinney: Yea it's no big deal, feel free to leave them in if it's easier. As long as different arguments are being used (e.g. targeting a workspace, targeting a module within a workspace, etc), we're still testing that the `configOverride` takes effect for different workspace scenarios.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b1fd52f16..d18ce4f9e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,6 +26,7 @@
 - Removed the `buf config ls-lint-rules` command in favor of `buf mod ls-lint-rules`.
 - Removed the `buf config migrate-v1beta1` command in favor of `buf beta migrate-v1beta1`.
 - Add `buf beta decode` command to decode message with provided image source and message type.
+- Disable `--config` flag for workspaces.
 
 ## [v1.0.0-rc12] - 2022-02-01
 
diff --git a/private/buf/bufwork/workspace_builder.go b/private/buf/bufwork/workspace_builder.go
index 3a227edba..6950811b2 100644
--- a/private/buf/bufwork/workspace_builder.go
+++ b/private/buf/bufwork/workspace_builder.go
@@ -56,6 +56,9 @@ func (w *workspaceBuilder) BuildWorkspace(
 	if workspaceConfig == nil {
 		return nil, errors.New("received a nil workspace config")
 	}
+	if configOverride != "" {
+		return nil, errors.New("the --config flag is not compatible with workspaces")
+	}
 	// We know that if the file is actually buf.work for legacy reasons, this will be wrong,
 	// but we accept that as this shouldn't happen often anymore and this is just
 	// used for error messages.


Test patch:
diff --git a/private/buf/cmd/buf/buf_test.go b/private/buf/cmd/buf/buf_test.go
index 8d71fbfa2..b06fe0e19 100644
--- a/private/buf/cmd/buf/buf_test.go
+++ b/private/buf/cmd/buf/buf_test.go
@@ -244,45 +244,32 @@ func TestFail7(t *testing.T) {
 	testRunStdout(
 		t,
 		nil,
-		bufcli.ExitCodeFileAnnotation,
-		filepath.FromSlash(`testdata/fail/buf/buf.proto:3:1:Files with package "other" must be within a directory "other" relative to root but were in directory "fail/buf".
-        testdata/fail/buf/buf.proto:6:9:Field name "oneTwo" should be lower_snake_case, such as "one_two".`),
+		1,
+		``,
 		"lint",
-		"--path",
 		filepath.Join("testdata", "fail", "buf", "buf.proto"),
-		filepath.Join("testdata"),
-		"--config",
-		`{"version":"v1beta1","lint":{"use":["BASIC"]}}`,
+		"--input-config",
+		`{"version":"v1","lint":{"use":["BASIC"]}}`,
 	)
-	testRunStdout(
+	testRunStdoutStderr(
 		t,
 		nil,
-		bufcli.ExitCodeFileAnnotation,
-		filepath.FromSlash(`testdata/fail/buf/buf.proto:3:1:Files with package "other" must be within a directory "other" relative to root but were in directory "fail/buf".
-        testdata/fail/buf/buf.proto:6:9:Field name "oneTwo" should be lower_snake_case, such as "one_two".`),
+		1,
+		"", // stdout should be empty
+		"Failure: the --config flag is not compatible with workspaces",
 		"lint",
 		"--path",
 		filepath.Join("testdata", "fail", "buf", "buf.proto"),
 		filepath.Join("testdata"),
 		"--config",
-		`{"version":"v1","lint":{"use":["BASIC"]}}`,
+		`{"version":"v1beta1","lint":{"use":["BASIC"]}}`,
 	)
-	testRunStdout(
+	testRunStdoutStderr(
 		t,
 		nil,
 		1,
-		``,
-		"lint",
-		filepath.Join("testdata", "fail", "buf", "buf.proto"),
-		"--input-config",
-		`{"version":"v1","lint":{"use":["BASIC"]}}`,
-	)
-	testRunStdout(
-		t,
-		nil,
-		bufcli.ExitCodeFileAnnotation,
-		filepath.FromSlash(`testdata/fail/buf/buf.proto:3:1:Files with package "other" must be within a directory "other" relative to root but were in directory "buf".
-        testdata/fail/buf/buf.proto:6:9:Field name "oneTwo" should be lower_snake_case, such as "one_two".`),
+		"", // stdout should be empty
+		"Failure: the --config flag is not compatible with workspaces",
 		"lint",
 		filepath.Join("testdata", "fail", "buf", "buf.proto"),
 		"--config",
diff --git a/private/buf/cmd/buf/workspace_test.go b/private/buf/cmd/buf/workspace_test.go
index 8cf47e735..4c9e0479a 100644
--- a/private/buf/cmd/buf/workspace_test.go
+++ b/private/buf/cmd/buf/workspace_test.go
@@ -161,12 +161,12 @@ func TestWorkspaceDir(t *testing.T) {
 			"--against",
 			filepath.Join("testdata", "workspace", "success", baseDirPath),
 		)
-		testRunStdout(
+		testRunStdoutStderr(
 			t,
 			nil,
-			bufcli.ExitCodeFileAnnotation,
-			filepath.FromSlash(`testdata/workspace/success/`+baseDirPath+`/other/proto/request.proto:3:1:Files with package "request" must be within a directory "request" relative to root but were in directory ".".
-		    testdata/workspace/success/`+baseDirPath+`/proto/rpc.proto:3:1:Files with package "example" must be within a directory "example" relative to root but were in directory ".".`),
+			1,
+			"", // stdout should be empty
+			"Failure: the --config flag is not compatible with workspaces",
 			"lint",
 			filepath.Join("testdata", "workspace", "success", baseDirPath),
 			"--config",
@@ -194,23 +194,12 @@ func TestWorkspaceDir(t *testing.T) {
 			"--path",
 			filepath.Join("testdata", "workspace", "success", baseDirPath, "other", "proto", "request.proto"),
 		)
-		testRunStdout(
+		testRunStdoutStderr(
 			t,
 			nil,
-			bufcli.ExitCodeFileAnnotation,
-			filepath.FromSlash(`testdata/workspace/success/`+baseDirPath+`/proto/rpc.proto:3:1:Files with package "example" must be within a directory "example" relative to root but were in directory ".".`),
-			"lint",
-			filepath.Join("testdata", "workspace", "success", baseDirPath),
-			"--config",
-			`{"version":"v1","lint": {"use": ["PACKAGE_DIRECTORY_MATCH"]}}`,
-			"--path",
-			filepath.Join("testdata", "workspace", "success", baseDirPath, "proto", "rpc.proto"),
-		)
-		testRunStdout(
-			t,
-			nil,
-			bufcli.ExitCodeFileAnnotation,
-			filepath.FromSlash(`testdata/workspace/success/`+baseDirPath+`/other/proto/request.proto:3:1:Files with package "request" must be within a directory "request" relative to root but were in directory ".".`),
+			1,
+			"", // stdout should be empty
+			"Failure: the --config flag is not compatible with workspaces",
 			"lint",
 			filepath.Join("testdata", "workspace", "success", baseDirPath),
 			"--config",
@@ -218,32 +207,6 @@ func TestWorkspaceDir(t *testing.T) {
 			"--path",
 			filepath.Join("testdata", "workspace", "success", baseDirPath, "other", "proto", "request.proto"),
 		)
-		testRunStdout(
-			t,
-			nil,
-			bufcli.ExitCodeFileAnnotation,
-			filepath.FromSlash(`testdata/workspace/success/`+baseDirPath+`/other/proto/request.proto:3:1:Files with package "request" must be within a directory "request" relative to root but were in directory ".".`),
-			"lint",
-			filepath.Join("testdata", "workspace", "success", baseDirPath),
-			"--config",
-			`{"version":"v1","lint": {"use": ["PACKAGE_DIRECTORY_MATCH"]}}`,
-			"--path",
-			filepath.Join(wd, "testdata", "workspace", "success", baseDirPath, "other", "proto", "request.proto"),
-		)
-		testRunStdout(
-			t,
-			nil,
-			bufcli.ExitCodeFileAnnotation,
-			filepath.FromSlash(fmt.Sprintf(`%s/testdata/workspace/success/`+baseDirPath+`/other/proto/request.proto:3:1:Files with package "request" must be within a directory "request" relative to root but were in directory ".".`,
-				wd,
-			)),
-			"lint",
-			filepath.Join(wd, "testdata", "workspace", "success", baseDirPath),
-			"--config",
-			`{"version":"v1","lint": {"use": ["PACKAGE_DIRECTORY_MATCH"]}}`,
-			"--path",
-			filepath.Join(wd, "testdata", "workspace", "success", baseDirPath, "other", "proto", "request.proto"),
-		)
 	}
 }