.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>

.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) }}

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

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
+}