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

Add cpp-to-wasm test #968

Merged
merged 6 commits into from
Aug 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 16 additions & 2 deletions .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Manishearth marked this conversation as resolved.
Show resolved Hide resolved
./emsdk activate latest
- name: Get Diplomat version
id: diplomat-version
run: |
Expand Down Expand Up @@ -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:
Expand Down
12 changes: 6 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion Makefile.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
21 changes: 19 additions & 2 deletions ffi/capi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Copy link
Member Author

Choose a reason for hiding this comment

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

dylib had to be removed because it doesn't work for emscripten (without what seems to be a large amount of extra effort),

we're not using cdylibs anyway, and it's still possible to compile icu_capi to a cdylib, just not in the emscripten context, and unfortunately rust-lang/cargo#4881 doesn't exist.

path = "src/lib.rs"

[features]
Expand Down Expand Up @@ -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"]
10 changes: 10 additions & 0 deletions ffi/capi/src/crate_type_hack.rs
Original file line number Diff line number Diff line change
@@ -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;
2 changes: 1 addition & 1 deletion ffi/cpp/examples/fixeddecimal/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ a.out: ../../../../target/debug/libicu_capi.a $(ALL_HEADERS) test.cpp
build: a.out

test: build
./a.out
./a.out
1 change: 0 additions & 1 deletion ffi/cpp/examples/fixeddecimal/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
8 changes: 8 additions & 0 deletions ffi/cpp/examples/fixeddecimal_wasm/.gitignore
Original file line number Diff line number Diff line change
@@ -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
53 changes: 53 additions & 0 deletions ffi/cpp/examples/fixeddecimal_wasm/Makefile
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions ffi/cpp/examples/fixeddecimal_wasm/README
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 6 additions & 0 deletions ffi/cpp/examples/fixeddecimal_wasm/node-test.js
Original file line number Diff line number Diff line change
@@ -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}`)
}
6 changes: 6 additions & 0 deletions ffi/cpp/examples/fixeddecimal_wasm/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "commonjs",
"scripts": {
"test": "node node-test.js"
}
}
65 changes: 65 additions & 0 deletions ffi/cpp/examples/fixeddecimal_wasm/test.cpp
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions tools/scripts/ffi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
28 changes: 21 additions & 7 deletions tools/scripts/wasm.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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
'''
Expand Down