rust icon indicating copy to clipboard operation
rust copied to clipboard

Test wasm32-wasip1 in CI, not wasm32-unknown-unknown

Open alexcrichton opened this issue 1 year ago • 13 comments

This commit changes CI to no longer test the wasm32-unknown-unknown target and instead test the wasm32-wasip1 target. There was some discussion of this in a Zulip thread, and the motivations for this PR are:

  • Runtime failures on wasm32-unknown-unknown print nothing, meaning all you get is "something failed". In contrast wasm32-wasip1 can print to stdout/stderr.

  • The unknown-unknown target is missing lots of pieces of libstd, and while wasm32-wasip1 is also missing some pieces (e.g. threads) it's missing fewer pieces. This means that many more tests can be run.

Overall my hope is to improve the debuggability of wasm failures on CI and ideally be a bit less of a maintenance burden.

This commit specifically removes the testing of wasm32-unknown-unknown and replaces it with testing of wasm32-wasip1. Along the way there were a number of other archiectural changes made as well, including:

  • A new target.*.runtool option can now be specified in config.toml which is passed as --runtool to compiletest. This is used to reimplement execution of WebAssembly in a less-wasm-specific fashion.

  • The default value for runtool is an ambiently located WebAssembly runtime found on the system, if any. I've implemented logic for Wasmtime.

  • Existing testing support for wasm32-unknown-unknown and Emscripten has been removed. I'm not aware of Emscripten testing being run any time recently and otherwise wasm32-wasip1 is in theory the focus now.

  • I've added a new //@ needs-threads directive for compiletest and classified a bunch of wasm-ignored tests as needing threads. In theory these tests can run on wasm32-wasi-preview1-threads, for example.

  • I've tried to audit all existing tests that are either ignore-emscripten or ignore-wasm*. Many now run on wasm32-wasip1 due to being able to emit error messages, for example. Many are updated with comments as to why they can't run as well.

  • The compiletest output matching for wasm32-wasip1 automatically uses "match a subset" mode implemented in compiletest. This is because WebAssembly runtimes often add extra information on failure, such as the unreachable instruction in panic!, which isn't able to be matched against the golden output from native platforms.

  • I've ported most existing run-make tests that use custom Node.js wrapper scripts to the new run-make-based-in-Rust infrastructure. To do this I added wasmparser as a dependency of run-make-support for the various wasm tests to use that parse wasm files. The one test that executed WebAssembly now uses wasmtime-the-CLI to execute the test instead. I have not ported over an exception-handling test as Wasmtime doesn't implement this yet.

  • I've updated the test crate to print out timing information for WASI targets as it can do that (gets a previously ignored test now passing).

  • The test-various image now builds a WASI sysroot for the WASI target and additionally downloads a fixed release of Wasmtime, currently the latest one at 18.0.2, and uses that for testing.

alexcrichton avatar Mar 05 '24 17:03 alexcrichton

r? @oli-obk

rustbot has assigned @oli-obk. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot avatar Mar 05 '24 17:03 rustbot

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged. Otherwise, you can ignore this comment.

This PR modifies config.example.toml.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

rustbot avatar Mar 05 '24 17:03 rustbot

The job mingw-check-tidy failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Getting action download info
Download action repository 'msys2/[email protected]' (SHA:cc11e9188b693c2b100158c3322424c4cc1dadea)
Download action repository 'actions/checkout@v4' (SHA:b4ffde65f46336ab88eb53be808477a3936bae11)
Download action repository 'actions/upload-artifact@v3' (SHA:a8a3f3ad30e3422c9c7b888a15615d19a852ae32)
Complete job name: PR - mingw-check-tidy
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
---
GITHUB_ENV=/home/runner/work/_temp/_runner_file_commands/set_env_876ab846-3a75-4759-a046-8b6bcf9872f5
GITHUB_EVENT_NAME=pull_request
GITHUB_EVENT_PATH=/home/runner/work/_temp/_github_workflow/event.json
GITHUB_GRAPHQL_URL=https://api.github.com/graphql
GITHUB_HEAD_REF=test-wasm-with-wasi
GITHUB_JOB=pr
GITHUB_PATH=/home/runner/work/_temp/_runner_file_commands/add_path_876ab846-3a75-4759-a046-8b6bcf9872f5
GITHUB_REF=refs/pull/122036/merge
GITHUB_REF_NAME=122036/merge
GITHUB_REF_PROTECTED=false
---
COPY scripts/sccache.sh /scripts/
RUN sh /scripts/sccache.sh

