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

Unable to disable default features in Wasmtime dependencies #189

Closed
mbestavros opened this issue Jul 11, 2019 · 8 comments
Closed

Unable to disable default features in Wasmtime dependencies #189

mbestavros opened this issue Jul 11, 2019 · 8 comments

Comments

@mbestavros
Copy link
Contributor

mbestavros commented Jul 11, 2019

I am interested in building Wasmtime against a Cranelift with all architectures disabled except for the target architecture.

From my research, it seems that the way to accomplish this would be to re-export the desired architecture feature in Wasmtime's cargo.toml:

riscv = ["cranelift-codegen/riscv"]

(For reference, the feature belongs to cranelift-codegen, whose features can be found in its cargo.toml file.)

However, cranelift-codegen's default configuration enables all architectures to begin with, so in order to re-apply only the desired one, I would need to disable the default features in Cranelift and then enable only the desired one by re-exporting the feature, as outlined above.

The Rust-official method to accomplish this is to use the --no-default-features flag, as outlined here. However, as I discovered today, the Wasmtime repository is set up as a workspace (rather than a package), and the flag is non-functional on workspaces -- it only works on packages.

cranelift-codegen is not a workspace by itself, so I also tried using the inline default-features = false option (example of that here); however, that didn't seem to work either.

Given this seems to be an upstream issue, I was wondering if there would be any guidance around this at all.

@alexcrichton
Copy link
Member

One important thing to note as well here is that Cargo unions dependencies for crates, and by default all dependency edges enable the default feature. That means to disable a default feature then every single crate depending on another must disable the default feature. I think that may be why inline default-features = false didn't work perhaps?

FWIW this practically means that heavily depended on crates (those with a good number of incoming dependency edges) are very difficult to disable default features. If that's the case it may best to not actually enable any architectures by default in cranelift-codegen and instead allow top-level projects to specify. (and perhaps having a feature for all-architectures or something like that).

I think overall there's two possible solutions here:

  • Leave all architectures enabled by default, but all crates depending on cranelift-codegen (and transitively) need to disable default features and then reexport the feature necessary to enable each architecture.
  • Instead remove the default feature from cranelift-codegen and replace it with a non-default "enable everything" feature, and then require crates to enable at least one feature to enable a backend.

@npmccallum
Copy link
Member

@alexcrichton There might be a third option. My intuition tells me that it is not a good one, but I can't put my finger on why. Maybe you could clarify this.

We could write a build.rs which detects whether or not any architecture features are manually defined. If some are defined, the build.rs would do nothing. If none are defined, it could manually enable default options.

@alexcrichton
Copy link
Member

Ah unfortunately I don't think that would be possible because build.rs would presumably be written in cranelift-codegen but doesn't have access to the Cargo.toml of all files depending on cranelift-codegen. A build.rs file knows that features are activated, but it doesn't know how features are activated.

@mbestavros
Copy link
Contributor Author

mbestavros commented Jul 12, 2019

Option 2 seems like the ideal way forward (from our point of view, at least); it would definitely simplify things for us. I think it also fits better with Rust's idea that "features should be additive".

Plus, I can imagine at least some other projects would be interested in running only on specific architectures.

@chaozju
Copy link

chaozju commented Jun 13, 2020

@mbestavros
Does this mean Wasmtime is available for riscv-cross-compiling? THX

@bjorn3
Copy link
Contributor

bjorn3 commented Jun 13, 2020

While Cranelift has basic risc-v support, it is nowhere near complete enough to be usable with Wasmtime.

dhil added a commit to dhil/wasmtime that referenced this issue Jun 7, 2024
@bjorn3
Copy link
Contributor

bjorn3 commented Jun 22, 2024

This has been fixed by bytecodealliance/cranelift#853, right?

@cfallin
Copy link
Member

cfallin commented Jun 22, 2024

I believe so; and the crate and feature structure has changed so much in the past five years anyway that any current problems can have new issues filed.

@cfallin cfallin closed this as completed Jun 22, 2024
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

6 participants