fix: read the real Burrito module and the running Elixir for CI pins - #112
Merged
Conversation
Two toolchain pins in the generated workflow could be wrong at once. zig_version never came from Burrito. `Tinfoil.Config` aliases `Tinfoil.Burrito`, so the bare `Burrito` in `infer_zig_version/0` resolved to that module, which loads fine and does not export `get_versions/0`. The guard was always false, so the fallback fired every time. Qualifying it as `Elixir.Burrito` fixes it; the alias cannot capture that form. The other two `Burrito` references in the module correctly mean `Tinfoil.Burrito` and are unchanged. This is why the 0.16.0 fallback bump mattered: with detection broken, the fallback was the only value anyone ever got. Projects on Burrito 1.5 would now have been pinned to Zig 0.16.0 against a 0.15.2 wrapper. Detection resolves both correctly. elixir_version came from the project's `:elixir` requirement floor while otp_version came from `System.otp_release/0`. Each is defensible alone; together they produced pairs that exist nowhere, such as Elixir 1.17 with OTP 29, which setup-beam cannot resolve. A requirement like `~> 1.17` states which Elixir versions may consume the library, not what to build the release with. Both values now come from the running system, so they are self-consistent by construction. An explicit `ci: [elixir_version: ...]` still wins. `mix tinfoil.plan` gained a toolchain row showing the resolved elixir/OTP/zig. These are the values most likely to be silently wrong and were previously visible only by reading the generated YAML. The zig regression test asserts against a deliberately implausible 9.9.9 rather than a real version. Pinning the expectation to a plausible value is how this survived: the fallback and the correct answer were the same string. Closes #105 Closes #106
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.
Closes #105. Closes #106.
Two of the three toolchain pins in a freshly generated workflow could be wrong at once, which is what happened in joshrotenberg/hexpm-mcp#68.
#105: zig_version never came from Burrito
Tinfoil.Configdoesalias Tinfoil.{Burrito, Target}, so the bareBurritoininfer_zig_version/0resolved toTinfoil.Burrito, not the dependency. That module loads fine and does not exportget_versions/0, so the guard was always false and the fallback fired every time.Reproduced in tinfoil's own tree with a stand-in module declaring
9.9.9:Fixed by qualifying as
Elixir.Burrito, which an alias cannot capture. Per the issue's request I audited the otherBurritoreferences inconfig.ex: lines 102-103 callextract_targets/2andresolve_all/3, which genuinely areTinfoil.Burritoand are correct as-is.config.exis the only module with that alias.This makes the recent 0.16.0 fallback bump load-bearing rather than cosmetic. With detection broken the fallback was the only value anyone ever got, so projects still on Burrito 1.5 would have been pinned to Zig 0.16.0 against a 0.15.2 wrapper. With detection working, both resolve correctly.
#106: elixir and otp could form an impossible pair
elixir_versioncame from the project's:elixirrequirement floor;otp_versionfromSystem.otp_release/0on the generating machine. Defensible alone, but together they produced pairs that existed nowhere:Elixir 1.17 supports OTP 25-27, so setup-beam cannot resolve this.
As the issue argues, the floor is the wrong reading in the first place:
~> 1.17states which Elixir versions may consume the library, which for a library routinely sits several minor versions below what you build releases with. Both values now come from the running system, so the pair is self-consistent by construction. An explicitci: [elixir_version: ...]still wins over everything.The compatibility-table validation (suggestion 2) is not included; it is a separate call and needs ongoing maintenance.
Before / after
Same project (
elixir: "~> 1.17"), same machine (Elixir 1.20.2 / OTP 29), Burrito wanting Zig 0.16.0:1.171.2029290.15.20.16.0Before, none of the three described anything real. After:
{"1.20", "29", "0.16.0"}.Plan visibility (#106 suggestion 3)
mix tinfoil.plannow prints the resolved pins, which is what would have surfaced #105 in the first place:Tests
228 pass, up from 224.
New
test/tinfoil/config_zig_inference_test.exsdefines and purges a top-levelBurritostub (non-async; ExUnit finishes all async modules before any sync one, so nothing can observe it). It asserts against a deliberately implausible9.9.9rather than a real version — pinning the expectation to a plausible value is exactly how this bug survived, since the fallback and the correct answer were the same string.I verified the test is load-bearing by reintroducing the bug:
The three config tests that encoded the requirement-floor behavior were rewritten to assert the requirement is ignored and that elixir/OTP both track the running system.
mix format --check-formatted,mix compile --warnings-as-errors, andmix credo --strict --ignore Consistency(63 of 68 checks) all clean.