Repository navigation
fix(extensions): keep build machine paths out of the WASM guests - #327
Merged
Merged
Conversation
The WASM of extension/rust/v0.4.0 built on macOS differed from the release job's build. Exactly two things caused that, which a rebuild of 0716e22 here proved: with the release job's paths and its host line reported to cargo, the bytes equal the published WASM. This fixes the first cause. Guests embedded the Cargo home and, with rust-src installed, the toolchain directory, so a local build recorded the builder's home path. The rust and elm-native artifact tasks now remap both, for the guest build only, and fail if a guest still contains either directory. The second cause stays: cargo hashes the host triple into crate metadata through build scripts and proc macros, so different host triples still give different bytes. Document this, so a local digest is not mistaken for a prediction of the published one. Bead morphir-0xtc.5.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1fea29e62
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
With CARGO_HOME=/cargo the remap changes nothing, so a correctly mapped /cargo/registry path matched the guard and packaging refused a valid guest. Skip the Cargo home check in that case; the toolchain check still runs. A trailing slash on CARGO_HOME had the opposite effect: the remap gave /cargoregistry paths and the guard searched for a double slash that never matches, so it could not fail. Normalize the home before both. Found by the Codex review; the trailing slash was found while checking it.
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.
Summary
Extension WASM guests recorded where they were built. This PR removes those paths. Bead
morphir-0xtc.5.The WASM of
extension/rust/v0.4.0built on macOS differed from the release job's build. A rebuild of0716e22showed exactly why: with the release job's paths remapped and itshost:line reported to cargo, the macOS build is byte-identical to the published WASM (3b3ce74b…). So there are exactly two causes:rust-srcis installed. A local build recorded the builder's home path. This PR fixes this.host:line ofrustc -vVfor build scripts and proc macros, and that hash reaches every crate that depends on them, so mangled symbols change. This PR does not change this.Changes
extension:artifact:rustandextension:artifact:elm-nativebuild the guest with--remap-path-prefix: the Cargo home becomes/cargo, and therust-srcdirectory becomes/rustc/<commit>, the path rustc records when that component is absent.CARGO_ENCODED_RUSTFLAGS(a path with spaces stays whole). With--target, they do not reach build scripts or proc macros, and the native test builds do not see them.docs/extensions/capability-claims.mdgets "Build paths and digests": a local run on another host triple checks the release steps, but does not predict the published digest.The next release of each extension gets a new digest from this change. Published releases do not change.
Not in this PR: the host triple
Making macOS and Linux builds byte-identical needs cargo to see one fixed
host:line. The experiment did that with a wrapper that gives cargo a falserustc -vV, and that needs its own decision before it goes into a release pipeline. The alternative is to run the task in anx86_64-unknown-linux-gnuenvironment.Tests
Written first, in
tests/ci/test_package_extension_task.py: the exact flags, with a Cargo home that has a space and with the default home; no flags on native builds; refusal of a guest that embeds either directory; refusal of an unknown toolchain; and both tasks having the same rules.