Cranelift: rework isle-in-source-tree functionality. #9633
Merged
Conversation
This file contains 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
Per bytecodealliance#9625 and bytecodealliance#9588, a downstream user of Cranelift was misusing the `isle-in-source-tree` feature, enabling it indiscriminately even in published crates (wasmerio/wasmer#5202). This went against the intent of the feature, to be a way for local developers to observe the generated source. As such, it ran afoul of some safety checks for that purpose, and also polluted users' Cargo caches in a way that we cannot now fix except in future releases. The best we can do is probably to take away features that are liable to be misused. Following discussion in today's Cranelift weekly meeting, we decided to provide this functionality under an environment variable instead. This allows folks who know what they're doing to get the same behavior, but does not expose a feature to crate users. Fixes bytecodealliance#9625.
alexcrichton
approved these changes
Nov 20, 2024
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
I think the merge queue run raced with the release; cargo-vet failed because it found a version on crates.io it didn't expect. Re-running, but noting in case we see it again later -- interesting corner case! |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
Ah I don't think this is spurious but should be fixed at #9635 |
bjorn3
reviewed
Mar 25, 2025
| std::process::exit(1); | ||
| } | ||
| } | ||
| let isle_dir = if let Ok(path) = std::env::var("ISLE_SOURCE_DIR") { |
There was a problem hiding this comment.
Shouldn't this use cargo::rerun-if-env-changed?
There was a problem hiding this comment.
Ah, yes, I suppose so -- thanks for noticing this! I'll post a PR.
cfallin
added a commit
to cfallin/wasmtime
that referenced
this pull request
Mar 25, 2025
- Add a `cargo:rerun-if-env-changed` clause for the `ISLE_SOURCE_DIR` environment variable to properly rebuild if the config changes. - Update docs to point to the new env-var-based approach rather than the old Cargo feature. As per this Zulip conversation [1] and this GitHub comment [2]. [1]: https://bytecodealliance.zulipchat.com/#narrow/channel/217117-cranelift/topic/How.20to.20enable.20the.20isle-in-source-tree.20feature.3F/near/508028830 [2]: bytecodealliance#9633 (comment)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 25, 2025
) - Add a `cargo:rerun-if-env-changed` clause for the `ISLE_SOURCE_DIR` environment variable to properly rebuild if the config changes. - Update docs to point to the new env-var-based approach rather than the old Cargo feature. As per this Zulip conversation [1] and this GitHub comment [2]. [1]: https://bytecodealliance.zulipchat.com/#narrow/channel/217117-cranelift/topic/How.20to.20enable.20the.20isle-in-source-tree.20feature.3F/near/508028830 [2]: #9633 (comment)
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.
Per #9625 and #9588, a downstream user of Cranelift was misusing the
isle-in-source-treefeature, enabling it indiscriminately even in published crates (wasmerio/wasmer#5202). This went against the intent of the feature, to be a way for local developers to observe the generated source. As such, it ran afoul of some safety checks for that purpose, and also polluted users' Cargo caches in a way that we cannot now fix except in future releases.The best we can do is probably to take away features that are liable to be misused. Following discussion in today's Cranelift weekly meeting, we decided to provide this functionality under an environment variable instead. This allows folks who know what they're doing to get the same behavior, but does not expose a feature to crate users.
Fixes #9625.