Repository navigation
fix(bin): cut lint partition wall time with per-root source views and up to four workers - #6
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
fix the lint first. Fix the lint before merging the Telegram formatting change. CI's 'Lint 1 (partition 1of2)' is killed by SIGTERM after about eight minutes with no diagnostics, so no pull request in this repository can reach green checks. It is pre-existing on main and not caused by the Telegram change (reproduced on the primary checkout with no branch changes). Cause, measured: same file, same machine, shellcheck 0.11.0, one flag apart: 'shellcheck --norc --external-sources -- bin/fm-afk-launch.sh' takes 13.5s real versus 0.57s without --external-sources, a 24x difference in user CPU on a single file. Partition 1of2 holds roughly forty shell files, about nine minutes, matching the eight-minute kill. So --external-sources follows every source directive and re-parses this repository's shared bin/ library graph once per dependent script. A finished reviewed change is parked at its CI gate waiting on this, and all planned pull request work is behind it. Make the lint fast enough to finish comfortably inside its budget WITHOUT losing what it currently checks: removing --external-sources or otherwise stopping shellcheck following sourced files is not the fix, because that silently drops cross-file coverage. bin/fm-lint.sh is the single owner of the lint definition used by both CI and the no-mistakes pre-push gate, so the fix belongs there. CI and the pre-push gate must keep behaving identically, partitioning must still shard completely with no file unchecked, the refusal to run under an unexpected linter version must survive, bin/*.sh must stay shellcheck-clean, and tests must pin behaviour rather than wall-clock. Out of scope: the Telegram changes, rebalancing partitions purely to hide the cost, and raising the CI timeout.
What Changed
bin/fm-lint.shnow runsshellcheck --external-sourceson each root separately, inside a private view of the repository. In that view each shared library is included by directive once, at its first top-level site. Later top-level sites in non-test files becomesource=/dev/null. Indented sites and test files keep every site, because their scoping affects findings. If the view build fails, that root falls back to the real tree.FM_LINT_DEDUPE_SOURCES=0turns the view off. The flags, the pinned-version refusal and the partition inventory are not changed.--jobsaccepts 1-4, and telemetry adds shard 3 and 4 weights. Diagnostics still replay in stable shard/root order.tests/fm-lint.test.shis updated for the new shard and worker behaviour, anddocs/fm-test-portable-shards.mdis updated to describe the per-root views and up to four workers.Risk Assessment
✅ Low: The fix stays inside bin/fm-lint.sh, keeps --external-sources and the version pin, and the prior scope finding is resolved: only column-0 include sites are deduped or counted as a first site, and tests/ keeps every site. I found no wrong-result path, but I did not run the lint or the tests, so the speedup and the diagnostic parity on the real bin/ tree are unmeasured.
Testing
I ran both canonical CI partitions live with the real ShellCheck 0.11.0 and actionlint. Both exited 0 with no diagnostics, the version pin was reported, and workflow lint passed. Partition 1of2 took 326s when run alone. The lint owner's test file passed. The dedupe-scope behavior was checked by reading code only, with no dedicated live parity run against the base. I did not time the base commit, so the speedup is not measured directly.
bash tests/fm-lint.test.shoutput all ok; partition output shows 'ShellCheck 0.11.0 (pinned 0.11.0)'Evidence: partition 1of2 output
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
bin/fm-lint.sh:109- Source dedupe view (FM_LINT_VIEW_PERL) rewrites every later include site of a library in non-test files to source=/dev/null, regardless of scope. The comment claims findings are identical, but this is only true if the first site is at an equivalent scope. If the first site is inside a subshell, function, or conditional and a later top-level site is dropped, scope-dependent diagnostics (SC2031, SC2154, SC2218 and similar) can be missed or spuriously produced. Only tests/ is exempted. The added test covers only two top-level includes. Fix the smallest honest way: dedupe only when the first site is a plain top-level include, or verify parity across the real bin/ tree in the test.🔧 Fix applied.
✅ Re-checked - no issues remain.
bash tests/fm-lint.test.shoutput all ok; partition output shows 'ShellCheck 0.11.0 (pinned 0.11.0)'CI=true bin/fm-lint.sh --partition 1of2(rc=0, 326s wall alone, 12 CPUs)CI=true bin/fm-lint.sh --partition 2of2(rc=0)bash tests/fm-lint.test.sh(all output lines ok)Read the view-builder code in bin/fm-lint.sh to confirm only top-level# shellcheck source=sites are deduped and tests/ files keep every site (code reading only)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.