Skip to content

make witx cli a new crate, refactor how we validate repo contents #398

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

Merged
merged 12 commits into from
Feb 24, 2021

Conversation

pchickey
Copy link
Contributor

These changes are designed to make the witx tooling more useful to proposal forks of this repository.

  • Rather than keep the cli-oriented functionality of the witx crate in an example, make a new crate witx-cli that creates a witx executable.
  • Add a --check flag to the witx docs subcommand that checks if docs in the filesystem match those calculated from the witx files.
  • Rewrite the wasi-docs test to invoke witx docs --check. This test now can live in .github/workflows/main.yml rather than in the rust sources.
  • Testing of the witx tooling and validating witx specs & docs can now happen in parallel in CI.
  • Speaking of things that don't belong in the rust sources: paths to witx files don't belong in there, so delete the witx::phases module. Really they never belonged in there, but I didn't see the harm before we started the repo re-organization in Repository organization #360.

@pchickey pchickey merged commit 7ec4b1a into main Feb 24, 2021
@pchickey pchickey deleted the pch/witx_cli branch February 24, 2021 18:47
cratelyn pushed a commit to cratelyn/WASI that referenced this pull request Jun 22, 2021
Per this discussion from WebAssembly#434:

> It looks like the crate was last published at
> [7ec4b1a](WebAssembly@7ec4b1a)
> ([diff from
> `main`](WebAssembly/WASI@ef8c1a5...main)),
> and if we'd like to make an 0.9.1 release (ideally just a bugfix
> release), I think there's another breaking change we merged in the
> meantime, notably the deletion of `tools/witx/src/phases.rs`, which I
> think happened as WebAssembly#398. I think we may need to revert that as well
> before publishing 0.9.1?

    - [@alexcrichton](WebAssembly#434 (comment))

This reintroduces 'witx::phases', so that we do not include any breaking
changes in what ought to be a pure bug release.
alexcrichton pushed a commit that referenced this pull request Jun 22, 2021
Per this discussion from #434:

> It looks like the crate was last published at
> [7ec4b1a](7ec4b1a)
> ([diff from
> `main`](ef8c1a5...main)),
> and if we'd like to make an 0.9.1 release (ideally just a bugfix
> release), I think there's another breaking change we merged in the
> meantime, notably the deletion of `tools/witx/src/phases.rs`, which I
> think happened as #398. I think we may need to revert that as well
> before publishing 0.9.1?

    - [@alexcrichton](#434 (comment))

This reintroduces 'witx::phases', so that we do not include any breaking
changes in what ought to be a pure bug release.
cratelyn pushed a commit to cratelyn/wasmtime that referenced this pull request Jun 24, 2021
This commit removes some items from the root manifest's workspace
members array, and adds `witx-cli` to the root `workspace.exclude`
array.

The motivation for this stems from a cargo bug described in
rust-lang/cargo#6745: `workspace.exclude` does not work if it is nested
under a `workspace.members` path.

See WebAssembly/WASI#438 for the underlying change to the WASI submodule
which reorganized the `witx-cli` crate, and WebAssembly/WASI#398 for the
original PR introducing `witx-cli`.

See [this
comment](bytecodealliance#3025 (comment))
for more details about the compilation errors, and failed alternative
approaches that necessitated this change.

N.B. This is not a functional change, these crates are still implicitly
workspace members as transitive dependencies, but this will allow us to
side-step the aforementioned cargo bug.

Co-Authored-By: Alex Crichton <[email protected]>
alexcrichton added a commit to bytecodealliance/wasmtime that referenced this pull request Jun 24, 2021
* wasi-common: update wasi submodule

This updates the WASI submodule, pulling in changes to the witx crate,
now that there is a 0.9.1 version including some bug fixes. See
WebAssembly/WASI#434 for more information.

* wiggle: update witx dependencies

* publish: verify and vendor witx-cli

* adjust root workspace members

This commit removes some items from the root manifest's workspace
members array, and adds `witx-cli` to the root `workspace.exclude`
array.

The motivation for this stems from a cargo bug described in
rust-lang/cargo#6745: `workspace.exclude` does not work if it is nested
under a `workspace.members` path.

See WebAssembly/WASI#438 for the underlying change to the WASI submodule
which reorganized the `witx-cli` crate, and WebAssembly/WASI#398 for the
original PR introducing `witx-cli`.

See [this
comment](#3025 (comment))
for more details about the compilation errors, and failed alternative
approaches that necessitated this change.

N.B. This is not a functional change, these crates are still implicitly
workspace members as transitive dependencies, but this will allow us to
side-step the aforementioned cargo bug.

Co-Authored-By: Alex Crichton <[email protected]>

Co-authored-by: Alex Crichton <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants