Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests cannot transitively depend on library #2583

Closed
linzhp opened this issue Jul 23, 2020 · 2 comments · Fixed by #3422
Closed

Tests cannot transitively depend on library #2583

linzhp opened this issue Jul 23, 2020 · 2 comments · Fixed by #3422
Labels

Comments

@linzhp
Copy link
Contributor

linzhp commented Jul 23, 2020

When the go_default_test of package A depends on a package B that depends on package A, rules_go removes the dependency of package B when compiling the go_default_test, causing compilation error.

deps

What version of rules_go are you using?

0.23.6

What version of gazelle are you using?

0.21.1

What version of Bazel are you using?

3.3.0

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

Darwin amd64

Any other potentially useful information about your toolchain?

What did you do?

$ git clone git@github.com:linzhp/bazel_examples.git
$ cd bazel_examples
$ git checkout 0f6b7a28a4fbd62b1868e25815f8a230d39d2994
$ bazel build //imports:go_default_test

What did you expect to see?

Build completed successfully

What did you see instead?

ERROR: /Users/zplin/gocode/src/github.com/linzhp/bazel_examples/imports/BUILD.bazel:10:8: GoCompilePkg imports/go_default_test.internal.recompileinternal.a failed (Exit 1) builder failed: error executing command bazel-out/host/bin/external/go_sdk/builder compilepkg -sdk external/go_sdk -installsuffix darwin_amd64 -src imports/client.go -src imports/client_test.go -importpath ... (remaining 17 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
compilepkg: missing strict dependencies:
	/private/var/tmp/_bazel_zplin/1b23f78923ad6c0028e8aab1a0a6d5fb/sandbox/darwin-sandbox/2/execroot/__main__/imports/client_test.go: import of "github.com/linzhp/bazel_examples/imports/testutils"
No dependencies were provided.
Check that imports in Go sources match importpath attributes in deps.
Target //imports:go_default_test failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 32.273s, Critical Path: 1.03s
INFO: 1 process: 1 darwin-sandbox.
FAILED: Build did NOT complete successfully
@jayconrod
Copy link
Contributor

This example has a cyclic dependency, so it can't be built as-is.

//imports:go_default_test just has an internal test: its sources are compiled with those from //imports:go_default_library into a single package. When //imports/testutils:go_default_library imports github.com/linzhp/bazel_examples/imports, it will be built against that archive instead of //imports:go_default_library.

Previously, before #2579 and rules_go 0.23.6, go_test did compile against //imports:go_default_library, but it would link against //imports:go_default_test. This was a bug, but it rarely caused problems (#1877). In Go 1.15, the new linker is stricter, so this would always be an error.

Just to compare go test behavior:

$ go test ./imports
# github.com/linzhp/bazel_examples/imports
package github.com/linzhp/bazel_examples/imports (test)
	imports github.com/linzhp/bazel_examples/imports/testutils
	imports github.com/linzhp/bazel_examples/imports: import cycle not allowed in test
FAIL	github.com/linzhp/bazel_examples/imports [setup failed]
FAIL

To avoid this issue, client_test.go must be part of the external test package (package imports_test). It may import the library directly or indirectly, and it may depend on exported definitions from the internal test.

@jayconrod
Copy link
Contributor

Marking this as a bug because the error message (missing strict dependencies) is not clear.

go_test doesn't know which dependencies are imported by the internal sources and which are imported by external sources, and it simply removes those that transitively depend on the library under test. Instead, it could pass those paths to the builder, and the builder could print a clear message when one of those packages is imported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants