fix(ci): replace yq TOML parsing with bash grep/sed#1325
Conversation
yq cannot parse TOML files with comments, breaking the "Find environments" CI job. Replace the inline yq-based logic with a standalone bash script that uses grep/sed for TOML extraction. - Add scripts/discover-envs.sh with three modes: discover (CI JSON), --list (table), --validate - Add Justfile for local developer convenience - Add just to flake.nix devShell - CI calls bash script directly (no extra tooling)
github-actions
bot
added
the
team-content
scripts/discover-envs.sh
Outdated
| # Check if manifest has real (uncommented) service commands. | ||
| has_services() { | ||
| local manifest="$1" | ||
| # Match lines like: name.command = "..." that are NOT comments | ||
| grep -qE '^[^#]*\.command[[:space:]]*=' "$manifest" | ||
| } | ||
|
|
||
| # Extract systems list from [options] section. | ||
| # Handles both single-line and multi-line arrays. | ||
| # Returns empty string if no systems found. | ||
| get_systems() { | ||
| local manifest="$1" | ||
| local raw | ||
| raw="$(sed -n '/^\[options\]/,/^\[/p' "$manifest")" || true | ||
| if [ -z "$raw" ]; then | ||
| return 0 | ||
| fi | ||
| echo "$raw" \ | ||
| | sed -n '/^systems[[:space:]]*=/,/\]/p' \ | ||
| | { grep -oE '"[^"]+"' || true; } \ | ||
| | tr -d '"' | ||
| } |
There was a problem hiding this comment.
👍🏻 for restructuring the inline script so that it's easier to read
- Replace inline yq-based TOML parsing with
scripts/discover-envs.sh(grep/sed)- yq cannot parse TOML files with comments, breaking the "Find environments" CI job
I don't think we should replace yq with grep/sed for manifest heuristics though because it will be even more brittle to changes, particularly false-negatives that we won't notice.
In the short term, if you rebase #1323 and #1324 against main to pick up the change from #1317 then the jobs should pass again. Or just a re-run might be sufficient since it looks like the updated version of yq landed in a runner image literally minutes ago: actions/runner-images@04114b0#diff-cc12e5c5d005932feaacf280af949d83e2cb6020dff8203d9ead2e00c30e8b7fL99-R99
Longer term, it would be good to wrap those dependencies in a Flox environment so that we have more control over when they change. We might also have to consider schema versions in the future now that the manifest shape may change more freely and be auto-upgraded between versions.
There was a problem hiding this comment.
Good call — switched to parsing manifest.lock (JSON) with jq instead. No more TOML heuristics at all. The lock file has the full parsed manifest under .manifest, so services and systems are just straightforward jq queries. Pushed in 25627ce.
There was a problem hiding this comment.
SGTM. We also benefit from getting the composed manifest and avoiding any stale changes on disk 👍🏻
Per Dan's review: grep/sed on TOML is even more brittle than yq. The lock file is JSON with the full parsed manifest, so jq handles everything cleanly -- services, systems, comments are all non-issues. Addresses: floxenvs#1325 (review feedback)
dcarley
left a comment
dcarley
left a comment
There was a problem hiding this comment.
Eyeballed the script changes a second time; look right and appears to produce the same list of events. Failing test is tailscale related.
2 participants
Summary
scripts/discover-envs.sh(grep/sed)Justfilefor local convenience,justadded to devShellTest plan
bash scripts/discover-envs.sh --validatepasses all manifests (including comment-heavy ones like postgres, rust, fooocus)UPDATE_ALL=true bash scripts/discover-envs.shproduces valid JSON (verified with jq)