Skip to content

stdlib packages incorrectly classified as external test packages by gopackagesdriver #354

Description

@hartblanc

When calling the gopackages driver, if it cannot find stdlib packages via dependencies (typically via a go_stdlib rule). It falls back to calling go list. It achieves this by checking whether there is any package already present with the ID "runtime". In this case it calls packageinfo.FromBuildPackage with the response from go list.

There is a bug in packageinfo.FromBuildPackage.
packageinfo.FromBuildPacakge identifies whether there are any external tests in the build package, which some do (e.g. runtime), and if there are any, categorises the entire build.Package as an external test package, and appends "_test" to the name and ID. The bug is that only the external test package is included in the response, the primary package is missing.

This causes two main issues.

  1. The fallback mechanism for calling go list kicks in when it shouldn't, because there is no package found called runtime only runtime_test. This could lead to surprising results if the user has different versions of go being used for go list and the go_stdlib rule.
  2. Packages which depend on the primary package (e.g.time) in the response will include the package with the "_test" suffix in the imports ID. However, there will be no valid package with that ID in the response (only e.g. time_test).

The second issue causes tools which rely on those dependencies being present to fail.

For example, running testifylint with the go packages driver on a file with stdlib dependencies surfaces this error message from go/packages.

/home/callum/core3/src/common/go/api/validate/validate.go:5:2: could not import errors (missing package: "errors")
/home/callum/core3/src/common/go/api/validate/validate.go:6:2: could not import net/url (missing package: "net/url")
/home/callum/core3/src/common/go/api/validate/validate.go:7:2: could not import time (missing package: "time")
/home/callum/core3/src/common/go/api/validate/validate.go:9:2: could not import google.golang.org/protobuf/types/known/timestamppb (missing package: "google.golang.org/protobuf/types/known/timestamppb")
testifylint: analysis skipped due to errors in package

I think the same thing is probably happening for go_module rules, as they follow roughly the same codepath.
I think probably the sensible thing to address this issue is to simply construct the package using only the Go files, rather than any test files. We can safely ignore any test files in go_module and go_stdlib as we don't depend on tests. I have applied this changes in my fork and it appears to resolve the issue.

I believe this bug was introduced in 5814cac.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions