fix(build): skip asm scan for synthetic test main - #2347
Conversation
There was a problem hiding this comment.
Review summary
This is a tight, well-scoped fix. pkgSFiles now short-circuits the synthetic test-main package (*.test) before the directory .s glob and the go list subprocess, which correctly avoids both false-positive assembly detection (the testmain shares the tested package's dir) and a guaranteed-to-fail go list on a non-listable generated import path. The added comment accurately explains the rationale, the guard caches nil consistently with the sibling early returns, and a focused test covers the happy path plus the cache entry.
Security: clean — subprocess uses exec.Command (no shell), inputs come from the toolchain's package graph, and only GOOS/GOARCH are added to the env.
Performance: a net win — the guard removes a filesystem glob and an external process on the test path.
Two non-blocking suggestions below (maintainability + test robustness). No blocking issues.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Compared with |
Summary
Fix
llgo testfor packages whose source directories contain assembly files.The synthetic test main package shares the tested package's directory, causing LLGo's assembly scan to find unrelated
.sfiles and rungo listwith a generated package path such asgithub.com/visualfc/gid.test. That path is notdirectly loadable, so the build failed.
Skip assembly discovery for synthetic test main packages and cache the empty result.
Testing
go test ./internal/build -count=1llgo test -vingithub.com/visualfc/gidgit diff --check