bytecodealliance / wasmtime

A fast and secure runtime for WebAssembly
https://wasmtime.dev/
Apache License 2.0
14.82k stars 1.24k forks source link

`#include <wasmtime/conf.h>` fails since `conf.h` is not generated #8890

Open dundargoc opened 1 week ago

dundargoc commented 1 week ago

When we try to bump to wasmtime v22.0.0 in https://github.com/tree-sitter/tree-sitter/pull/3428 we get the following error:

cargo:warning=/home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wasmtime-c-api-impl-22.0.0/include/wasi.h:12:10: fatal error: wasmtime/conf.h: No such file or directory
  cargo:warning=   12 | #include <wasmtime/conf.h>
  cargo:warning=      |          ^~~~~~~~~~~~~~~~~

When I checked this locally it seems that conf.h is not being generated when building the c-api via a crate as I could not find it in cargo:warning=/home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wasmtime-c-api-impl-22.0.0/include/, which is where I assumed it would be as that's where the headers are looking for it.

clason commented 3 days ago

Related: https://github.com/Homebrew/homebrew-core/pull/175234

alexcrichton commented 2 hours ago

This changed in https://github.com/bytecodealliance/wasmtime/pull/8642 where conf.h is only generated as part of the cmake-based build process at this time. If using via the Rust build script that'll need to have support as well for generating conf.h, probably in the build directory, and generating another cargo build directive to add another include path pointing to the generated file

clason commented 2 hours ago

Just to be clear: These changes need to happen here in wasmtime-c-api, right?

alexcrichton commented 1 hour ago

I think there may be multiple issues in play here, so I'm not certain if a single fix in a single location will fix both issues here. One issue is that this line is no longer sufficient for the includes of Wasmtime because the conf.h file isn't generated anywhere as part of that build script. I think that's what tree-sitter is using and I think that's what tree-sitter is running into. Fixing that would involve updating the build script to do what CMake is doing which is to read conf.h.in and then postprocess it to generate a new conf.h. A new directive would then need to be printed to point to the location of this generated header file.

As for the Homebrew PR I don't know what's going on there. That looks like a possibly buggy installation of the C API because it looks like it was built from source and installed but conf.h is missing. That may indicate that CMake wasn't used to build/install but the PR looks like it was updated to use CMake. Given all that I don't know what's happening.

clason commented 1 hour ago

One issue is that this line is no longer sufficient for the includes of Wasmtime because the conf.h file isn't generated anywhere as part of that build script. I think that's what tree-sitter is using and I think that's what tree-sitter is running into

Yes. The main question is whether that is considered a regression to be fixed, or whether the official policy is now "use the cmake crate to build the C API" (or hack it yourself in your own build.rs).

As for the Homebrew PR I don't know what's going on there.

Sorry, that was just meant to show that the effect is not limited to one consumer: Homebrew had the same issue of no longer being able to build wasmtime-c-api via (pure) cargo and had to switch to a CMake based build. (Which is less intrusive there since it's a standalone build that does not need to interface with a larger Rust project.)

alexcrichton commented 14 minutes ago

If CMake can be used that'd be best yeah, primarily because that's what we're testing in CI and it's what's used to produce the artifacts. If that's not feasible this is minor enough I think it's ok to add custom logic to the build script too.