Make wac plug output deterministic#212
Open
techieyann wants to merge 1 commit into
Open
Conversation
`wac plug` collected its plugs into a `std::collections::HashMap` and then iterated it to register and instantiate them. Rust's `HashMap` has randomised iteration order, so the order in which plug components were emitted into the composed output varied between runs on byte-identical inputs. The outputs were always valid and semantically equivalent -- each plug's payload was embedded verbatim, and only its position in the outer component section (and the corresponding `instantiate` indices) moved -- but they were not byte-identical, which makes the command unusable in content-addressed or reproducible build pipelines. Observed with wac-cli 0.10.1 on linux/amd64 and darwin/arm64: - 2 plugs, 20 runs -> 2 distinct output digests - 4 plugs, 30 runs -> 16 distinct output digests (4! = 24 permutations) Command-line order did not help: running `--plug a --plug b` and `--plug b --plug a` produced the same set of digests in the same proportion, so the argument order was discarded before emission. Use `indexmap::IndexMap` (already a dependency) instead, which preserves insertion order. Plugs are now registered in command-line order, so the output is byte-identical across runs and the argument order is meaningful. With this change, on the same inputs: - 2 plugs, 20 runs -> 1 digest - 4 plugs, 30 runs -> 1 digest - reversing the two `--plug` arguments yields a different, stable digest Fixes bytecodealliance#211 Signed-off-by: Ian McEachern <techieyann@gmail.com>
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.
wac plugcollected its plugs into astd::collections::HashMapand then iterated it to register and instantiate them. Rust'sHashMaphas randomised iteration order, so the order in which plug components were emitted into the composed output varied between runs on byte-identical inputs.The outputs were always valid and semantically equivalent — each plug's payload was embedded verbatim, and only its position in the outer component section (and the corresponding
instantiateindices) moved — but they were not byte-identical, which makes the command unusable in content-addressed or reproducible build pipelines.This switches to
indexmap::IndexMap, already a dependency of the CLI crate, which preserves insertion order. Plugs are now registered in command-line order, so the output is byte-identical across runs and the argument order becomes meaningful.Verification
Same inputs throughout (one 4-dependency and one 2-dependency socket component):
Output passes
wasm-tools validatein all cases.The last row is worth noting: the two digests that previously appeared at random for two plugs turn out to be exactly the two argument permutations. After the change, argument order selects between them deterministically.
Tests
I have not added a regression test — there is no existing harness for the
plugcommand, and a determinism test needs either golden files or a repeated-invocation loop. Happy to add one in whichever style you would prefer.Fixes #211