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

Sync wasi-cli with wit definitions in standards repo #6806

Merged
merged 19 commits into from
Aug 15, 2023
Merged

Conversation

pchickey
Copy link
Contributor

@pchickey pchickey commented Aug 4, 2023

This PR synchronizes the wasi:cli package with the upstream spec definition.

As part of this PR I upstreamed changes from the wasmtime repo:
WebAssembly/wasi-cli#19
WebAssembly/wasi-cli#20

The changes arriving downstream in this change are

  1. eliminating the patchwork of "temporary" fixes to the cli definition as the wit language evolved alongside the implementation. This involved calling the package "cli-base" in the wasmtime repo, and putting world definitions separately in a "preview" package.
  2. adding implementations of the terminal-{input, output, stdin, stdout, stderr} functions to the cli implementation, which allows us to implement isatty, bringing preview2's view of stdio to parity with preview 1, where fd_filestat_get could be used to determine whether stdio were ttys. I made some changes in wasi-tests to exercise this functionality to show that it works in all 5 of our WASI implementations. (yes i roll my eyes every time i type all 5 wasi implementations too. because i wrote them. what have i done. what did they do to us)

@github-actions github-actions bot added the wasi Issues pertaining to WASI label Aug 5, 2023
@pchickey pchickey force-pushed the pch/sync_wasi_cli branch 2 times, most recently from 8ef69e5 to 0e59b26 Compare August 14, 2023 21:03
@pchickey pchickey marked this pull request as ready for review August 14, 2023 23:25
@pchickey pchickey requested review from a team as code owners August 14, 2023 23:25
@pchickey pchickey requested review from fitzgen and elliottt and removed request for a team and fitzgen August 14, 2023 23:25
Copy link
Member

@elliottt elliottt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!

Pat Hickey added 16 commits August 14, 2023 17:15
it turns out this isnt semantically meaningful, since the package name
is in the document itself now, but lets be consistient
…rminal

CI environments vary - let the test runner make sure the host process's
stdio is in fact a terminal before asserting that the guest sees it is a
terminal.
which means making a newtype around Stdout and Stderr instead of using
a type alias there.

and then use the is-terminal impl to fill in the isatty field in the
builder when inheriting. if you need to override it you can always
builder.stdin(stdio::stdin(), your_own_idea_of_isatty)
@elliottt elliottt enabled auto-merge August 15, 2023 00:16
@elliottt
Copy link
Member

(I rebased the branch on main to resolve the merge conflict.)

@elliottt elliottt added this pull request to the merge queue Aug 15, 2023
Merged via the queue into main with commit e3b4954 Aug 15, 2023
@elliottt elliottt deleted the pch/sync_wasi_cli branch August 15, 2023 16:57
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Aug 18, 2023
…e#6806)

* rename wasi-cli-base to wasi-cli, delete `preview` package, import wasi-cli

wasi-cli import is sum of
WebAssembly/wasi-cli#19 and
WebAssembly/wasi-cli#20

* wasi impl: change bindgen arguments and mod paths from cli_base to cli

* correct name of wasi-cli deps dir to just `deps/cli/`

it turns out this isnt semantically meaningful, since the package name
is in the document itself now, but lets be consistient

* track whether stdio isatty in ctx, and impl the cli/terminal-* interfaces

* rebase fixup

* wasi wits: define the reactor adapter's world

* component adapter: fixes

* test-programs/command-tests: fix renaming cli_base to cli

* component adapter: fix manually-defined export and import names

* test harness fixes

* preview1 component adapter: fill in isatty detection

* implement isatty in preview2-to-preview1 host adapter

* test-programs: cover both when stdio isatty and not

prtest:full

* split isatty test for regular file and stdio, detect host stdio is_terminal

CI environments vary - let the test runner make sure the host process's
stdio is in fact a terminal before asserting that the guest sees it is a
terminal.

* provide an is-terminal impl for all preview2's stdio types

which means making a newtype around Stdout and Stderr instead of using
a type alias there.

and then use the is-terminal impl to fill in the isatty field in the
builder when inheriting. if you need to override it you can always
builder.stdin(stdio::stdin(), your_own_idea_of_isatty)

* finally, rename IsATTY variants to Yes and No

* Fix the reference to IsATTY::No

* more forgotten renamings

---------

Co-authored-by: Trevor Elliott <telliott@fastly.com>
geekbeast pushed a commit to geekbeast/wasmtime that referenced this pull request Aug 18, 2023
…time into feature/wasi-nn-preview-2

* 'feature/wasi-nn-preview-2' of github.com:geekbeast/wasmtime:
  Memcheck for Wasm guests in Wasmtime (bytecodealliance#6820)
  CI: upgrade to qemu 8.0.4. (bytecodealliance#6849)
  Sync wasi-cli with wit definitions in standards repo  (bytecodealliance#6806)
  Rename `preview2::preview2` to `preview2::host` (bytecodealliance#6847)
  winch: Simplify the MacroAssembler and Assembler interfaces (bytecodealliance#6841)
  There are no files in `preview1` other than `mod.rs` (bytecodealliance#6845)
  Update stdio on Unix to fall back to worker threads (bytecodealliance#6833)
  Update RELEASES.md (bytecodealliance#6838)
  Minor documentation updates to docs/WASI-tutorial.md (bytecodealliance#6839)
  Add support for vector in DataValueExt::int() (bytecodealliance#6844)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasi Issues pertaining to WASI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants