wasmtime / PR #10582 c-api: Bring wasmtime-cpp into this ... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #10582 c-api: Bring wasmtime-cpp into this ...

Wasmtime GitHub notifications bot (Apr 15 2025 at 00:09):

MangoPeachGrape opened PR #10582 from MangoPeachGrape:c-api/cpp to bytecodealliance:main:

See discussion in #10566 for more context.

Folloving files have been moved without edits:

wasmtime-cpp/include/wasmtime.hh -> /crates/c-api/include/

wasmtime-cpp/tests/* -> /crates/c-api/tests/

wasmtime-cpp/examples/* -> /examples/

The CMakeLists.txt content is copied to /crates/c-api/CMakeLists.txt with slight adjustments. I haven't fully gone through and checked if everything from the old CMake file still makes sense.

The tests inside /crates/c-api/tests/ seems to run when examples ("Test C-API" from CI) is executed.

Added a CMake option to disable tests, feel free to let me know if this is not desired.

Wasmtime GitHub notifications bot (Apr 15 2025 at 00:09):

MangoPeachGrape requested fitzgen for a review on PR #10582.

Wasmtime GitHub notifications bot (Apr 15 2025 at 00:09):

MangoPeachGrape requested wasmtime-core-reviewers for a review on PR #10582.

Wasmtime GitHub notifications bot (Apr 15 2025 at 00:11):

MangoPeachGrape edited PR #10582:

See discussion in #10566 for more context.

Folloving files have been moved without edits:

wasmtime-cpp/include/wasmtime.hh -> /crates/c-api/include/

wasmtime-cpp/tests/* -> /crates/c-api/tests/

wasmtime-cpp/examples/* -> /examples/

The CMakeLists.txt content is copied to /crates/c-api/CMakeLists.txt with slight adjustments. I haven't fully gone through and checked if everything from the old CMake file still makes sense.

The tests inside /crates/c-api/tests/ seem to run when examples ("Test C-API" from CI) is executed.

Added a CMake option to disable tests, feel free to let me know if this is not desired.

Wasmtime GitHub notifications bot (Apr 15 2025 at 15:16):

alexcrichton submitted PR review:

Thanks for this! This looks pretty good to me, but I'd also want to test some more integration-y things before landing. To that end can you try a few things:

Add a commit to this PR with "prtest:full" somewhere in the commit message so we can see the full CI run

With a full run I want to double-check that the header file is present in the installation artifacts

Mind updating crates/c-api/README.md with a mention that the C++ API is present as well? I can also do this as a follow-up if you'd prefer.

With a full run I want to double-check that the docs here are present in the c-api docs.

Wasmtime GitHub notifications bot (Apr 15 2025 at 15:16):

alexcrichton created PR review comment:

Could this default to OFF but we be careful to pass it as ON in CI? Agreed though that this is good to have, thanks for adding this!

Wasmtime GitHub notifications bot (Apr 15 2025 at 15:16):

alexcrichton commented on PR #10582:

Also, after merging, I'll take on the bits of archiving the old repo.

Wasmtime GitHub notifications bot (Apr 15 2025 at 19:58):

MangoPeachGrape requested wasmtime-default-reviewers for a review on PR #10582.

Wasmtime GitHub notifications bot (Apr 15 2025 at 19:58):

MangoPeachGrape updated PR #10582.

Wasmtime GitHub notifications bot (Apr 15 2025 at 20:10):

MangoPeachGrape commented on PR #10582:

CI doesn't seem to have C++ compiler? Should a C++ compiler be a requirement, even when we don't build any C++ code?

Wasmtime GitHub notifications bot (Apr 15 2025 at 20:17):

alexcrichton commented on PR #10582:

Could this be updated to only require a C++ compiler for tests? For a normal build it shouldn't be required I think?

Wasmtime GitHub notifications bot (Apr 15 2025 at 20:41):

MangoPeachGrape updated PR #10582.

Wasmtime GitHub notifications bot (Apr 15 2025 at 21:41):

alexcrichton commented on PR #10582:

Ok nice looks like everything's integrated well, I can see the C++ tests running and I see wasmtime.hh in the distribution artifacts as well. Can you double-check the docs though? I don't see the doxygen docs for the C++ API in the gh-pages artifact, just the C API. I suspect we need to tweak doxygen.conf a bit?

Wasmtime GitHub notifications bot (Apr 15 2025 at 22:28):

MangoPeachGrape updated PR #10582.

Wasmtime GitHub notifications bot (Apr 15 2025 at 22:31):

MangoPeachGrape commented on PR #10582:

Would it be preferred to only silence the undocumented warnings from wasm.hh instead of disabling it all together?

Wasmtime GitHub notifications bot (Apr 16 2025 at 14:58):

alexcrichton commented on PR #10582:

If possible, yeah, although I'm not sure if it's easy to configure doxygen as such.

Wasmtime GitHub notifications bot (Apr 16 2025 at 16:03):

MangoPeachGrape commented on PR #10582:

I was thinking of EXCLUDE = wasm.hh, but now I realize that it obviously removes it from the output as well. What's better, exclude wasm.hh and enable the warning, or keep it like it is now?

Wasmtime GitHub notifications bot (Apr 16 2025 at 16:15):

alexcrichton commented on PR #10582:

Excluding in this case I think may actually be ok because while it works I think we'll still want to steer users towards wasmtime.hh. It's not like the C API where some utilities come from the wasm.h API and others from wasmtime.h

Wasmtime GitHub notifications bot (Apr 16 2025 at 17:43):

MangoPeachGrape updated PR #10582.

Wasmtime GitHub notifications bot (Apr 16 2025 at 18:28):

alexcrichton submitted PR review:

Nice docs are looking great. Thank you again for working on this!

Wasmtime GitHub notifications bot (Apr 16 2025 at 18:59):

alexcrichton merged PR #10582.

Last updated: Dec 06 2025 at 06:05 UTC