Exclude unstable compatibility packages - #33
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review summary
Scoped, low-risk PR that stabilizes the LLGo-compatibility CI run. I verified the structural invariants and ran the manifest test locally — all green:
- Counts consistent:
benchmarks-llgo-compatibility.tomlhas exactly 200[[Benchmarks]]entries with uniqueName/Repovalues, matchingsum(Limit) = 200inprojects.toml. Per-project counts line up (etcd 15+8+3 = 26, protobuf 35, gonum 45, client_go 45, zap 11, gorm 5, workiva 28, plus 5 singletons). - Renumbering clean:
Namesequences are contiguous within every prefix — no gaps or duplicates after the removed/backfilled entries. - Exclude lists disjoint from included repos; I checked the near-miss paths (
go.uber.org/zapvszap/*,gorm.io/gorm/logger,.../auth/execvs.../tools/auth/exec). test_manifest.pypasses with the updated etcd assertion (26).- Go change is correct: the new
BENT_QUIET_RUN_OUTPUTbranch inconfiguration.goreuses the same env idiom already atconfiguration.go:356, sits correctly between theverbose > 0and default-dot branches, and writes to the driver log (notbench/*.stdout), soreport.pyparsing is unaffected. -S=0disables build shuffling (default2), giving deterministic compile ordering that pairs well with the newcompiling: <name> (<config>)progress line.- No security or performance concerns (the per-compile
os.Getenvis negligible next to thego test -csubprocess it precedes).
No blocking issues. A few optional notes below.
Optional, non-blocking:
benchmarks-llgo-compatibility.tomlis a generatedDO NOT EDITfile (ci/llgo-compatibility/generate_manifest.py). Please confirm these entries came from re-running the generator against the updatedprojects.tomlrather than hand-editing — otherwise a future generator run could clobber them and reintroduce the excluded packages.cmd/bent/README.md:56documents the shuffle flag as lowercase-s, but bent registers it as-S(bent.go:211). This PR is the first to use-Sin CI; a one-char README fix would keep them in sync. (Pre-existing, not introduced here.)- The etcd
26intest_manifest.py:23is a second magic number that must track the three etcdLimits inprojects.toml. Deriving it from the quotas would prevent drift. (Optional maintainability.)
| if verbose > 0 { | ||
| fmt.Println(asCommandLine(cwd, cmd)) | ||
| } else if os.Getenv("BENT_QUIET_RUN_OUTPUT") != "" { | ||
| fmt.Printf("\ncompiling: %s (%s)\n", bench.Name, config.Name) |
There was a problem hiding this comment.
The new quiet-mode compile line is correct and consistent with the existing BENT_QUIET_RUN_OUTPUT handling at configuration.go:356. Non-blocking: run.sh's comment describes BENT_QUIET_RUN_OUTPUT purely in terms of run/test output ("echoing them all into the Actions log can starve a hosted runner"). Now that the same variable also toggles per-compile progress here, consider updating that comment so its documented scope matches the new compile-output behavior.
| for config in Go LLGo; do | ||
| arguments=( | ||
| -T -N=1 | ||
| -T -N=1 -S=0 |
There was a problem hiding this comment.
-S=0 disables build shuffling (default is 2), producing deterministic compile ordering — a good match for the new compiling: <name> (<config>) line so the CI log reads as a clean, ordered stream. Looks correct.
1 participant
The first full compatibility run exposed packages that either failed/timed out under the Go baseline or failed LLGo dependency discovery before the hosted runner was terminated.
This change:
Validation:
go test ./cmd/bentpython3 -m unittest discover -s ci/llgo-compatibility -p "test_*.py"bash -n ci/llgo-compatibility/run.shhumanize_001Go/LLGo smoke: 28/28 tests passed; raw test2json outputs and results.json retained