runner: detect GPUs on Windows via Win32_VideoController (#39) #40

Merged

linus merged 3 commits from ash/windows-gpu-detect-39 into develop

2026-11-01 00:36:42 +00:00

ash commented

2026-11-01 00:15:07 +00:00

Collaborator

Closes #39

Why

On Windows the runner had no host GPU probe (only /sys/class/drm and lspci), so DetectGPUBackends() returned ok=false and every job was forced to CPU. That blocks the ai1 Windows 520-560 render test.

What changed

internal/runner/blender/detect.go: probes are now an ordered list (runGPUProbes). New DetectGPUs() returns vendors plus probe source and device names; DetectGPUBackends() keeps its signature and wraps it.
Build-tagged probe lists: detect_probes_other.go (!windows) = DRM then lspci, same order and same functions as before. detect_probes_windows.go = Get-CimInstance Win32_VideoController via PowerShell, parsed by parseWin32VideoControllers (AdapterCompatibility|Name; maps AMD/ATI/Radeon, NVIDIA, Intel; ignores Microsoft Basic/Remote Display adapters).
runner.go: logs GPU devices detected via <source>: <devices> at startup, before the existing GPU backend detection: AMD=... line.
zstd: runner and manager error is no longer apt-specific (Linux apt hint + Windows zstd.exe on PATH). README gains a "Windows runner" section (zstd.exe, --sandbox none, GPU detection log).
Linux refactor needed only for tests: DRM dir is a package var (drmClassDir, default /sys/class/drm) and lspci parsing moved into parseLSPCI (same body).

Acceptance criteria (Mira)

Windows detects NVIDIA/AMD via Win32_VideoController and does not force CPU: Windows probe returns ok=true for AMD/NVIDIA adapters, so gpuDetectionFailed stays false. Unit tested; real-host proof pending (see Gap).
Linux unchanged: DRM then lspci, identical matching. New tests pin it with a fake sysfs tree (card nodes, connector/renderD skipped, unknown vendor, missing dir), lspci parsing, probe order DRM→lspci on non-Windows, DRM hit short-circuits lspci, fallthrough to lspci. One test deliberately pins the existing lspci quirk ("NVIDIA Corporation" also sets AMD because "corporATIon" contains "ati") so this PR can't silently change it; fixing that is a separate issue if wanted. Existing detect tests unchanged and passing.
Windows parsing unit tests: detect_windows_test.go (Radeon 8060S + Basic Display adapter, NVIDIA + Intel, no-GPU inputs incl. CRLF/empty, vendor-only name). Platform-neutral, so they run in Linux CI.
Startup log of detected device: GPU devices detected via win32_videocontroller: AMD Radeon(TM) 8060S Graphics (Linux: via drm: AMD etc.).
On merge: merger posts PR + merge commit on #39 for Jerry.

Testing

go test -race -count=3 -shuffle=on ./internal/runner/... ./internal/manager/... on the jumpbox: all pass except internal/runner/encoding TestIntegration_Encode_EXR_H264, which needs ffmpeg (not installed on the jumpbox; CI installs it).
internal/runner/blender: 24 tests pass.
Win32 parser tests now run in Linux CI as of 677d240 (after review 1087 renamed detect_windows_test.go to detect_win32_parse_test.go). Jumpbox go test -v -run Win32 ./internal/runner/blender/:

--- PASS: TestParseWin32VideoControllersAMD (0.00s)
--- PASS: TestParseWin32VideoControllersNVIDIAAndIntel (0.00s)
--- PASS: TestParseWin32VideoControllersNoGPU (0.00s)
--- PASS: TestParseWin32VideoControllersVendorOnlyName (0.00s)
ok  	jiggablend/internal/runner/blender	0.002s

CGO_ENABLED=0 GOOS=windows GOARCH=amd64 go build ./cmd/jiggablend and GOOS=windows go vet ./internal/runner/... clean.
This repo's CI is ubuntu-latest only (no Windows job), so the windows build-tag file is compiled-checked locally, not run in CI.

Gap

Not run on real Windows hardware. The PowerShell call and Jerry's issue acceptance (render log "Render devices (GPU only, 1 enabled) ... [HIP]" / "Final Cycles device: GPU") are proven only by Jerry's post-merge ai1 Windows 520-560 test.

Risk / rollback

Windows-only behavior change (build tag). Linux path is the same functions in the same order plus one extra log line. Rollback: revert this PR.

Closes #39 ## Why On Windows the runner had no host GPU probe (only `/sys/class/drm` and `lspci`), so `DetectGPUBackends()` returned ok=false and every job was forced to CPU. That blocks the ai1 Windows 520-560 render test. ## What changed - `internal/runner/blender/detect.go`: probes are now an ordered list (`runGPUProbes`). New `DetectGPUs()` returns vendors plus probe source and device names; `DetectGPUBackends()` keeps its signature and wraps it. - Build-tagged probe lists: `detect_probes_other.go` (`!windows`) = DRM then lspci, same order and same functions as before. `detect_probes_windows.go` = `Get-CimInstance Win32_VideoController` via PowerShell, parsed by `parseWin32VideoControllers` (AdapterCompatibility|Name; maps AMD/ATI/Radeon, NVIDIA, Intel; ignores Microsoft Basic/Remote Display adapters). - `runner.go`: logs `GPU devices detected via <source>: <devices>` at startup, before the existing `GPU backend detection: AMD=... ` line. - zstd: runner and manager error is no longer apt-specific (Linux apt hint + Windows zstd.exe on PATH). README gains a "Windows runner" section (zstd.exe, `--sandbox none`, GPU detection log). - Linux refactor needed only for tests: DRM dir is a package var (`drmClassDir`, default `/sys/class/drm`) and lspci parsing moved into `parseLSPCI` (same body). ## Acceptance criteria (Mira) 1. Windows detects NVIDIA/AMD via Win32_VideoController and does not force CPU: Windows probe returns ok=true for AMD/NVIDIA adapters, so `gpuDetectionFailed` stays false. Unit tested; real-host proof pending (see Gap). 2. Linux unchanged: DRM then lspci, identical matching. New tests pin it with a fake sysfs tree (card nodes, connector/renderD skipped, unknown vendor, missing dir), lspci parsing, probe order DRM→lspci on non-Windows, DRM hit short-circuits lspci, fallthrough to lspci. One test deliberately pins the existing lspci quirk ("NVIDIA Corporation" also sets AMD because "corporATIon" contains "ati") so this PR can't silently change it; fixing that is a separate issue if wanted. Existing detect tests unchanged and passing. 3. Windows parsing unit tests: `detect_windows_test.go` (Radeon 8060S + Basic Display adapter, NVIDIA + Intel, no-GPU inputs incl. CRLF/empty, vendor-only name). Platform-neutral, so they run in Linux CI. 4. Startup log of detected device: `GPU devices detected via win32_videocontroller: AMD Radeon(TM) 8060S Graphics` (Linux: `via drm: AMD` etc.). 5. On merge: merger posts PR + merge commit on #39 for Jerry. ## Testing - `go test -race -count=3 -shuffle=on ./internal/runner/... ./internal/manager/...` on the jumpbox: all pass except `internal/runner/encoding` TestIntegration_Encode_EXR_H264, which needs ffmpeg (not installed on the jumpbox; CI installs it). - `internal/runner/blender`: 24 tests pass. - Win32 parser tests now run in Linux CI as of 677d240 (after review 1087 renamed `detect_windows_test.go` to `detect_win32_parse_test.go`). Jumpbox `go test -v -run Win32 ./internal/runner/blender/`: ``` --- PASS: TestParseWin32VideoControllersAMD (0.00s) --- PASS: TestParseWin32VideoControllersNVIDIAAndIntel (0.00s) --- PASS: TestParseWin32VideoControllersNoGPU (0.00s) --- PASS: TestParseWin32VideoControllersVendorOnlyName (0.00s) ok jiggablend/internal/runner/blender 0.002s ``` - `CGO_ENABLED=0 GOOS=windows GOARCH=amd64 go build ./cmd/jiggablend` and `GOOS=windows go vet ./internal/runner/...` clean. - This repo's CI is ubuntu-latest only (no Windows job), so the `windows` build-tag file is compiled-checked locally, not run in CI. ## Gap Not run on real Windows hardware. The PowerShell call and Jerry's issue acceptance (render log "Render devices (GPU only, 1 enabled) ... [HIP]" / "Final Cycles device: GPU") are proven only by Jerry's post-merge ai1 Windows 520-560 test. ## Risk / rollback Windows-only behavior change (build tag). Linux path is the same functions in the same order plus one extra log line. Rollback: revert this PR.

ash added 2 commits

2026-11-01 00:15:07 +00:00

runner: detect GPUs on Windows via Win32_VideoController (#39 ) 4f3d76db14

runner: pin existing lspci matching in tests, drop unrelated whitespace edits (#39 )

CI / check-and-test (pull_request) Successful in 37s

Details

815a0fa91b

ash requested review from eva

2026-11-01 00:15:07 +00:00

eva requested changes

2026-11-01 00:17:11 +00:00

Dismissed

eva left a comment

Close, but one thing needs fixing before this can go.

internal/runner/blender/detect_windows_test.go won't run in Linux CI. Go treats a _windows_test.go file name as a build constraint, so that file only compiles when GOOS=windows. The PR body says those parser tests are platform-neutral and run in CI, but on ubuntu-latest (and in your jumpbox run) they're silently skipped, so acceptance criterion 3 isn't actually covered yet.

Fix on this same branch: rename it to something without a GOOS suffix, for example detect_win32_parse_test.go. parseWin32VideoControllers lives in detect.go with no build tag, so the tests will compile and run on Linux as-is. Then confirm with go test -v -run Win32 ./internal/runner/blender/ that all four show up as PASS, and push.

detect_linux_test.go is fine to keep (Linux-only is what it tests), just know it's also suffix-gated.

Everything else looks good: base is current develop (77a22e2), CI is green, the Linux path keeps the same probes in the same order with tests pinning it, the Windows probe is behind a build tag, and the startup log line matches what Jerry needs. Not security-sensitive, so no Blake review needed. I'll re-review on the next push.

Close, but one thing needs fixing before this can go. `internal/runner/blender/detect_windows_test.go` won't run in Linux CI. Go treats a `_windows_test.go` file name as a build constraint, so that file only compiles when GOOS=windows. The PR body says those parser tests are platform-neutral and run in CI, but on ubuntu-latest (and in your jumpbox run) they're silently skipped, so acceptance criterion 3 isn't actually covered yet. Fix on this same branch: rename it to something without a GOOS suffix, for example `detect_win32_parse_test.go`. `parseWin32VideoControllers` lives in `detect.go` with no build tag, so the tests will compile and run on Linux as-is. Then confirm with `go test -v -run Win32 ./internal/runner/blender/` that all four show up as PASS, and push. `detect_linux_test.go` is fine to keep (Linux-only is what it tests), just know it's also suffix-gated. Everything else looks good: base is current develop (77a22e2), CI is green, the Linux path keeps the same probes in the same order with tests pinning it, the Windows probe is behind a build tag, and the startup log line matches what Jerry needs. Not security-sensitive, so no Blake review needed. I'll re-review on the next push.

ash added 1 commit

2026-11-01 00:33:58 +00:00

runner: run Win32 parser tests on all platforms (#39 )

CI / check-and-test (pull_request) Successful in 54s

Details

677d2407c6

eva approved these changes

2026-11-01 00:36:05 +00:00

eva left a comment

GO. The rename fixed it: detect_win32_parse_test.go has no GOOS suffix now, so the four Win32 parser tests compile and run in Linux CI, which covers acceptance criterion 3.

Re-checked on 677d240: base is still current develop (77a22e2), CI is green, the Linux path keeps DRM then lspci with the same matching and tests pinning it, the PowerShell probe sits behind the windows build tag, and the startup log line is what Jerry needs. The remaining gap is the one you already called out: no run on real Windows hardware yet, which is Jerry's ai1 test after merge.

Not security-sensitive, so no Blake review needed. Linus can merge, and on merge the PR and merge commit go on #39 for Jerry.

GO. The rename fixed it: `detect_win32_parse_test.go` has no GOOS suffix now, so the four Win32 parser tests compile and run in Linux CI, which covers acceptance criterion 3. Re-checked on 677d240: base is still current develop (77a22e2), CI is green, the Linux path keeps DRM then lspci with the same matching and tests pinning it, the PowerShell probe sits behind the `windows` build tag, and the startup log line is what Jerry needs. The remaining gap is the one you already called out: no run on real Windows hardware yet, which is Jerry's ai1 test after merge. Not security-sensitive, so no Blake review needed. Linus can merge, and on merge the PR and merge commit go on #39 for Jerry.