COPY host-x86_64/mingw-check/reuse-requirements.txt /tmp/
RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-requirements.txt \
    && pip3 install virtualenv
COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
COPY host-x86_64/mingw-check/validate-error-codes.sh /scripts/

# NOTE: intentionally uses python2 for x.py so we can test it still works.
# NOTE: intentionally uses python2 for x.py so we can test it still works.
# validate-toolstate only runs in our CI, so it's ok for it to only support python3.
ENV SCRIPT TIDY_PRINT_DIFF=1 python2.7 ../x.py test \
           --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint
# This file is autogenerated by pip-compile with Python 3.10
# by the following command:
#
#    pip-compile --allow-unsafe --generate-hashes reuse-requirements.in
---

#10 [5/8] COPY host-x86_64/mingw-check/reuse-requirements.txt /tmp/
#10 DONE 0.0s

#11 [6/8] RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-requirements.txt     && pip3 install virtualenv
#11 0.596   Downloading binaryornot-0.4.4-py2.py3-none-any.whl (9.0 kB)
#11 0.612 Collecting boolean-py==4.0
#11 0.619   Downloading boolean.py-4.0-py3-none-any.whl (25 kB)
#11 0.636 Collecting chardet==5.1.0
---
#11 3.787 Building wheels for collected packages: reuse
#11 3.788   Building wheel for reuse (pyproject.toml): started
#11 4.119   Building wheel for reuse (pyproject.toml): finished with status 'done'
#11 4.120   Created wheel for reuse: filename=reuse-1.1.0-cp310-cp310-manylinux_2_35_x86_64.whl size=181117 sha256=f5f58750481f69515c2c0d1d503daf565e2565c370d07fc6aeb95fe3498b4269
#11 4.120   Stored in directory: /tmp/pip-ephem-wheel-cache-3r5ndo0g/wheels/c2/3c/b9/1120c2ab4bd82694f7e6f0537dc5b9a085c13e2c69a8d0c76d
#11 4.123 Installing collected packages: boolean-py, binaryornot, setuptools, reuse, python-debian, markupsafe, license-expression, jinja2, chardet
#11 4.147   Attempting uninstall: setuptools
#11 4.147     Found existing installation: setuptools 59.6.0
#11 4.148     Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
#11 4.148     Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
#11 4.149     Can't uninstall 'setuptools'. No files were found to uninstall.
#11 4.826 Successfully installed binaryornot-0.4.4 boolean-py-4.0 chardet-5.1.0 jinja2-3.1.2 license-expression-30.0.0 markupsafe-2.1.1 python-debian-0.1.49 reuse-1.1.0 setuptools-66.0.0
#11 4.827 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#11 5.344 Collecting virtualenv
#11 5.397   Downloading virtualenv-20.25.1-py3-none-any.whl (3.8 MB)
#11 5.588      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 3.8/3.8 MB 20.0 MB/s eta 0:00:00
#11 5.629 Collecting distlib<1,>=0.3.7
#11 5.636   Downloading distlib-0.3.8-py2.py3-none-any.whl (468 kB)
#11 5.648      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 468.9/468.9 KB 47.1 MB/s eta 0:00:00
#11 5.697 Collecting platformdirs<5,>=3.9.1
#11 5.705   Downloading platformdirs-4.2.0-py3-none-any.whl (17 kB)
#11 5.738 Collecting filelock<4,>=3.12.2
#11 5.745   Downloading filelock-3.13.1-py3-none-any.whl (11 kB)
#11 5.831 Installing collected packages: distlib, platformdirs, filelock, virtualenv
#11 5.994 Successfully installed distlib-0.3.8 filelock-3.13.1 platformdirs-4.2.0 virtualenv-20.25.1
#11 DONE 6.1s

#12 [7/8] COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
#12 DONE 0.0s
---
DirectMap4k:      180160 kB
DirectMap2M:     6111232 kB
DirectMap1G:    12582912 kB
##[endgroup]
Executing TIDY_PRINT_DIFF=1 python2.7 ../x.py test            --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint
+ TIDY_PRINT_DIFF=1 python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint
    Finished dev [unoptimized] target(s) in 0.03s
