Skip to content

Commit

Permalink
fix(gpd): keep compiled files from stdlib packages (#4114)
Browse files Browse the repository at this point in the history
**What type of PR is this?**

Bug fix

**What does this PR do? Why is it needed?**

When we create a new dedicated package for all external tests, we should
not override the list of CompiledGoFiles by the list of GoFiles.

**Which issues(s) does this PR fix?**

Fixes: #4113
  • Loading branch information
lbcjbb authored Sep 21, 2024
1 parent 8b8294b commit dde2c1d
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 15 deletions.
14 changes: 10 additions & 4 deletions go/tools/gopackagesdriver/flatpackage.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ import (
"fmt"
"go/parser"
"go/token"
"io"
"os"
"strconv"
"strings"
"io"
)

type ResolvePkgFunc func(importPath string) string
Expand Down Expand Up @@ -134,12 +134,18 @@ func (fp *FlatPackage) filterTestSuffix(files []string) (err error, testFiles []

func (fp *FlatPackage) MoveTestFiles() *FlatPackage {
err, tgf, xtgf, gf := fp.filterTestSuffix(fp.GoFiles)

if err != nil {
return nil
}

fp.GoFiles = append(gf, tgf...)
fp.CompiledGoFiles = append(gf, tgf...)

err, ctgf, cxtgf, cgf := fp.filterTestSuffix(fp.CompiledGoFiles)
if err != nil {
return nil
}

fp.CompiledGoFiles = append(cgf, ctgf...)

if len(xtgf) == 0 {
return nil
Expand All @@ -160,7 +166,7 @@ func (fp *FlatPackage) MoveTestFiles() *FlatPackage {
Imports: newImports,
Errors: fp.Errors,
GoFiles: append([]string{}, xtgf...),
CompiledGoFiles: append([]string{}, xtgf...),
CompiledGoFiles: append([]string{}, cxtgf...),
OtherFiles: fp.OtherFiles,
ExportFile: fp.ExportFile,
Standard: fp.Standard,
Expand Down
68 changes: 57 additions & 11 deletions go/tools/gopackagesdriver/gopackagesdriver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,16 @@ import (
"bytes"
"context"
"encoding/json"
"fmt"
"os"
"path"
"path/filepath"
"strings"
"testing"

"github.com/bazelbuild/rules_go/go/tools/bazel_testing"
)

type response struct {
Roots []string `json:",omitempty"`
Packages []*FlatPackage
}

func TestMain(m *testing.M) {
bazel_testing.TestMain(m, bazel_testing.Args{
Main: `
Expand Down Expand Up @@ -81,6 +78,29 @@ func main() {
fmt.Fprintln(os.Stderr, "Subdirectory Hello World!")
}
`,
// Explicitly disable bzlmod which is enabled by default with bazel >=7.0.
// Remove this setup function when the test will be launched with bzlmod and
// the RULES_GO_REPO_NAME_FOR_TEST will be the canonical name of this rules_go module.
// This is required to match the stdlib label returned by the 'bazel query --consistent_labels',
// otherwise the gopackagesdriver will not return any stdlib packages.
SetUp: func() error {
f, err := os.OpenFile(".bazelrc", os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644)
if err != nil {
return fmt.Errorf("can't open bazelrc file: %v", err)
}

_, err = fmt.Fprintf(f, "common --noexperimental_enable_bzlmod\n")
if err != nil {
_ = f.Close()
return fmt.Errorf("can't update bazelrc file: %v", err)
}

if err := f.Close(); err != nil {
return fmt.Errorf("can't close bazelrc file: %v", err)
}

return nil
},
})
}

Expand All @@ -89,6 +109,33 @@ const (
bzlmodOsPkgID = "@@io_bazel_rules_go//stdlib:os"
)

func TestStdlib(t *testing.T) {
resp := runForTest(t, DriverRequest{}, ".", "std")

if len(resp.Packages) == 0 {
t.Fatal("Expected stdlib packages")
}

for _, pkg := range resp.Packages {
// net, plugin and user stdlib packages seem to have compiled files only for Linux.
if pkg.Name != "cgo" {
continue
}

var hasCompiledFiles bool
for _, x := range pkg.CompiledGoFiles {
if filepath.Ext(x) == "" {
hasCompiledFiles = true
break
}
}

if !hasCompiledFiles {
t.Errorf("%q stdlib package should have compiled files", pkg.Name)
}
}
}

func TestBaseFileLookup(t *testing.T) {
resp := runForTest(t, DriverRequest{}, ".", "file=hello.go")

Expand Down Expand Up @@ -255,21 +302,21 @@ func TestOverlay(t *testing.T) {
subhelloPath := path.Join(wd, "subhello/subhello.go")

expectedImportsPerFile := map[string][]string{
helloPath: []string{"fmt"},
helloPath: []string{"fmt"},
subhelloPath: []string{"os", "encoding/json"},
}

overlayDriverRequest := DriverRequest {
Overlay: map[string][]byte {
helloPath: []byte (`
overlayDriverRequest := DriverRequest{
Overlay: map[string][]byte{
helloPath: []byte(`
package hello
import "fmt"
import "unknown/unknown-package"
func main() {
invalid code
}`),
subhelloPath: []byte (`
subhelloPath: []byte(`
package subhello
import "os"
import "encoding/json"
Expand Down Expand Up @@ -301,7 +348,6 @@ func TestOverlay(t *testing.T) {
expectSetEquality(t, expectedImportsPerFile[subhelloPath], subhelloPkgImportPaths, "subhello imports")
}


func runForTest(t *testing.T, driverRequest DriverRequest, relativeWorkingDir string, args ...string) driverResponse {
t.Helper()

Expand Down

0 comments on commit dde2c1d

Please sign in to comment.