Skip to content

Updated test/gorepo/run.go's build constraint checker #1289

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

Merged
merged 1 commit into from
Apr 15, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 27 additions & 47 deletions tests/gorepo/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"errors"
"flag"
"fmt"
"go/build/constraint"
"hash/fnv"
"io"
"log"
Expand Down Expand Up @@ -366,6 +367,7 @@ func goFiles(dir string) []string {
f, err := os.Open(dir)
check(err)
dirnames, err := f.Readdirnames(-1)
f.Close()
check(err)
names := []string{}
for _, name := range dirnames {
Expand Down Expand Up @@ -520,53 +522,26 @@ func shouldTest(src string, goos, goarch string) (ok bool, whyNot string) {
}

for _, line := range strings.Split(src, "\n") {
line = strings.TrimSpace(line)
if strings.HasPrefix(line, "//") {
line = line[2:]
} else {
continue
}
line = strings.TrimSpace(line)
if len(line) == 0 || line[0] != '+' {
continue
if strings.HasPrefix(line, "package ") {
break
}
ctxt := &context{
GOOS: goos,
GOARCH: goarch,
}
words := strings.Fields(line)
if words[0] == "+build" {
ok := false
for _, word := range words[1:] {
if ctxt.match(word) {
ok = true
break
}
if expr, err := constraint.Parse(line); err == nil {
ctxt := &context{
GOOS: goos,
GOARCH: goarch,
}
if !ok {
// no matching tag found.
if !expr.Eval(ctxt.match) {
return false, line
}
}
}
// no build tags
return true, ""
}

func (ctxt *context) match(name string) bool {
if name == "" {
return false
}
if i := strings.Index(name, ","); i >= 0 {
// comma-separated list
return ctxt.match(name[:i]) && ctxt.match(name[i+1:])
}
if strings.HasPrefix(name, "!!") { // bad syntax, reject always
return false
}
if strings.HasPrefix(name, "!") { // negation
return len(name) > 1 && !ctxt.match(name[1:])
}

// Tags must be letters, digits, underscores or dots.
// Unlike in Go identifiers, all digits are fine (e.g., "386").
Expand All @@ -576,10 +551,18 @@ func (ctxt *context) match(name string) bool {
}
}

// GOPHERJS: Ignore "goexperiment." for now
// GOPHERJS: Don't match "cgo" since not supported
// GOPHERJS: Don't match "gc"
if name == ctxt.GOOS || name == ctxt.GOARCH {
return true
}

// GOPHERJS: Don't match "gcflags_noopt"
if name == "test_run" {
return true
}

return false
}

Expand Down Expand Up @@ -611,26 +594,23 @@ func (t *test) run() {
}

// Execution recipe stops at first blank line.
pos := strings.Index(t.src, "\n\n")
if pos == -1 {
t.err = errors.New("double newline not found")
action, _, ok := strings.Cut(t.src, "\n\n")
if !ok {
t.err = fmt.Errorf("double newline ending execution recipe not found in %s", t.goFileName())
return
}
action := t.src[:pos]
if nl := strings.Index(action, "\n"); nl >= 0 && strings.Contains(action[:nl], "+build") {
if firstLine, rest, ok := strings.Cut(action, "\n"); ok && strings.Contains(firstLine, "+build") {
// skip first line
action = action[nl+1:]
}
if strings.HasPrefix(action, "//") {
action = action[2:]
action = rest
}
action = strings.TrimPrefix(action, "//")

// Check for build constraints only up to the actual code.
pkgPos := strings.Index(t.src, "\npackage")
if pkgPos == -1 {
pkgPos = pos // some files are intentionally malformed
header, _, ok := strings.Cut(t.src, "\npackage")
if !ok {
header = action // some files are intentionally malformed
}
if ok, why := shouldTest(t.src[:pkgPos], goos, goarch); !ok {
if ok, why := shouldTest(header, goos, goarch); !ok {
t.action = "skip"
if *showSkips {
fmt.Printf("%-20s %-20s: %s\n", t.action, t.goFileName(), why)
Expand Down