##[endgroup]
downloading https://ci-artifacts.rust-lang.org/rustc-builds-alt/c7beecf3e3cef7a8226a99aec4e4f6bfc114ba8e/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz
extracting /checkout/obj/build/cache/llvm-c7beecf3e3cef7a8226a99aec4e4f6bfc114ba8e-true/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz to /checkout/obj/build/x86_64-unknown-linux-gnu/ci-llvm
---
    Finished release [optimized] target(s) in 27.46s
##[endgroup]
fmt check
tidy check
tidy error: Stray file with UI testing output: "/checkout/tests/ui/test-attrs/test-passed-wasm.run.stdout"
removing old virtual environment
removing old virtual environment
creating virtual environment at '/checkout/obj/build/venv' using 'python3.10'
Requirement already satisfied: pip in ./build/venv/lib/python3.10/site-packages (24.0)
Collecting black==23.3.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 7))
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.7/1.7 MB 15.7 MB/s eta 0:00:00
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.7/1.7 MB 15.7 MB/s eta 0:00:00
Collecting click==8.1.3 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 34))
  Downloading click-8.1.3-py3-none-any.whl (96 kB)
Collecting importlib-metadata==6.7.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 38))
  Downloading importlib_metadata-6.7.0-py3-none-any.whl (22 kB)
  Downloading importlib_metadata-6.7.0-py3-none-any.whl (22 kB)
Collecting mypy-extensions==1.0.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 42))
  Downloading mypy_extensions-1.0.0-py3-none-any.whl (4.7 kB)
Collecting packaging==23.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 46))
  Downloading packaging-23.1-py3-none-any.whl (48 kB)
Collecting pathspec==0.11.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 50))
  Downloading pathspec-0.11.1-py3-none-any.whl (29 kB)
  Downloading pathspec-0.11.1-py3-none-any.whl (29 kB)
Collecting platformdirs==3.6.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 54))
  Downloading platformdirs-3.6.0-py3-none-any.whl (16 kB)
Collecting ruff==0.0.272 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 58))
  Downloading ruff-0.0.272-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (5.9 MB)
Collecting tomli==2.0.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 77))
  Downloading tomli-2.0.1-py3-none-any.whl (12 kB)
Collecting typed-ast==1.5.4 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 81))
  Downloading typed_ast-1.5.4-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl (877 kB)
  Downloading typed_ast-1.5.4-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl (877 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 877.7/877.7 kB 69.3 MB/s eta 0:00:00
Collecting typing-extensions==4.6.3 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 107))
  Downloading typing_extensions-4.6.3-py3-none-any.whl (31 kB)
Collecting zipp==3.15.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 114))
  Downloading zipp-3.15.0-py3-none-any.whl (6.8 kB)
Installing collected packages: zipp, typing-extensions, typed-ast, tomli, ruff, platformdirs, pathspec, packaging, mypy-extensions, click, importlib-metadata, black
Successfully installed black-23.3.0 click-8.1.3 importlib-metadata-6.7.0 mypy-extensions-1.0.0 packaging-23.1 pathspec-0.11.1 platformdirs-3.6.0 ruff-0.0.272 tomli-2.0.1 typed-ast-1.5.4 typing-extensions-4.6.3 zipp-3.15.0
some tidy checks failed
Build completed unsuccessfully in 0:01:02
  local time: Tue Mar  5 17:51:44 UTC 2024
  network time: Tue, 05 Mar 2024 17:51:44 GMT

rust-log-analyzer avatar Mar 05 '24 17:03 rust-log-analyzer

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged. Otherwise, you can ignore this comment.

This PR modifies config.example.toml.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

rustbot avatar Mar 05 '24 17:03 rustbot

please land the following on master as separate PRs:

  • I've added a new //@ needs-threads directive for compiletest and classified a bunch of wasm-ignored tests as needing threads. In theory these tests can run on wasm32-wasi-preview1-threads, for example.
  • A new target.*.runtool option can now be specified in config.toml which is passed as --runtool to compiletest. This is used to reimplement execution of WebAssembly in a less-wasm-specific fashion.
  • Existing testing support for [..] Emscripten has been removed. I'm not aware of Emscripten testing being run any time recently

oli-obk avatar Mar 05 '24 17:03 oli-obk

please land the following on master as separate PRs:

Can do! I'll do that over the next few days.

alexcrichton avatar Mar 05 '24 18:03 alexcrichton

I've split out https://github.com/rust-lang/rust/pull/122108 and https://github.com/rust-lang/rust/pull/122109 and then rebased this PR on top of them (that's the first two commits). I've split the remaining commit into smaller chunks as well with this commit being the one that removes some Emscripten-specific bits. I'd like to confirm, but would you like me to still separate out that to its own PR? Happy to do so, but the change would be quite small since the wasm32-unknown-unknown bits would need to stick around until this PR lands.

