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.

Sign up for GitHub

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

Jump to bottom

Provide a mechanism for disabling on-by-default config options regardless of the enabled cargo feature flags #10454

Open
Stebalien opened this issue Mar 21, 2025 · 6 comments · May be fixed by #10455
Open

Provide a mechanism for disabling on-by-default config options regardless of the enabled cargo feature flags #10454

Stebalien opened this issue Mar 21, 2025 · 6 comments · May be fixed by #10455

Comments

@Stebalien
Copy link
Contributor

Feature

Provide some mechanism for disabling on-by-default config options without having to enable their associated cargo feature flags.

Benefit

The wasmtime Config has a bunch of options like Config::wasm_reference_types that are enabled by-default if the corresponding cargo feature is enabled. Unfortunately, these config options are gated by the same feature flags making it impossible to turn them off without enabling the underlying cargo feature.

This is fine when building an application, but is a problem for library authors. If a library author wants to configure wasmtime in a specific way, they need to enable features (in cargo) they don't actually want so they can disable them in the config. Unfortunately, there's no way to actually detect if the feature has been enabled.

Implementation

The simplest solution is to provide some form of Config::minimal() constructor (or something like that) that turns off all optional features. New versions of wasmtime may make some of these features mandatory, but, IMO, that's fine. I'm mostly looking for predictability.

Alternatives

Another option is to let users call all config methods even if the associated capabilities are disabled via cargo features:

  1. Remove the #[cfg(feature = ...)] gates on all Config methods and their associated fields in the config.
  2. Return an error from Engine::new if unsupported features are requested.

The main drawback is that we'll lose the compile-time check on whether or not the feature is enabled.
@Stebalien Stebalien mentioned this issue Mar 21, 2025
Draft
@fitzgen
Copy link
Member

fitzgen commented Mar 21, 2025

Your motivation definitely makes sense, however I think the alternative you describe is probably the best approach here, mainly because

  1. I think that a Config::minimal() is a little too ambiguous and some people might think it means "good config for embedded use with minimal platform assumptions" while other might think it means "good config for my target platform with minimal wasm features" and I don't think it is worth trying to word-smith our way out of that kind of confusion or whatever and that trying to make everyone happy will make no one happy.

  2. We already do some config validation during engine creation and return errors if incompatible config options are set and things like that.

@alexcrichton
Copy link
Member

Agreed yeah, I think it'd be reasonable to un-gate most of the Config methods and have them always available

@Stebalien
Copy link
Contributor Author

I'm quite happy with either option and I'm happy to take a stab at implementing this.

@fitzgen
Copy link
Member

fitzgen commented Mar 21, 2025

I'm quite happy with either option and I'm happy to take a stab at implementing this.

Great! Let me know (here or zulip) if you need any pointers or anything like that 👍

@bjorn3
Copy link
Contributor

bjorn3 commented Mar 21, 2025

Would it make sense to have a method which returns a Config containing exactly the features in the lime1 target (as defined in the tool convensions repo) without any other features enabled?

@Stebalien Stebalien linked a pull request Mar 21, 2025 that will close this issue
Open
@Stebalien
Copy link
Contributor Author

I have a PR here (#10455) but I need a small clarification on the default status of reference types with the "gc" feature disabled.

Stebalien added a commit to Stebalien/wasmtime that referenced this issue Mar 21, 2025
@Stebalien
Keep Config options for disabling compiled-out features
88bab50 
This patch keeps config options for disabling default-enabled wasmtime
features, even when those features are disabled via crate feature flags.
That way, these wasm features can be disabled by libraries even if the
crate-level features are later enabled by some other crate in the
build-tree.

Specifically, the following methods are available on the
`wasmtime::Config` type, regardless of the enabled crate features:

1. `Config::wasm_threads`.
2. `Config::wasm_reference_types`.
3. `Config::wasm_component_model`.
4. `Config::parallel_compilation`.

Of these, enabling any of the first three while the corresponding crate
feature is disabled will lead to a runtime error when constructing the
`wasmtime::Engine`. Enabling the last one, parallel compilation, will
simply do nothing.

fixes bytecodealliance#10454
Stebalien added a commit to Stebalien/wasmtime that referenced this issue Mar 21, 2025
@Stebalien
Keep Config options for disabling compiled-out features
ef13941 
This patch keeps config options for disabling default-enabled wasmtime
features, even when those features are disabled via crate feature flags.
That way, these wasm features can be disabled by libraries even if the
crate-level features are later enabled by some other crate in the
build-tree.

Specifically, the following `wasmtime::Config` methods are now enabled
regardless of compile-time features (removed dependency on the GC feature):

1. `Config::wasm_reference_types`.
2. `Config::wasm_function_references`.

The following methods are available on the `wasmtime::Config` type,
regardless of the enabled crate features but will lead to runtime errors
when constructing a `wasmtime::Engine`:

1. `Config::wasm_threads`.
2. `Config::wasm_gc`.
3. `Config::wasm_component_model`.
4. `Config::wasm_component_model_async`.

Finally, `Config::parallel_compilation` is defined regardless of whether
or not the parallel compilation feature is enabled. However, without the
corresponding crate feature, this config option is a no-op.

fixes bytecodealliance#10454
Stebalien added a commit to Stebalien/wasmtime that referenced this issue Mar 21, 2025
@Stebalien
Keep Config options for disabling compiled-out features
655afbd 
This patch keeps config options for disabling default-enabled wasmtime
features, even when those features are disabled via crate feature flags.
That way, these wasm features can be disabled by libraries even if the
crate-level features are later enabled by some other crate in the
build-tree.

Specifically, the following `wasmtime::Config` methods are now enabled
regardless of compile-time features (removed dependency on the GC feature):

1. `Config::wasm_reference_types`.
2. `Config::wasm_function_references`.

The following methods are available on the `wasmtime::Config` type,
regardless of the enabled crate features but will lead to runtime errors
when constructing a `wasmtime::Engine`:

1. `Config::wasm_threads`.
2. `Config::wasm_gc`.
3. `Config::wasm_component_model`.
4. `Config::wasm_component_model_async`.

Finally, `Config::parallel_compilation` is defined regardless of whether
or not the parallel compilation feature is enabled. However, without the
corresponding crate feature, this config option is a no-op.

fixes bytecodealliance#10454
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Keep Config options for disabling compiled-out features Stebalien/wasmtime
4 participants
@alexcrichton @fitzgen @Stebalien @bjorn3