Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give a bit more informative error message related to deleting isle_generated_code/ #9588

Closed
larry0x opened this issue Nov 9, 2024 · 3 comments

Comments

@larry0x
Copy link

larry0x commented Nov 9, 2024

PR #4143 added this error if a isle_generated_code/ directory exists and an "isle-in-source-tree" feature isn't enabled:

#[cfg(not(feature = "isle-in-source-tree"))]
{
    if explicit_isle_dir.is_dir() {
        eprintln!(concat!(
            "Error: directory isle_generated_code/ exists but is only used when\n",
            "`--feature isle-in-source-tree` is specified. To prevent confusion,\n",
            "this build script requires the directory to be removed when reverting\n",
            "to the usual generated code in target/. Please delete the directory and\n",
            "re-run this build.\n",
        ));
        std::process::exit(1);
    }
}

It says to delete the directory. However, for users who are not familiar with the inner workings of cranelift, it's not clear where this directory is. It took me a long while to find it:

~/.cargo/registry/src/index.crates.io-{hash}/cranelift-codegen-{version}/isle_generated_code

It may be helpful to add the explicit_isle_dir value to the error message so that users are informed exactly which directory they are supposed to delete.

@cfallin
Copy link
Member

cfallin commented Nov 9, 2024

Hmm, something is going wrong here -- you shouldn't ever need to delete a directory in ~/.cargo. The intent of this feature was to allow Cranelift developers to see a version of the code in-tree when developing Cranelift, and should not be used when Cranelift is pulled in as a dependency from crates.io. Could you say more about your environment and how you're using Cranelift?

@larry0x
Copy link
Author

larry0x commented Nov 9, 2024

@cfallin I don't use cranelift directly so not sure what's supposed to happen... I use wasmer. At wasmer 5.0.0 my program compiles with no problem. Once bumped to wasmer 5.0.1, compiling my program results in this error:

error: failed to run custom build command for `cranelift-codegen v0.110.2`

Caused by:
  process didn't exit successfully: `/Users/[...]/target/release/build/cranelift-codegen-725296997bf7694e/build-script-build` (exit status: 1)
  --- stdout
  cargo:rerun-if-changed=build.rs

  --- stderr
  Error: directory isle_generated_code/ exists but is only used when
  `--feature isle-in-source-tree` is specified. To prevent confusion,
  this build script requires the directory to be removed when reverting
  to the usual generated code in target/. Please delete the directory and
  re-run this build.

I followed the instruction and deleted:

~/.cargo/registry/src/index.crates.io-{hash}/cranelift-codegen-{version}/isle_generated_code

Now the program compiles just fine.

@cfallin
Copy link
Member

cfallin commented Nov 10, 2024

@larry0x OK, thanks for the additional info. This error does not happen when using Cranelift by itself, or with Wasmtime (in this repo). We do not provide support for Wasmer -- that is a different project. I would recommend asking them for help if the issue occurs again; they may be setting options in some way that is causing this issue.

@cfallin cfallin closed this as completed Nov 10, 2024
cfallin added a commit to cfallin/wasmtime that referenced this issue Nov 20, 2024
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.
github-merge-queue bot pushed a commit that referenced this issue Nov 20, 2024
Per #9625 and #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 #9625.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants