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

* make the witx cli tool a separate crate

* delete phases.rs: not the witx crate's responsibility

* use `witx docs --check` to replace specialized tests

* repo-docs can be a shell script

* parallelize CI a little

* wibble

* typo; only need to validate witx on one os

* add ephemeral

* delete wasi-docs test file

* i love too program in yaml

* repo-docs: reduce duplication

* drop dev-dependencies in witx

Author: Pat Hickey

.github/workflows/main.yml

@@ -3,7 +3,7 @@ on: [push, pull_request]
 
 jobs:
   test:
-    name: Test
+    name: Test witx tools
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
@@ -21,17 +21,27 @@ jobs:
       if: matrix.os == 'macos-latest'
     - run: cargo fetch
       working-directory: tools/witx
-    - run: cargo build
-      working-directory: tools/witx
-    - run: cargo test
+    - run: cargo test --all
       working-directory: tools/witx
 
+  validate:
+    name: Validate witx files and docs
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v1
+    - name: Install Rust (rustup)
+      shell: bash
+      run: rustup update stable --no-self-update && rustup default stable
+
+    - name: Check that repository docs reflect witx
+      run: ./tools/repo_docs.sh --check
+
   rustfmt:
     name: Rustfmt
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v1
     - name: Install Rust
       run: rustup update stable && rustup default stable && rustup component add rustfmt
-    - run: cargo fmt -- --check
+    - run: cargo fmt --all -- --check
       working-directory: tools/witx

tools/repo_docs.sh

@@ -0,0 +1,17 @@
+#!/usr/bin/env bash
+set -ex
+cd $(dirname $(realpath $0))/witx
+cargo run -p witx-cli -- docs $1 ../../phases/snapshot/witx/wasi_snapshot_preview1.witx --output ../../phases/snapshot/docs.md
+cargo run -p witx-cli -- docs $1 ../../phases/old/snapshot_0/witx/wasi_unstable.witx --output ../../phases/old/snapshot_0/docs.md
+cargo run -p witx-cli -- docs $1 \
+  ../../phases/ephemeral/witx/wasi_ephemeral_args.witx \
+  ../../phases/ephemeral/witx/wasi_ephemeral_clock.witx \
+  ../../phases/ephemeral/witx/wasi_ephemeral_environ.witx \
+  ../../phases/ephemeral/witx/wasi_ephemeral_fd.witx \
+  ../../phases/ephemeral/witx/wasi_ephemeral_path.witx \
+  ../../phases/ephemeral/witx/wasi_ephemeral_poll.witx \
+  ../../phases/ephemeral/witx/wasi_ephemeral_proc.witx \
+  ../../phases/ephemeral/witx/wasi_ephemeral_random.witx \
+  ../../phases/ephemeral/witx/wasi_ephemeral_sched.witx \
+  ../../phases/ephemeral/witx/wasi_ephemeral_sock.witx \
+  --output ../../phases/ephemeral/docs.md

tools/witx/Cargo.toml

@@ -20,11 +20,13 @@ thiserror = "1.0"
 wast = { version = "33.0.0", default-features = false }
 
 [dev-dependencies]
-diff = "0.1.11"
-pretty_env_logger = "0.4"
-structopt = "0.3"
 rayon = "1.0"
 
 [[test]]
 name = "witxt"
 harness = false
+
+[workspace]
+members = [
+  "cli",
+]

tools/witx/cli/Cargo.toml

@@ -0,0 +1,25 @@
+[package]
+name = "witx-cli"
+version = "0.9.0"
+description = "CLI for operating on witx file format"
+homepage = "https://github.com/WebAssembly/WASI"
+repository = "https://github.com/WebAssembly/WASI"
+license = "Apache-2.0"
+categories = ["wasm"]
+keywords = ["webassembly", "wasm"]
+authors = ["Pat Hickey <[email protected]>", "Alex Crichton <[email protected]>"]
+edition = "2018"
+
+[[bin]]
+name = "witx"
+path = "src/main.rs"
+
+[dependencies]
+witx = { path = "../", version = "0.9.0" }
+anyhow = "1"
+log = "0.4"
+thiserror = "1.0"
+diff = "0.1.11"
+pretty_env_logger = "0.4"
+structopt = "0.3"
+rayon = "1.0"

tools/witx/examples/witx.rs renamed to tools/witx/cli/src/main.rs

@@ -4,7 +4,7 @@ use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::process;
 use structopt::{clap::AppSettings, StructOpt};
-use witx::{load, phases, Document, Documentation};
+use witx::{load, Document, Documentation};
 
 /// Validate and process witx files
 #[derive(StructOpt, Debug)]
@@ -31,6 +31,9 @@ enum Command {
         /// Path to root of witx document
         #[structopt(number_of_values = 1, value_name = "INPUT", parse(from_os_str))]
         input: Vec<PathBuf>,
+        /// Perform check that output matches witx documents
+        #[structopt(long = "check")]
+        check: bool,
         /// Path to generated documentation in Markdown format
         #[structopt(
             short = "o",
@@ -40,8 +43,6 @@ enum Command {
         )]
         output: Option<PathBuf>,
     },
