Skip to content

Commit

Permalink
Add compatibility shims for Wasmtime 13 CLI (#7385)
Browse files Browse the repository at this point in the history
* Add compatibility shims for Wasmtime 13 CLI

This commit introduces a compatibility shim for the Wasmtime 13 CLI and
prior. The goal of this commit is to address concerns raised in #7336
and other locations as well. While the new CLI cannot be un-shipped at
this point this PR attempts to ameliorate the situation somewhat through
a few avenues:

* A complete copy of the old CLI parser is now included in `wasmtime` by
  default.
* The `WASMTIME_NEW_CLI=0` environment variable can force usage of the
  old CLI parser for the `run` and `compile` commands.
* The `WASMTIME_NEW_CLI=1` environment variable can force usage of the
  new CLI parser.
* Otherwise both the old and the new CLI parser are executed. Depending
  on the result one is selected to be executed, possibly with a warning
  printed.
* If both CLI parsers succeed but produce the same result, then no
  warning is emitted and execution continues as usual.
* If both CLI parsers succeed but produce different results then a
  warning is emitted indicating this. The warning points to #7384 which
  has further examples of how to squash the warning. The warning also
  mentions the above env var to turn the warning off. In this situation
  the old semantics are used at this time instead of the new semantics.
  It's intended that eventually this will change in the future.
* If the new CLI succeeds and the old CLI fails then the new semantics
  are executed without warning.
* If the old CLI succeeds and the new CLI fails then a warning is issued
  and the old semantics are used.
* If both the old and the new CLI fail to parse then the error message
  for the new CLI is emitted.

Note that this doesn't add up to a perfect transition. The hope is that
most of the major cases of change at the very least have something
printed. My plan is to land this on `main` and then backport it to the
14.0.0 branch as a 14.0.3 release.

* Wordsmith messages

* Update messages

* More wording updates

* Fix grammar

* More updates
  • Loading branch information
alexcrichton authored Oct 28, 2023
1 parent ecd3747 commit 519808f
Show file tree
Hide file tree
Showing 18 changed files with 1,421 additions and 38 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ serde_json = { workspace = true }
wasmparser = { workspace = true }
tracing = { workspace = true }
log = { workspace = true }
humantime = { workspace = true }

async-trait = { workspace = true }
bytes = { workspace = true }
Expand Down Expand Up @@ -266,6 +267,7 @@ syn = "2.0.25"
test-log = { version = "0.2", default-features = false, features = ["trace"] }
tracing-subscriber = { version = "0.3.1", default-features = false, features = ['fmt', 'env-filter', 'ansi', 'tracing-log'] }
url = "2.3.1"
humantime = "2.0.0"

# =============================================================================
#
Expand Down Expand Up @@ -309,6 +311,10 @@ default = [
# cost, so allow disabling this through disabling of our own `default`
# feature.
"clap/default",

# By default include compatibility with the "old" CLI from Wasmtime 13 and
# prior.
"old-cli",
]

# ========================================
Expand Down Expand Up @@ -348,6 +354,9 @@ coredump = ["wasmtime-cli-flags/coredump"]
addr2line = ["wasmtime/addr2line"]
debug-builtins = ["wasmtime/debug-builtins"]

# Enables compatibility shims with Wasmtime 13 and prior's CLI.
old-cli = []

# CLI subcommands for the `wasmtime` executable. See `wasmtime $cmd --help`
# for more information on each subcommand.
serve = ["wasi-http", "component-model", "dep:http-body-util"]
Expand Down
2 changes: 1 addition & 1 deletion crates/cli-flags/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ file-per-thread-logger = { workspace = true, optional = true }
tracing-subscriber = { workspace = true, optional = true }
rayon = { version = "1.5.0", optional = true }
wasmtime = { workspace = true }
humantime = "2.0.0"
humantime = { workspace = true }

[features]
pooling-allocator = []
Expand Down
37 changes: 35 additions & 2 deletions crates/cli-flags/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ fn init_file_per_thread_logger(prefix: &'static str) {
}

wasmtime_option_group! {
#[derive(PartialEq, Clone)]
pub struct OptimizeOptions {
/// Optimization level of generated code (0-2, s; default: 0)
pub opt_level: Option<wasmtime::OptLevel>,
Expand Down Expand Up @@ -74,6 +75,7 @@ wasmtime_option_group! {
}

wasmtime_option_group! {
#[derive(PartialEq, Clone)]
pub struct CodegenOptions {
/// Either `cranelift` or `winch`.
///
Expand Down Expand Up @@ -103,6 +105,7 @@ wasmtime_option_group! {
}

wasmtime_option_group! {
#[derive(PartialEq, Clone)]
pub struct DebugOptions {
/// Enable generation of DWARF debug information in compiled code.
pub debug_info: Option<bool>,
Expand All @@ -122,6 +125,7 @@ wasmtime_option_group! {
}

wasmtime_option_group! {
#[derive(PartialEq, Clone)]
pub struct WasmOptions {
/// Enable canonicalization of all NaN values.
pub nan_canonicalization: Option<bool>,
Expand Down Expand Up @@ -212,6 +216,7 @@ wasmtime_option_group! {
}

wasmtime_option_group! {
#[derive(PartialEq, Clone)]
pub struct WasiOptions {
/// Enable support for WASI common APIs
pub common: Option<bool>,
Expand Down Expand Up @@ -256,14 +261,14 @@ wasmtime_option_group! {
}
}

#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq)]
pub struct WasiNnGraph {
pub format: String,
pub dir: String,
}

/// Common options for commands that translate WebAssembly modules
#[derive(Parser)]
#[derive(Parser, Clone)]
pub struct CommonOptions {
// These options groups are used to parse `-O` and such options but aren't
// the raw form consumed by the CLI. Instead they're pushed into the `pub`
Expand Down Expand Up @@ -562,3 +567,31 @@ impl CommonOptions {
Ok(())
}
}

impl PartialEq for CommonOptions {
fn eq(&self, other: &CommonOptions) -> bool {
let mut me = self.clone();
me.configure();
let mut other = other.clone();
other.configure();
let CommonOptions {
opts_raw: _,
codegen_raw: _,
debug_raw: _,
wasm_raw: _,
wasi_raw: _,
configured: _,

opts,
codegen,
debug,
wasm,
wasi,
} = me;
opts == other.opts
&& codegen == other.codegen
&& debug == other.debug
&& wasm == other.wasm
&& wasi == other.wasi
}
}
11 changes: 5 additions & 6 deletions crates/cli-flags/src/opt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use std::time::Duration;
#[macro_export]
macro_rules! wasmtime_option_group {
(
$(#[$attr:meta])*
pub struct $opts:ident {
$(
$(#[doc = $doc:tt])*
Expand All @@ -32,6 +33,7 @@ macro_rules! wasmtime_option_group {
}
) => {
#[derive(Default, Debug)]
$(#[$attr])*
pub struct $opts {
$(
pub $opt: $container<$payload>,
Expand All @@ -41,7 +43,7 @@ macro_rules! wasmtime_option_group {
)?
}

#[derive(Clone, Debug)]
#[derive(Clone, Debug,PartialEq)]
#[allow(non_camel_case_types)]
enum $option {
$(
Expand Down Expand Up @@ -106,7 +108,7 @@ macro_rules! wasmtime_option_group {
}

/// Parser registered with clap which handles parsing the `...` in `-O ...`.
#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq)]
pub struct CommaSeparated<T>(pub Vec<T>);

impl<T> ValueParserFactory for CommaSeparated<T>
Expand Down Expand Up @@ -366,10 +368,7 @@ impl WasmtimeOptionValue for wasmtime::Strategy {
match String::parse(val)?.as_str() {
"cranelift" => Ok(wasmtime::Strategy::Cranelift),
"winch" => Ok(wasmtime::Strategy::Winch),
other => bail!(
"unknown optimization level `{}`, only 0,1,2,s accepted",
other
),
other => bail!("unknown compiler `{other}` only `cranelift` and `winch` accepted",),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmtime/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1866,7 +1866,7 @@ impl fmt::Debug for Config {
///
/// This is used as an argument to the [`Config::strategy`] method.
#[non_exhaustive]
#[derive(Clone, Debug, Copy)]
#[derive(PartialEq, Eq, Clone, Debug, Copy)]
pub enum Strategy {
/// An indicator that the compilation strategy should be automatically
/// selected.
Expand Down
Loading

0 comments on commit 519808f

Please sign in to comment.