Skip to content

Commit

Permalink
gopls/internal/cache: fix handling of cgo standalone files
Browse files Browse the repository at this point in the history
It is not a bug for go/packages to return no packages for standalone
files, if they use cgo and cgo is disabled. There may also be other
scenarios where standalone packages are not produced. In any case,
downgrade the bug report to a normal error.

Also, fix the failing test to require cgo.

Fixes golang/go#70363
Fixes golang/go#70362

Change-Id: I2b8a8b3947e4d7ff6482279540638e13dc9de2b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/628017
Reviewed-by: Tim King <taking@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
findleyr committed Nov 18, 2024
1 parent ed19fc7 commit 60bc93d
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 21 deletions.
26 changes: 5 additions & 21 deletions gopls/internal/cache/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,37 +163,21 @@ func (s *Snapshot) load(ctx context.Context, allowNetwork AllowNetwork, scopes .
// package. We don't support this; theoretically we could, but it seems
// unnecessarily complicated.
//
// Prior to golang/go#64233 we just assumed that we'd get exactly one
// package here. The categorization of bug reports below may be a bit
// verbose, but anticipates that perhaps we don't fully understand
// possible failure modes.
errorf := bug.Errorf
if s.view.typ == GoPackagesDriverView {
errorf = fmt.Errorf // all bets are off
}
for _, pkg := range pkgs {
// Don't report bugs if any packages have errors.
// For example: given go list errors, go/packages may synthesize a
// package with ID equal to the query.
if len(pkg.Errors) > 0 {
errorf = fmt.Errorf
break
}
}

// It's possible that we get no packages here, for example if the file is a
// cgo file and cgo is not enabled.
var standalonePkg *packages.Package
for _, pkg := range pkgs {
if pkg.ID == "command-line-arguments" {
if standalonePkg != nil {
return errorf("go/packages returned multiple standalone packages")
return fmt.Errorf("go/packages returned multiple standalone packages")
}
standalonePkg = pkg
} else if pkg.ForTest == "" && !strings.HasSuffix(pkg.ID, ".test") {
return errorf("go/packages returned unexpected package %q for standalone file", pkg.ID)
return fmt.Errorf("go/packages returned unexpected package %q for standalone file", pkg.ID)
}
}
if standalonePkg == nil {
return errorf("go/packages failed to return non-test standalone package")
return fmt.Errorf("go/packages failed to return non-test standalone package")
}
if len(standalonePkg.CompiledGoFiles) > 0 {
pkgs = []*packages.Package{standalonePkg}
Expand Down
6 changes: 6 additions & 0 deletions gopls/internal/test/marker/marker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,12 @@ func Test(t *testing.T) {
testenv.SkipAfterGoCommand1Point(t, go1point)
}
if test.cgo {
if os.Getenv("CGO_ENABLED") == "0" {
// NeedsTool causes the test to fail if cgo is available but disabled
// on the current platform through the environment. I'm not sure why it
// behaves this way, but if CGO_ENABLED=0 is set, we want to skip.
t.Skip("skipping due to CGO_ENABLED=0")
}
testenv.NeedsTool(t, "cgo")
}
config := fake.EditorConfig{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,4 @@ import "golang.org/lsptests/a"
func main() {
a.F() //@hovererr("F", "no package")
}

Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
This test checks that we can load standalone files that use cgo.

-- flags --
-cgo

-- go.mod --
module example.com

Expand Down

0 comments on commit 60bc93d

Please sign in to comment.