Add cpp-to-wasm test (#968)

* Add cpp-to-wasm test

* Run wasm test on CI

* Fix CI

* Add host test to test-ffi

* pin emsdk

* Pin emscripten test to older nightly too

.github/workflows/build-test.yml

@@ -229,7 +229,9 @@ jobs:
       - name: Load nightly Rust toolchain for WASM.
         run: |
           rustup install nightly-2021-02-28
-          rustup target add wasm32-unknown-unknown --toolchain nightly-2021-02-28-x86_64-unknown-linux-gnu
+          rustup target add wasm32-unknown-unknown --toolchain nightly-2021-02-28
+          rustup target add wasm32-unknown-emscripten --toolchain nightly-2021-02-28
+          rustup component add rust-src --toolchain nightly-2021-02-28
       - name: Install WASM tools
         run: |
           sudo apt-get install wabt binaryen
@@ -273,7 +275,13 @@ jobs:
         with:
           crate: cargo-make
           version: latest
-
+      - name: Install emsdk
+        run: |
+          cd ~
+          git clone https://github.com/emscripten-core/emsdk.git --branch 2.0.27
+          cd emsdk
+          ./emsdk install latest
+          ./emsdk activate latest
       - name: Get Diplomat version
         id: diplomat-version
         run: |
@@ -310,6 +318,12 @@ jobs:
         with:
           command: make
           args: wasm-test-release
+      # This has to be a separate test since the emscripten sdk
+      # will otherwise interfere with other node-using tests
+      - name: Run emscripten test
+        run: |
+          . ~/emsdk/emsdk_env.sh
+          cargo make wasm-cpp-emscripten
 
   # Fmt job - runs cargo fmt
   fmt:

Makefile.toml

@@ -87,7 +87,11 @@ dependencies = [
 description = "Run all tests for the CI 'wasm' job"
 category = "CI"
 dependencies = [
-    "wasm-test-release",
+    # note: CI does not call `cargo make ci-job-wasm` since
+    # we have to set up the environment for the emscripten job separately
+    # Instead, each of these is called individually.
+    "wasm-release",
+    "wasm-cpp-emscripten",
 ]
 
 [tasks.ci-all]

ffi/capi/Cargo.toml

@@ -31,11 +31,11 @@ all-features = true
 skip_optional_dependencies = true
 # Bench feature gets tested separately and is only relevant for CI.
 # wearos/freertos/x86tiny are not relevant in normal environments,
-# and smaller_static gets tested on the FFI job anyway
+# smaller_static gets tested on the FFI job anyway
 denylist = ["bench", "wearos", "freertos", "x86tiny", "smaller_static"]
 
 [lib]
-crate-type = ["staticlib", "rlib", "cdylib"]
+crate-type = ["staticlib", "rlib"]
 path = "src/lib.rs"
 
 [features]
@@ -86,3 +86,20 @@ icu_provider_fs = { path = "../../provider/fs/", optional = true }
 [target.'cfg(target_os = "none")'.dependencies]
 freertos-rust = { version = "0.1.2", optional = true }
 cortex-m = { version = "0.7.3", optional = true }
+
+# Unfortunately, --crate-type cannot be set per-target
+# (https://github.com/rust-lang/cargo/issues/4881)
+# and emscripten has link errors when compiling icu_capi due to
+# symbols like log_js being undefined. There is no way to ask Cargo
+# to only build a particular crate type for an invocation
+#
+# As a workaround, we define an example crate that just reexports icu_capi,
+# but is built as a cdylib. Due to how Cargo invocations work around examples,
+# `--features` is still passed down to `icu_capi`, but the end result is an
+# `icu_capi_cdylib.wasm`/`icu_capi_cdylib.so`/etc file that is for all intents
+# and purposes identical to the file one would get from adding "cdylib" to
+# `crate-type` above.
+[[example]]
+name = "icu_capi_cdylib"
+path = "src/crate_type_hack.rs"
+crate-type = ["cdylib"]

ffi/capi/src/crate_type_hack.rs

@@ -0,0 +1,10 @@
+// This file is part of ICU4X. For terms of use, please see the file
+// called LICENSE at the top level of the ICU4X source tree
+// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).
+
+// See comment in icu_capi's Cargo.toml
+//
+// This is essentially a hack that allows icu_capi to be compiled
+// to arbitrary `--crate-type`s without having to add a `--crate-type`
+// to the list in Cargo.toml
+extern crate icu_capi;

ffi/cpp/examples/fixeddecimal/Makefile

@@ -22,4 +22,4 @@ a.out: ../../../../target/debug/libicu_capi.a $(ALL_HEADERS) test.cpp
 build: a.out
 
 test: build
-	./a.out
+	./a.out

ffi/cpp/examples/fixeddecimal/test.cpp

@@ -12,7 +12,6 @@ int main() {
     ICU4XLocale locale = ICU4XLocale::create("bn").value();
     std::cout << "Running test for locale " << locale.tostring().ok().value() << std::endl;
     ICU4XDataProvider dp = ICU4XDataProvider::create_fs(path).provider.value();
-
     ICU4XFixedDecimalFormatOptions opts = {ICU4XFixedDecimalGroupingStrategy::Auto, ICU4XFixedDecimalSignDisplay::Auto};
     ICU4XFixedDecimalFormat fdf = ICU4XFixedDecimalFormat::try_new(locale, dp, opts).fdf.value();
 

ffi/cpp/examples/fixeddecimal_wasm/.gitignore

@@ -0,0 +1,8 @@
+web-version.html
+web-version.wasm
+web-version.js
+node-version.js
+node-version.wasm
+package-lock.json
+node_modules
+a.out

ffi/cpp/examples/fixeddecimal_wasm/Makefile

@@ -0,0 +1,53 @@
+# This file is part of ICU4X. For terms of use, please see the file
+# called LICENSE at the top level of the ICU4X source tree
+# (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).
+
+.DEFAULT_GOAL := build
+.PHONY: build test clean serve build-host test-host
+
+ALL_HEADERS := $(wildcard ../../include/*.hpp) $(wildcard ../../../capi/include/*.h)
+ALL_RUST := $(wildcard ../../../capi//src/*.rs)
+
+$(ALL_RUST):
+
+$(ALL_HEADERS):
+
+../../../../target/debug/libicu_capi.a: $(ALL_RUST)
+	cargo build -p icu_capi
+
+a.out: ../../../../target/debug/libicu_capi.a $(ALL_HEADERS) test.cpp
+	g++ -std=c++17 test.cpp ../../../../target/debug/libicu_capi.a -ldl -lpthread -lm -g
+
+../../../../target/wasm32-unknown-emscripten/release/libicu_capi.a: $(ALL_RUST)
+	RUSTFLAGS="-Cpanic=abort" cargo +nightly-2021-02-28 build --release -p icu_capi --target wasm32-unknown-emscripten -Z build-std=std,panic_abort -Z build-std-features=panic_immediate_abort
+
+web-version.html: ../../../../target/wasm32-unknown-emscripten/release/libicu_capi.a $(ALL_HEADERS) test.cpp
+	emcc -std=c++17 test.cpp ../../../../target/wasm32-unknown-emscripten/release/libicu_capi.a -ldl -lpthread -lm -g  -o web-version.html --bind --emrun -sENVIRONMENT=web -sWASM=1 -sEXPORT_ES6=1 -sMODULARIZE=1
+
+node-version.js: ../../../../target/wasm32-unknown-emscripten/release/libicu_capi.a $(ALL_HEADERS) test.cpp
+	emcc -std=c++17 test.cpp ../../../../target/wasm32-unknown-emscripten/release/libicu_capi.a -ldl -lpthread -lm -g  -o node-version.js --bind -sWASM=1 -sENVIRONMENT=node -sWASM_ASYNC_COMPILATION=0 -DNOMAIN
+
+build: web-version.html node-version.js
+
+test: node-version.js
+	exec node ./node-test.js
+
+serve: web-version.html
+	emrun web-version.html
+
+# These make it possible to ensure that the C++ code is up to date with the bindings
+# without needing to set up emsdk. This way `make test-ffi` works without emsdk.
+build-host: a.out
+
+test-host: build-host
+	./a.out
+
+clean:
+	rm -f web-version.html
+	rm -f web-version.wasm
+	rm -f web-version.js
+	rm -f node-version.js
+	rm -f node-version.wasm
+	rm -f ../../../../target/wasm32-unknown-emscripten/release/libicu_capi.a
+	rm -f ../../../../target/debug/libicu_capi.a
+	rm -f a.out

ffi/cpp/examples/fixeddecimal_wasm/README

@@ -0,0 +1,7 @@
+This folder contains a test for calling ICU4X from C++ compiled to WASM (via emscripten).
+
+You need the [Emscripten SDK](https://emscripten.org/docs/getting_started/downloads.html) downloaded and sourced into your environment to run this.
+
+There are two ways to run the test. Firstly, you can call `make test`, which runs `node node-test.js` after building the appropriate WASM files. This runs a CLI test with the fixed decimal example in test.cpp.
+
+The other way is to run `make serve`, which will open a web page running test.cpp in your browser.

ffi/cpp/examples/fixeddecimal_wasm/node-test.js

@@ -0,0 +1,6 @@
+wasm = require("./node-version.js");
+
+const exitCode = wasm.runFixedDecimal();
+if (exitCode !== 0) {
+  throw new Error(`Test failed with exit code ${exitCode}`)
+}

ffi/cpp/examples/fixeddecimal_wasm/package.json

@@ -0,0 +1,6 @@
+{
+    "type": "commonjs",
+    "scripts": {
+        "test": "node node-test.js"
+    }
+}

ffi/cpp/examples/fixeddecimal_wasm/test.cpp

@@ -0,0 +1,65 @@
+// This file is part of ICU4X. For terms of use, please see the file
+// called LICENSE at the top level of the ICU4X source tree
+// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).
+
+#ifdef __EMSCRIPTEN__
+#include <emscripten/bind.h>
+#endif
+
+#include "../../include/ICU4XFixedDecimalFormat.hpp"
+
+#include <iostream>
+
+extern "C" void diplomat_init();
+extern "C" void log_js(char* s) {
+    std::cout<<"LOG: " <<s <<std::endl;
+}
+
+int runFixedDecimal() {
+#ifdef __EMSCRIPTEN__
+    diplomat_init();
+#endif
+    ICU4XLocale locale = ICU4XLocale::create("bn").value();
+    std::cout << "Running test for locale " << locale.tostring().ok().value() << std::endl;
+    ICU4XDataProvider dp = ICU4XDataProvider::create_static().provider.value();
+    ICU4XFixedDecimalFormatOptions opts = {ICU4XFixedDecimalGroupingStrategy::Auto, ICU4XFixedDecimalSignDisplay::Auto};
+    ICU4XFixedDecimalFormat fdf = ICU4XFixedDecimalFormat::try_new(locale, dp, opts).fdf.value();
+
+    ICU4XFixedDecimal decimal = ICU4XFixedDecimal::create(1000007);
+    std::string out = fdf.format(decimal).ok().value();
+    std::cout << "Formatted value is " << out << std::endl;
+    if (out != "১০,০০,০০৭") {
+        std::cout << "Output does not match expected output" << std::endl;
+        return 1;
+    }
+
+    std::string out2;
+    fdf.format_to_writeable(decimal, out2);
+    std::cout << "Formatted writeable value is " << out2 << std::endl;
+    if (out2 != "১০,০০,০০৭") {
+        std::cout << "Output does not match expected output" << std::endl;
+        return 1;
+    }
+
+    decimal.multiply_pow10(2);
+    decimal.negate();
+    out = fdf.format(decimal).ok().value();
+    std::cout << "Value x100 and negated is " << out << std::endl;
+    if (out != "-১০,০০,০০,৭০০") {
+        std::cout << "Output does not match expected output" << std::endl;
+        return 1;
+    }
+    return 0;
+}
+
+#ifdef __EMSCRIPTEN__
+EMSCRIPTEN_BINDINGS(testFixedDecimal) {
+  emscripten::function("runFixedDecimal", &runFixedDecimal);
+}
+#endif
+
+#ifndef NOMAIN
+int main() {
+    return runFixedDecimal();
+}
+#endif

tools/scripts/ffi.toml

@@ -76,6 +76,8 @@ cd ffi/cpp/examples/pluralrules
 exec --fail-on-error make
 cd ../fixeddecimal
 exec --fail-on-error make
+cd ../fixeddecimal_wasm
+exec --fail-on-error make test-host
 '''
 
 [tasks.test-cppdoc]

tools/scripts/wasm.toml

@@ -10,7 +10,7 @@ category = "ICU4X WASM"
 install_crate = { rustup_component_name = "rust-src" }
 toolchain = "nightly-2021-02-28"
 command = "cargo"
-args = ["wasm-build-dev", "--package", "icu_capi"]
+args = ["wasm-build-dev", "--package", "icu_capi", "--example", "icu_capi_cdylib"]
 
 [tasks.wasm-build-release]
 description = "Build WASM FFI into the target directory (release mode)"
@@ -20,7 +20,7 @@ toolchain = "nightly-2021-02-28"
 # We don't care about panics in release mode because most incorrect inputs are handled by result types.
 env = { "RUSTFLAGS" = "-C panic=abort -C opt-level=s" }
 command = "cargo"
-args = ["wasm-build-release", "--package", "icu_capi"]
+args = ["wasm-build-release", "--package", "icu_capi", "--example", "icu_capi_cdylib"]
 
 [tasks.wasm-build-examples]
 description = "Build WASM examples into the target directory"
@@ -47,16 +47,26 @@ args = ["-p", "wasmpkg"]
 description = "Copy the WASM files from dev into wasmpkg"
 category = "ICU4X WASM"
 command = "cp"
-args = ["${CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}/target/wasm32-unknown-unknown/debug/icu_capi.wasm", "wasmpkg/"]
+args = ["${CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}/target/wasm32-unknown-unknown/debug/examples/icu_capi_cdylib.wasm", "wasmpkg/icu_capi.wasm"]
 dependencies = ["wasm-build-dev", "wasm-dir"]
 
 [tasks.wasm-wasm-release]
 description = "Copy the WASM files from release into wasmpkg"
 category = "ICU4X WASM"
 command = "cp"
-args = ["${CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}/target/wasm32-unknown-unknown/release/icu_capi.wasm", "wasmpkg/"]
+args = ["${CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}/target/wasm32-unknown-unknown/release/examples/icu_capi_cdylib.wasm", "wasmpkg/icu_capi.wasm"]
 dependencies = ["wasm-build-release", "wasm-dir"]
 
+[tasks.wasm-cpp-emscripten]
+description = "Run the C++-emscripten test (needs emsdk)"
+category = "ICU4X WASM"
+script_runner = "@duckscript"
+script = '''
+exit_on_error true
+cd ffi/cpp/examples/fixeddecimal_wasm
+exec make test
+'''
+
 [tasks.wasm-wasm-examples]
 description = "Copy the WASM files from examples into wasmpkg"
 category = "ICU4X WASM"
@@ -83,11 +93,15 @@ for json_message in ${json_messages}
     is_compiler_artifact = eq ${json_obj.reason} "compiler-artifact"
     is_example = eq ${json_obj.target.kind[0]} "example"
     if ${is_compiler_artifact} and ${is_example}
-
         # Copy the wasm file to the output directory
         filename = basename ${json_obj.executable}
-        out_path = concat wasmpkg/ ${filename}
-        cp ${json_obj.executable} ${out_path}
+        # We have the icu_capi_cdylib hack example, which is not a real "example"
+        # and won't produce an executable here. We should filter it out.
+        empty = is_empty ${filename}
+        if not ${empty}
+            out_path = concat wasmpkg/ ${filename}
+            cp ${json_obj.executable} ${out_path}
+        end
     end
 end
 '''