alexcrichton avatar Mar 06 '24 20:03 alexcrichton

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
GITHUB_ENV=/home/runner/work/_temp/_runner_file_commands/set_env_3a3f9432-bce9-4315-bbe3-dc25ffedec1d
GITHUB_EVENT_NAME=pull_request
GITHUB_EVENT_PATH=/home/runner/work/_temp/_github_workflow/event.json
GITHUB_GRAPHQL_URL=https://api.github.com/graphql
GITHUB_HEAD_REF=test-wasm-with-wasi
GITHUB_JOB=pr
GITHUB_PATH=/home/runner/work/_temp/_runner_file_commands/add_path_3a3f9432-bce9-4315-bbe3-dc25ffedec1d
GITHUB_REF=refs/pull/122036/merge
GITHUB_REF_NAME=122036/merge
GITHUB_REF_PROTECTED=false
---
#13 4.179 Building wheels for collected packages: reuse
#13 4.180   Building wheel for reuse (pyproject.toml): started
#13 4.507   Building wheel for reuse (pyproject.toml): finished with status 'done'
#13 4.508   Created wheel for reuse: filename=reuse-1.1.0-cp310-cp310-manylinux_2_35_x86_64.whl size=181117 sha256=f5f58750481f69515c2c0d1d503daf565e2565c370d07fc6aeb95fe3498b4269
#13 4.508   Stored in directory: /tmp/pip-ephem-wheel-cache-hz6zl9tf/wheels/c2/3c/b9/1120c2ab4bd82694f7e6f0537dc5b9a085c13e2c69a8d0c76d
#13 4.511 Installing collected packages: boolean-py, binaryornot, setuptools, reuse, python-debian, markupsafe, license-expression, jinja2, chardet
#13 4.533   Attempting uninstall: setuptools
#13 4.534     Found existing installation: setuptools 59.6.0
#13 4.535     Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
---
   Compiling lazycell v1.3.0
   Compiling home v0.5.9
   Compiling glob v0.3.1
   Compiling compiletest v0.0.0 (/checkout/src/tools/compiletest)
error[E0609]: no field `runtool` on type `&'test Config`
     |
     |
4656 |             self.props.compare_output_lines_by_subset || self.config.runtool.is_some();
     |
     |
     = note: available fields are: `bless`, `compile_lib_path`, `run_lib_path`, `rustc_path`, `rustdoc_path` ... and 72 others
For more information about this error, try `rustc --explain E0609`.
error: could not compile `compiletest` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
Build completed unsuccessfully in 0:00:58

rust-log-analyzer avatar Mar 06 '24 20:03 rust-log-analyzer

:umbrella: The latest upstream changes (presumably #122050) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Mar 11 '24 07:03 bors

@rustbot ready

alexcrichton avatar Mar 11 '24 16:03 alexcrichton

Thanks for the PR and commit splits!

@bors r+ rollup=never (let's not mix CI changes with other PRs)

oli-obk avatar Mar 11 '24 16:03 oli-obk

:pushpin: Commit cf6d6050f7d1ea62c9aae54ddd345106b6e31382 has been approved by oli-obk

It is now in the queue for this repository.

bors avatar Mar 11 '24 16:03 bors

:hourglass: Testing commit cf6d6050f7d1ea62c9aae54ddd345106b6e31382 with merge dc2ffa405407ffb3654658c50ab3dfda124fbdfd...

bors avatar Mar 12 '24 00:03 bors

:sunny: Test successful - checks-actions Approved by: oli-obk Pushing dc2ffa405407ffb3654658c50ab3dfda124fbdfd to master...

bors avatar Mar 12 '24 02:03 bors

Finished benchmarking commit (dc2ffa405407ffb3654658c50ab3dfda124fbdfd): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
2.7% [2.7%, 2.7%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.2% [-2.2%, -0.1%] 2
Improvements ✅
(secondary)
-3.6% [-5.2%, -2.0%] 2
All ❌✅ (primary) 0.1% [-2.2%, 2.7%] 3

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.1% [-3.7%, -2.4%] 2
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 671.842s -> 672.134s (0.04%) Artifact size: 310.08 MiB -> 310.00 MiB (-0.03%)

rust-timer avatar Mar 12 '24 03:03 rust-timer