From 4b1f4e63c3fb403363e149ab9a1fd6aaa327e7ba Mon Sep 17 00:00:00 2001 From: Piepmatz Date: Wed, 25 Dec 2024 09:50:02 +0100 Subject: [PATCH] Replace `std::time::Instant` with `web_time::Instant` (#14668) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description The `std::time::Instant` type panics in the WASM context. To prevent this, I replaced all uses of `std::time::Instant` in WASM-relevant crates with `web_time::Instant`. This ensures commands using `Instant` work in WASM without issues. For non-WASM targets, `web-time` simply reexports `std::time`, so this change doesn’t affect regular builds ([docs](https://docs.rs/web-time/latest/web_time/)). To ensure future code doesn't reintroduce `std::time::Instant` in WASM contexts, I added a `clippy wasm` command to the toolkit. This runs `cargo clippy` with a `clippy.toml` configured to disallow `std::time::Instant`. Since `web-time` aliases `std::time` by default, the `clippy.toml` is stored in `clippy/wasm` and is only loaded when targeting WASM. I also added a new CI job that tests this too. # User-Facing Changes None. --- .github/workflows/ci.yml | 56 +++++++---- Cargo.lock | 3 + Cargo.toml | 1 + clippy/wasm/clippy.toml | 3 + crates/nu-command/Cargo.toml | 1 + crates/nu-command/src/debug/timeit.rs | 2 +- crates/nu-command/src/viewers/table.rs | 2 +- crates/nu-protocol/Cargo.toml | 1 + crates/nu-protocol/src/debugger/profiler.rs | 2 +- crates/nu-system/Cargo.toml | 1 + crates/nu-system/src/windows.rs | 3 +- toolkit.nu | 101 +++++++++++++------- 12 files changed, 118 insertions(+), 58 deletions(-) create mode 100644 clippy/wasm/clippy.toml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f3daa9a466..16d2976114 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -163,7 +163,23 @@ jobs: echo "no changes in working directory"; fi - build-wasm: + wasm: + env: + WASM_OPTIONS: --no-default-features --target wasm32-unknown-unknown + CLIPPY_CONF_DIR: ${{ github.workspace }}/clippy/wasm/ + + strategy: + matrix: + job: + - name: Build WASM + command: cargo build + args: + - name: Clippy WASM + command: cargo clippy + args: -- $CLIPPY_OPTIONS + + name: ${{ matrix.job.name }} + runs-on: ubuntu-latest steps: - uses: actions/checkout@v4.1.7 @@ -174,22 +190,22 @@ jobs: - name: Add wasm32-unknown-unknown target run: rustup target add wasm32-unknown-unknown - - run: cargo build -p nu-cmd-base --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-cmd-extra --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-cmd-lang --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-color-config --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-command --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-derive-value --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-engine --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-glob --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-json --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-parser --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-path --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-pretty-hex --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-protocol --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-std --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-system --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-table --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-term-grid --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nu-utils --no-default-features --target wasm32-unknown-unknown - - run: cargo build -p nuon --no-default-features --target wasm32-unknown-unknown + - run: ${{ matrix.job.command }} -p nu-cmd-base $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-cmd-extra $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-cmd-lang $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-color-config $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-command $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-derive-value $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-engine $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-glob $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-json $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-parser $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-path $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-pretty-hex $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-protocol $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-std $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-system $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-table $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-term-grid $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nu-utils $WASM_OPTIONS ${{ matrix.job.args }} + - run: ${{ matrix.job.command }} -p nuon $WASM_OPTIONS ${{ matrix.job.args }} diff --git a/Cargo.lock b/Cargo.lock index af3ea85532..3b8058c207 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3420,6 +3420,7 @@ dependencies = [ "uuid", "v_htmlescape", "wax", + "web-time", "which", "windows 0.56.0", "winreg", @@ -3659,6 +3660,7 @@ dependencies = [ "tempfile", "thiserror 2.0.6", "typetag", + "web-time", "windows-sys 0.48.0", ] @@ -3687,6 +3689,7 @@ dependencies = [ "ntapi", "procfs", "sysinfo 0.32.1", + "web-time", "windows 0.56.0", ] diff --git a/Cargo.toml b/Cargo.toml index 36c5da2ffd..0948360b20 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -178,6 +178,7 @@ uucore = "0.0.28" uuid = "1.11.0" v_htmlescape = "0.15.0" wax = "0.6" +web-time = "1.1.0" which = "7.0.0" windows = "0.56" windows-sys = "0.48" diff --git a/clippy/wasm/clippy.toml b/clippy/wasm/clippy.toml new file mode 100644 index 0000000000..d1092aff44 --- /dev/null +++ b/clippy/wasm/clippy.toml @@ -0,0 +1,3 @@ +[[disallowed-types]] +path = "std::time::Instant" +reason = "WASM panics if used, use `web_time::Instant` instead" diff --git a/crates/nu-command/Cargo.toml b/crates/nu-command/Cargo.toml index f19abf35d0..7e8962d195 100644 --- a/crates/nu-command/Cargo.toml +++ b/crates/nu-command/Cargo.toml @@ -106,6 +106,7 @@ wax = { workspace = true } which = { workspace = true, optional = true } unicode-width = { workspace = true } data-encoding = { version = "2.6.0", features = ["alloc"] } +web-time = { workspace = true } [target.'cfg(windows)'.dependencies] winreg = { workspace = true } diff --git a/crates/nu-command/src/debug/timeit.rs b/crates/nu-command/src/debug/timeit.rs index 182cab87d7..6c372db3f6 100644 --- a/crates/nu-command/src/debug/timeit.rs +++ b/crates/nu-command/src/debug/timeit.rs @@ -1,6 +1,6 @@ use nu_engine::{command_prelude::*, ClosureEvalOnce}; use nu_protocol::engine::Closure; -use std::time::Instant; +use web_time::Instant; #[derive(Clone)] pub struct TimeIt; diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs index da57198efb..4f74e4b0a8 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -20,9 +20,9 @@ use std::{ io::{IsTerminal, Read}, path::PathBuf, str::FromStr, - time::Instant, }; use url::Url; +use web_time::Instant; const STREAM_PAGE_SIZE: usize = 1000; diff --git a/crates/nu-protocol/Cargo.toml b/crates/nu-protocol/Cargo.toml index 3f2817fa78..d162999b9d 100644 --- a/crates/nu-protocol/Cargo.toml +++ b/crates/nu-protocol/Cargo.toml @@ -40,6 +40,7 @@ thiserror = "2.0" typetag = "0.2" os_pipe = { workspace = true, optional = true, features = ["io_safety"] } log = { workspace = true } +web-time = { workspace = true } [target.'cfg(unix)'.dependencies] nix = { workspace = true, default-features = false, features = ["signal"] } diff --git a/crates/nu-protocol/src/debugger/profiler.rs b/crates/nu-protocol/src/debugger/profiler.rs index bffadfe5ef..70771ed50b 100644 --- a/crates/nu-protocol/src/debugger/profiler.rs +++ b/crates/nu-protocol/src/debugger/profiler.rs @@ -10,8 +10,8 @@ use crate::{ ir::IrBlock, record, PipelineData, ShellError, Span, Value, }; -use std::time::Instant; use std::{borrow::Borrow, io::BufRead}; +use web_time::Instant; #[derive(Debug, Clone, Copy)] struct ElementId(usize); diff --git a/crates/nu-system/Cargo.toml b/crates/nu-system/Cargo.toml index 9b0b6aab7d..c1134e9796 100644 --- a/crates/nu-system/Cargo.toml +++ b/crates/nu-system/Cargo.toml @@ -20,6 +20,7 @@ libc = { workspace = true } log = { workspace = true } sysinfo = { workspace = true } itertools = { workspace = true } +web-time = { workspace = true } [target.'cfg(target_family = "unix")'.dependencies] nix = { workspace = true, default-features = false, features = ["fs", "term", "process", "signal"] } diff --git a/crates/nu-system/src/windows.rs b/crates/nu-system/src/windows.rs index cc0ab99559..77b5b8834e 100644 --- a/crates/nu-system/src/windows.rs +++ b/crates/nu-system/src/windows.rs @@ -18,7 +18,8 @@ use std::ptr; use std::ptr::null_mut; use std::sync::LazyLock; use std::thread; -use std::time::{Duration, Instant}; +use std::time::Duration; +use web_time::Instant; use windows::core::{PCWSTR, PWSTR}; diff --git a/toolkit.nu b/toolkit.nu index d848dce18e..8b34e9a53b 100644 --- a/toolkit.nu +++ b/toolkit.nu @@ -6,6 +6,8 @@ # (**2**) catch classical flaws in the new changes with *clippy* and (**3**) # make sure all the tests pass. +const toolkit_dir = path self . + # check standard code formatting and apply the changes export def fmt [ --check # do not apply the format changes, only check the syntax @@ -363,40 +365,6 @@ export def build [ } } -# build crates for wasm -export def "build wasm" [] { - ^rustup target add wasm32-unknown-unknown - - # these crates should compile for wasm - let compatible_crates = [ - "nu-cmd-base", - "nu-cmd-extra", - "nu-cmd-lang", - "nu-color-config", - "nu-command", - "nu-derive-value", - "nu-engine", - "nu-glob", - "nu-json", - "nu-parser", - "nu-path", - "nu-pretty-hex", - "nu-protocol", - "nu-std", - "nu-system", - "nu-table", - "nu-term-grid", - "nu-utils", - "nuon" - ] - - for crate in $compatible_crates { - print $'(char nl)Building ($crate) for wasm' - print '----------------------------' - ^cargo build -p $crate --target wasm32-unknown-unknown --no-default-features - } -} - def "nu-complete list features" [] { open Cargo.toml | get features | transpose feature dependencies | get feature } @@ -625,4 +593,69 @@ export def 'release-pkg windows' [ } } +# these crates should compile for wasm +const wasm_compatible_crates = [ + "nu-cmd-base", + "nu-cmd-extra", + "nu-cmd-lang", + "nu-color-config", + "nu-command", + "nu-derive-value", + "nu-engine", + "nu-glob", + "nu-json", + "nu-parser", + "nu-path", + "nu-pretty-hex", + "nu-protocol", + "nu-std", + "nu-system", + "nu-table", + "nu-term-grid", + "nu-utils", + "nuon" +] + +def "prep wasm" [] { + ^rustup target add wasm32-unknown-unknown +} + +# build crates for wasm +export def "build wasm" [] { + prep wasm + + for crate in $wasm_compatible_crates { + print $'(char nl)Building ($crate) for wasm' + print '----------------------------' + ( + ^cargo build + -p $crate + --target wasm32-unknown-unknown + --no-default-features + ) + } +} + +# make sure no api is used that doesn't work with wasm +export def "clippy wasm" [] { + prep wasm + + $env.CLIPPY_CONF_DIR = $toolkit_dir | path join clippy wasm + + for crate in $wasm_compatible_crates { + print $'(char nl)Checking ($crate) for wasm' + print '----------------------------' + ( + ^cargo clippy + -p $crate + --target wasm32-unknown-unknown + --no-default-features + -- + -D warnings + -D clippy::unwrap_used + -D clippy::unchecked_duration_subtraction + ) + } +} + export def main [] { help toolkit }