Skip to content

Commit bd074fa

Browse files
authored
gopackagesdriver: skip root packages that can't be built (#3806)
**What type of PR is this?** > Bug fix **What does this PR do? Why is it needed?** Omits package IDs from gopackagesdriver's roots list if we can't build package metadata for them. **Which issues(s) does this PR fix?** Fixes #3805 **Other notes for review**
1 parent 4d699f7 commit bd074fa

File tree

3 files changed

+67
-15
lines changed

3 files changed

+67
-15
lines changed

go/tools/gopackagesdriver/bazel.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"os"
2727
"os/exec"
2828
"path/filepath"
29+
"regexp"
2930
"strconv"
3031
"strings"
3132
)
@@ -149,18 +150,25 @@ func (b *Bazel) Build(ctx context.Context, args ...string) ([]string, error) {
149150
return files, nil
150151
}
151152

153+
var newlineRe = regexp.MustCompile(`\r?\n`)
154+
152155
func (b *Bazel) Query(ctx context.Context, args ...string) ([]string, error) {
153156
output, err := b.run(ctx, "query", args...)
154157
if err != nil {
155158
return nil, fmt.Errorf("bazel query failed: %w", err)
156159
}
157160

158-
trimmedOutput := strings.TrimSpace(output)
159-
if len(trimmedOutput) == 0 {
160-
return nil, nil
161+
lines := newlineRe.Split(output, -1)
162+
r, w := 0, 0
163+
for ; r < len(lines); r++ {
164+
line := strings.TrimSpace(lines[r])
165+
if line != "" {
166+
lines[w] = line
167+
w++
168+
}
161169
}
162-
163-
return strings.Split(trimmedOutput, "\n"), nil
170+
lines = lines[:w]
171+
return lines, nil
164172
}
165173

166174
func (b *Bazel) WorkspaceRoot() string {

go/tools/gopackagesdriver/gopackagesdriver_test.go

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,19 @@ go_library(
2727
)
2828
2929
go_test(
30-
name = "hello_test",
31-
srcs = [
32-
"hello_test.go",
33-
"hello_external_test.go",
34-
],
35-
embed = [":hello"],
30+
name = "hello_test",
31+
srcs = [
32+
"hello_test.go",
33+
"hello_external_test.go",
34+
],
35+
embed = [":hello"],
36+
)
37+
38+
go_library(
39+
name = "incompatible",
40+
srcs = ["incompatible.go"],
41+
importpath = "example.com/incompatible",
42+
target_compatible_with = ["@platforms//:incompatible"],
3643
)
3744
3845
-- hello.go --
@@ -58,6 +65,10 @@ import "testing"
5865
5966
func TestHelloExternal(t *testing.T) {}
6067
68+
-- incompatible.go --
69+
//go:build ignore
70+
package hello
71+
6172
-- subhello/BUILD.bazel --
6273
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
6374
@@ -76,7 +87,7 @@ import "os"
7687
func main() {
7788
fmt.Fprintln(os.Stderr, "Subdirectory Hello World!")
7889
}
79-
`,
90+
`,
8091
})
8192
}
8293

@@ -278,8 +289,8 @@ func TestOverlay(t *testing.T) {
278289
subhelloPath := path.Join(wd, "subhello/subhello.go")
279290

280291
expectedImportsPerFile := map[string][]string{
281-
helloPath: []string{"fmt"},
282-
subhelloPath: []string{"os", "encoding/json"},
292+
helloPath: {"fmt"},
293+
subhelloPath: {"os", "encoding/json"},
283294
}
284295

285296
overlayDriverRequest := DriverRequest{
@@ -324,6 +335,33 @@ func TestOverlay(t *testing.T) {
324335
expectSetEquality(t, expectedImportsPerFile[subhelloPath], subhelloPkgImportPaths, "subhello imports")
325336
}
326337

338+
// TestIncompatible checks that a target that can be queried but not analyzed
339+
// does not appear in .Roots.
340+
func TestIncompatible(t *testing.T) {
341+
resp := runForTest(t, DriverRequest{}, ".", "./...")
342+
343+
rootLabels := make(map[string]bool)
344+
for _, root := range resp.Roots {
345+
rootLabels[root] = true
346+
}
347+
348+
// Verify //:hello is in .Roots and check whether its label starts with
349+
// "@@" (bzlmod) or "@" (not bzlmod).
350+
var incompatibleLabel string
351+
if rootLabels["@@//:hello"] {
352+
incompatibleLabel = "@@//:incompatible"
353+
} else if rootLabels["@//:hello"] {
354+
incompatibleLabel = "@//:incompatible"
355+
} else {
356+
t.Fatalf("response does not contain //:hello; roots were %s", strings.Join(resp.Roots, ", "))
357+
}
358+
359+
// Verify //:incompatible is NOT in .Roots.
360+
if rootLabels[incompatibleLabel] {
361+
t.Fatalf("response contains root %s", incompatibleLabel)
362+
}
363+
}
364+
327365
func runForTest(t *testing.T, driverRequest DriverRequest, relativeWorkingDir string, args ...string) driverResponse {
328366
t.Helper()
329367

go/tools/gopackagesdriver/packageregistry.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,18 @@ func (pr *PackageRegistry) Match(labels []string) ([]string, []*FlatPackage) {
115115
roots[pkg.ID] = struct{}{}
116116
}
117117
}
118-
} else {
118+
} else if _, ok := pr.packagesByID[label]; ok {
119119
roots[label] = struct{}{}
120120
// If an xtest package exists for this package add it to the roots
121121
if _, ok := pr.packagesByID[label+"_xtest"]; ok {
122122
roots[label+"_xtest"] = struct{}{}
123123
}
124+
} else {
125+
// Skip a package if we don't have .pkg.json for it.
126+
// This happens if 'bazel query' matches the target, but 'bazel build'
127+
// can't analyze it, for example, if target_compatible_with is set
128+
// with contraints not compatible with the host platform.
129+
continue
124130
}
125131
}
126132

0 commit comments

Comments
 (0)