-    /// Update documentation in WASI repository to reflect witx specs
-    RepoDocs,
     /// Examine differences between interfaces
     Polyfill {
         /// Path to root of witx document
@@ -80,22 +81,32 @@ pub fn main() {
     let verbose = args.verbose;
 
     match args.cmd {
-        Command::Docs { input, output } => {
+        Command::Docs {
+            input,
+            check,
+            output,
+        } => {
             let doc = load_witx(&input, "input", verbose);
-            if let Some(output) = output {
-                write_docs(&doc, output)
+            if check {
+                let output = output.expect("output argument required in docs --check mode");
+                if diff_against_filesystem(&doc.to_md(), &output).is_err() {
+                    println!("Docs in tree are out-of-date with witx files. Re-run this executable with the following arguments to to re-generate:");
+                    println!(
+                        "> witx docs {} --output {}",
+                        input
+                            .iter()
+                            .map(|p| p.to_string_lossy().into_owned())
+                            .collect::<Vec<String>>()
+                            .join(" "),
+                        output.to_string_lossy(),
+                    );
+                }
             } else {
-                println!("{}", doc.to_md())
-            }
-        }
-        Command::RepoDocs => {
-            for phase in &[
-                phases::snapshot().unwrap(),
-                phases::ephemeral().unwrap(),
-                phases::old::snapshot_0().unwrap(),
-            ] {
-                let doc = load(&phase).expect("parse phase");
-                write_docs(&doc, phases::docs_path(&phase));
+                if let Some(output) = output {
+                    write_docs(&doc, output)
+                } else {
+                    println!("{}", doc.to_md())
+                }
             }
         }
         Command::Polyfill {
@@ -173,3 +184,87 @@ fn parse_module_mapping(m: &str) -> Result<(String, String)> {
     };
     Ok((n.to_string(), o.to_string()))
 }
+
+fn dos2unix(s: &str) -> String {
+    let mut t = String::new();
+    t.reserve(s.len());
+    for c in s.chars() {
+        if c != '\r' {
+            t.push(c)
+        }
+    }
+    t
+}
+
+fn diff_against_filesystem(expected: &str, path: &Path) -> Result<(), ()> {
+    let actual = std::fs::read_to_string(path)
+        .unwrap_or_else(|e| panic!("couldn't read {}: {:?}", Path::display(path), e));
+    // Git may checkout the file with dos line endings on windows. Strip all \r:
+    let actual = dos2unix(&actual);
+    if &actual == expected {
+        return Ok(());
+    }
+
+    eprintln!("The following diff was found between the docs generated from .witx and the");
+    eprintln!("source {:?} in the tree:", path);
+    eprintln!();
+
+    let mut expected_line = 1;
+    let mut actual_line = 1;
+    let mut separated = false;
+    let mut any_lines = false;
+    for diff in diff::lines(&expected, &actual) {
+        match diff {
+            diff::Result::Left(l) => {
+                eprintln!("line {}: -{}", expected_line, l);
+                expected_line += 1;
+                separated = false;
+                any_lines = true;
+            }
+            diff::Result::Both(_, _) => {
+                expected_line += 1;
+                actual_line += 1;
+                if !separated {
+                    eprintln!("...");
+                    separated = true;
+                }
+            }
+            diff::Result::Right(r) => {
+                eprintln!("line {}: +{}", actual_line, r);
+                actual_line += 1;
+                separated = false;
+                any_lines = true;
+            }
+        }
+    }
+
+    if !any_lines {
+        eprintln!();
+        eprintln!(
+            "Somehow there was a diff with no lines differing. Lengths: {} and {}.",
+            expected.len(),
+            actual.len()
+        );
+        for (index, (a, b)) in actual.chars().zip(expected.chars()).enumerate() {
+            if a != b {
+                eprintln!("char difference at index {}: '{}' != '{}'", index, a, b);
+            }
+        }
+        for (index, (a, b)) in actual.bytes().zip(expected.bytes()).enumerate() {
+            if a != b {
+                eprintln!("byte difference at index {}: b'{}' != b'{}'", index, a, b);
+            }
+        }
+        eprintln!();
+        eprintln!("actual: {}", actual);
+        eprintln!();
+        eprintln!("expected: {}", expected);
+    }
+
+    eprintln!();
+    eprintln!(
+        "To regenerate the files, run `cd tools/witx && cargo run --example witx repo-docs`."
+    );
+    eprintln!();
+    Err(())
+}

tools/witx/src/lib.rs

@@ -10,8 +10,6 @@ mod io;
 mod layout;
 /// Witx syntax parsing from SExprs
 pub mod parser;
-/// Paths to witx documents for various proposal phases
-pub mod phases;
 /// Calculate required polyfill between interfaces
 pub mod polyfill;
 /// Render ast to text