summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorBrad Fitzpatrick <bradfitz@tailscale.com>2025-06-16 21:10:59 -0700
committerBrad Fitzpatrick <brad@danga.com>2025-06-17 07:37:10 -0700
commitd37e8d0bfaaf3a40ef2432f6bed7bab2004e36eb (patch)
tree94b5fb660454ea9a99895b023a9eea531b64c659
parent42f71e959dff4dc55b138c358764c8fbfe8cdb7f (diff)
downloadtailscale-d37e8d0bfaaf3a40ef2432f6bed7bab2004e36eb.tar.xz
tailscale-d37e8d0bfaaf3a40ef2432f6bed7bab2004e36eb.zip
.github/workflows: remove redundant work between staticcheck jobs
Make the OS-specific staticcheck jobs only test stuff that's specialized for that OS. Do that using a new ./tool/listpkgs program that's a fancy 'go list' with more filtering flags. Updates tailscale/corp#28679 Change-Id: I790be2e3a0b42b105bd39f68c4b20e217a26de60 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
-rw-r--r--.github/workflows/test.yml87
-rw-r--r--Makefile2
-rw-r--r--tool/listpkgs/listpkgs.go206
3 files changed, 283 insertions, 12 deletions
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 313ce609f..6d8ab863c 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -232,10 +232,6 @@ jobs:
- name: Restore Cache
uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
with:
- # Note: unlike the other setups, this is only grabbing the mod download
- # cache, rather than the whole mod directory, as the download cache
- # contains zips that can be unpacked in parallel faster than they can be
- # fetched and extracted by tar
path: |
~/.cache/go-build
~\AppData\Local\go-build
@@ -722,14 +718,40 @@ jobs:
staticcheck:
runs-on: ubuntu-24.04
needs: gomod-cache
+ name: staticcheck (${{ matrix.name }})
strategy:
fail-fast: false # don't abort the entire matrix if one element fails
matrix:
- goos: ["linux", "windows", "darwin"]
- goarch: ["amd64"]
include:
- - goos: "windows"
- goarch: "386"
+ - name: "macOS"
+ goos: "darwin"
+ goarch: "arm64"
+ flags: "--with-tags-all=darwin"
+ - name: "Windows"
+ goos: "windows"
+ goarch: "amd64"
+ flags: "--with-tags-all=windows"
+ - name: "Linux"
+ goos: "linux"
+ goarch: "amd64"
+ flags: "--with-tags-all=linux"
+ - name: "Portable (1/4)"
+ goos: "linux"
+ goarch: "amd64"
+ flags: "--without-tags-any=windows,darwin,linux --shard=1/4"
+ - name: "Portable (2/4)"
+ goos: "linux"
+ goarch: "amd64"
+ flags: "--without-tags-any=windows,darwin,linux --shard=2/4"
+ - name: "Portable (3/4)"
+ goos: "linux"
+ goarch: "amd64"
+ flags: "--without-tags-any=windows,darwin,linux --shard=3/4"
+ - name: "Portable (4/4)"
+ goos: "linux"
+ goarch: "amd64"
+ flags: "--without-tags-any=windows,darwin,linux --shard=4/4"
+
steps:
- name: checkout
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
@@ -741,14 +763,14 @@ jobs:
path: gomodcache
key: ${{ needs.gomod-cache.outputs.cache-key }}
enableCrossOsArchive: true
- - name: run staticcheck
+ - name: run staticcheck (${{ matrix.name }})
working-directory: src
run: |
export GOROOT=$(./tool/go env GOROOT)
./tool/go run -exec \
"env GOOS=${{ matrix.goos }} GOARCH=${{ matrix.goarch }}" \
honnef.co/go/tools/cmd/staticcheck -- \
- $(env GOOS=${{ matrix.goos }} GOARCH=${{ matrix.goarch }} ./tool/go list ./... | grep -v tempfork)
+ $(./tool/go run ./tool/listpkgs --ignore-3p --goos=${{ matrix.goos }} --goarch=${{ matrix.goarch }} ${{ matrix.flags }} ./...)
notify_slack:
if: always()
@@ -795,7 +817,7 @@ jobs:
}]
}
- check_mergeability:
+ merge_blocker:
if: always()
runs-on: ubuntu-24.04
needs:
@@ -819,3 +841,46 @@ jobs:
uses: re-actors/alls-green@05ac9388f0aebcb5727afa17fcccfecd6f8ec5fe # v1.2.2
with:
jobs: ${{ toJSON(needs) }}
+
+ # This waits on all the jobs which must never fail. Branch protection rules
+ # enforce these. No flaky tests are allowed in these jobs. (We don't want flaky
+ # tests anywhere, really, but a flaky test here prevents merging.)
+ check_mergeability_strict:
+ if: always()
+ runs-on: ubuntu-24.04
+ needs:
+ - android
+ - cross
+ - crossmin
+ - ios
+ - tailscale_go
+ - depaware
+ - go_generate
+ - go_mod_tidy
+ - licenses
+ - staticcheck
+ steps:
+ - name: Decide if change is okay to merge
+ if: github.event_name != 'push'
+ uses: re-actors/alls-green@05ac9388f0aebcb5727afa17fcccfecd6f8ec5fe # v1.2.2
+ with:
+ jobs: ${{ toJSON(needs) }}
+
+ check_mergeability:
+ if: always()
+ runs-on: ubuntu-24.04
+ needs:
+ - check_mergeability_strict
+ - test
+ - windows
+ - vm
+ - wasm
+ - fuzz
+ - race-root-integration
+ - privileged
+ steps:
+ - name: Decide if change is okay to merge
+ if: github.event_name != 'push'
+ uses: re-actors/alls-green@05ac9388f0aebcb5727afa17fcccfecd6f8ec5fe # v1.2.2
+ with:
+ jobs: ${{ toJSON(needs) }}
diff --git a/Makefile b/Makefile
index 1978af90d..41c67c711 100644
--- a/Makefile
+++ b/Makefile
@@ -64,7 +64,7 @@ buildmultiarchimage: ## Build (and optionally push) multiarch docker image
check: staticcheck vet depaware buildwindows build386 buildlinuxarm buildwasm ## Perform basic checks and compilation tests
staticcheck: ## Run staticcheck.io checks
- ./tool/go run honnef.co/go/tools/cmd/staticcheck -- $$(./tool/go list ./... | grep -v tempfork)
+ ./tool/go run honnef.co/go/tools/cmd/staticcheck -- $$(./tool/go run ./tool/listpkgs --ignore-3p ./...)
kube-generate-all: kube-generate-deepcopy ## Refresh generated files for Tailscale Kubernetes Operator
./tool/go generate ./cmd/k8s-operator
diff --git a/tool/listpkgs/listpkgs.go b/tool/listpkgs/listpkgs.go
new file mode 100644
index 000000000..400bf90c1
--- /dev/null
+++ b/tool/listpkgs/listpkgs.go
@@ -0,0 +1,206 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+// listpkgs prints the import paths that match the Go package patterns
+// given on the command line and conditionally filters them in various ways.
+package main
+
+import (
+ "bufio"
+ "flag"
+ "fmt"
+ "go/build/constraint"
+ "log"
+ "os"
+ "slices"
+ "strings"
+ "sync"
+
+ "golang.org/x/tools/go/packages"
+)
+
+var (
+ ignore3p = flag.Bool("ignore-3p", false, "ignore third-party packages forked/vendored into Tailscale")
+ goos = flag.String("goos", "", "GOOS to use for loading packages (default: current OS)")
+ goarch = flag.String("goarch", "", "GOARCH to use for loading packages (default: current architecture)")
+ withTagsAllStr = flag.String("with-tags-all", "", "if non-empty, a comma-separated list of builds tags to require (a package will only be listed if it contains all of these build tags)")
+ withoutTagsAnyStr = flag.String("without-tags-any", "", "if non-empty, a comma-separated list of build constraints to exclude (a package will be omitted if it contains any of these build tags)")
+ shard = flag.String("shard", "", "if non-empty, a string of the form 'N/M' to only print packages in shard N of M (e.g. '1/3', '2/3', '3/3/' for different thirds of the list)")
+)
+
+func main() {
+ flag.Parse()
+
+ patterns := flag.Args()
+ if len(patterns) == 0 {
+ flag.Usage()
+ os.Exit(1)
+ }
+
+ cfg := &packages.Config{
+ Mode: packages.LoadFiles,
+ Env: os.Environ(),
+ }
+ if *goos != "" {
+ cfg.Env = append(cfg.Env, "GOOS="+*goos)
+ }
+ if *goarch != "" {
+ cfg.Env = append(cfg.Env, "GOARCH="+*goarch)
+ }
+
+ pkgs, err := packages.Load(cfg, patterns...)
+ if err != nil {
+ log.Fatalf("loading packages: %v", err)
+ }
+
+ var withoutAny []string
+ if *withoutTagsAnyStr != "" {
+ withoutAny = strings.Split(*withoutTagsAnyStr, ",")
+ }
+ var withAll []string
+ if *withTagsAllStr != "" {
+ withAll = strings.Split(*withTagsAllStr, ",")
+ }
+
+ seen := map[string]bool{}
+ matches := 0
+Pkg:
+ for _, pkg := range pkgs {
+ if pkg.PkgPath == "" { // malformed (shouldn’t happen)
+ continue
+ }
+ if seen[pkg.PkgPath] {
+ continue // suppress duplicates when patterns overlap
+ }
+ seen[pkg.PkgPath] = true
+
+ pkgPath := pkg.PkgPath
+
+ if *ignore3p && isThirdParty(pkgPath) {
+ continue
+ }
+ if withAll != nil {
+ for _, t := range withAll {
+ if !hasBuildTag(pkg, t) {
+ continue Pkg
+ }
+ }
+ }
+ for _, t := range withoutAny {
+ if hasBuildTag(pkg, t) {
+ continue Pkg
+ }
+ }
+ matches++
+
+ if *shard != "" {
+ var n, m int
+ if _, err := fmt.Sscanf(*shard, "%d/%d", &n, &m); err != nil || n < 1 || m < 1 {
+ log.Fatalf("invalid shard format %q; expected 'N/M'", *shard)
+ }
+ if m > 0 && (matches-1)%m != n-1 {
+ continue // not in this shard
+ }
+ }
+ fmt.Println(pkgPath)
+ }
+
+ // If any package had errors (e.g. missing deps) report them via packages.PrintErrors.
+ // This mirrors `go list` behaviour when -e is *not* supplied.
+ if packages.PrintErrors(pkgs) > 0 {
+ os.Exit(1)
+ }
+}
+
+func isThirdParty(pkg string) bool {
+ return strings.HasPrefix(pkg, "tailscale.com/tempfork/")
+}
+
+// hasBuildTag reports whether any source file in pkg mentions `tag`
+// in a //go:build constraint.
+func hasBuildTag(pkg *packages.Package, tag string) bool {
+ all := slices.Concat(pkg.CompiledGoFiles, pkg.OtherFiles, pkg.IgnoredFiles)
+ suffix := "_" + tag + ".go"
+ for _, name := range all {
+ if strings.HasSuffix(name, suffix) {
+ return true
+ }
+ ok, err := fileMentionsTag(name, tag)
+ if err != nil {
+ log.Printf("reading %s: %v", name, err)
+ continue
+ }
+ if ok {
+ return true
+ }
+ }
+ return false
+}
+
+// tagSet is a set of build tags.
+// The values are always true. We avoid non-std set types
+// to make this faster to "go run" on empty caches.
+type tagSet map[string]bool
+
+var (
+ mu sync.Mutex
+ fileTags = map[string]tagSet{} // abs path -> set of build tags mentioned in file
+)
+
+func getFileTags(filename string) (tagSet, error) {
+ mu.Lock()
+ tags, ok := fileTags[filename]
+ mu.Unlock()
+ if ok {
+ return tags, nil
+ }
+
+ f, err := os.Open(filename)
+ if err != nil {
+ return nil, err
+ }
+ defer f.Close()
+
+ ts := make(tagSet)
+ s := bufio.NewScanner(f)
+ for s.Scan() {
+ line := s.Text()
+ if strings.TrimSpace(line) == "" {
+ continue // still in leading blank lines
+ }
+ if !strings.HasPrefix(line, "//") {
+ // hit real code – done with header comments
+ // TODO(bradfitz): care about /* */ comments?
+ break
+ }
+ if !strings.HasPrefix(line, "//go:build") {
+ continue // some other comment
+ }
+ expr, err := constraint.Parse(line)
+ if err != nil {
+ return nil, fmt.Errorf("parsing %q: %w", line, err)
+ }
+ // Call Eval to populate ts with the tags mentioned in the expression.
+ // We don't care about the result, just the side effect of populating ts.
+ expr.Eval(func(tag string) bool {
+ ts[tag] = true
+ return true // arbitrary
+ })
+ }
+ if err := s.Err(); err != nil {
+ return nil, fmt.Errorf("reading %s: %w", filename, err)
+ }
+
+ mu.Lock()
+ defer mu.Unlock()
+ fileTags[filename] = ts
+ return tags, nil
+}
+
+func fileMentionsTag(filename, tag string) (bool, error) {
+ tags, err := getFileTags(filename)
+ if err != nil {
+ return false, err
+ }
+ return tags[tag], nil
+}