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

[QUESTION] Conan C++ support #1

Closed
redradist opened this issue Jun 23, 2021 · 31 comments
Closed

[QUESTION] Conan C++ support #1

redradist opened this issue Jun 23, 2021 · 31 comments

Comments

@redradist
Copy link
Contributor

Do you consider supporting Conan C++ as package manager for this wrapper ?

@alexcrichton
Copy link
Member

I think that'd be fine to add! For me at least this is the first piece of C++ I've ever published as a library, so I'm pretty new to this. I've never worked with Conan myself, but would you be up for sending a PR? Ideally it'd include CI to test the build configuration too

@redradist
Copy link
Contributor Author

It would require to write pr into conan-center-index

@alexcrichton
Copy link
Member

Ah ok, well if I can help out with that let me know! I unfortunately don't know enough about Conan to make a PR myself though.

@redradist
Copy link
Contributor Author

I know how to do it ...

Do you need a help with this wrapper ?
If I would have some time I can help with it I know C++ and Rust, but unfortunately I do not understand current status of this wrapper ... I mean how much is finished and so on

@redradist
Copy link
Contributor Author

redradist commented Jun 23, 2021

Also seems like conan is not needed for current implementation because all wrapper functionallity is header only ...
Do you plan to make separate cpp files for better maintainability ?

@alexcrichton
Copy link
Member

Oh no the wrapper is ready to go and can be used whenever. It's header-only because the actual implementation comes from libwasmtime.so which is the C API of wasmtime. The C++ is currently built on the C API and doesn't need any extra accompanying *.cpp files.

@alexcrichton
Copy link
Member

er sorry didn't mean to close..

I meant to say though that I don't know if Conan supports header-only libraries? Or if it supports a header-only library with a Rust C API as a backend?

@alexcrichton alexcrichton reopened this Jun 23, 2021
@redradist
Copy link
Contributor Author

er sorry didn't mean to close..

I meant to say though that I don't know if Conan supports header-only libraries? Or if it supports a header-only library with a Rust C API as a backend?

@alexcrichton Yes, conan supports header only library, but during installation it also verify system dependencies, compiler versions and so on

Actually, conan for C++ (with conanfile.txt) is like cargo for Rust (with Cargo.toml)

@redradist
Copy link
Contributor Author

@alexcrichton

Oh no the wrapper is ready to go and can be used whenever. It's header-only because the actual implementation comes from libwasmtime.so which is the C API of wasmtime. The C++ is currently built on the C API and doesn't need any extra accompanying *.cpp files.

Does it mean that wasmtime.hh was just generated for wasmtime-cpp ?

@alexcrichton
Copy link
Member

The wasmtime.hh header was hand-written and isn't generated (intentionally). It depends on libwasmtime.so, the Rust-based C API of Wasmtime, but otherwise this repository houses the wasmtime.hh to be separately tested from the main wasmtime repo itself.

@redradist
Copy link
Contributor Author

It think I would have some time on the week to create receipt for wasmtime-cpp, but it could require small changes in CMakeLists.txt

@alexcrichton
Copy link
Member

Oh that's fine! This was the first time I've ever written a CMakeLists.txt so changes are more than welcome

@redradist
Copy link
Contributor Author

@alexcrichton What kind of features from C++20 do you use ?
Currently no major compiler supports C++20 completely ...

@alexcrichton
Copy link
Member

I believe it's only the std::span type, which at least is supported by the compilers tested on CI here

@redradist
Copy link
Contributor Author

redradist commented Jun 29, 2021

@alexcrichton Here is initial receipt for wasmtime in conan-center:
conan-io/conan-center-index#6083

Then I will added separate package wasmtime-cpp that will have dependency on wasmtime

@redradist
Copy link
Contributor Author

@alexcrichton
I expect that soon conan package for wasmtime will be merged ...
For working on wasmtime-cpp package we need to have tags for releases

Do you consider adding tags for versions ?

@alexcrichton
Copy link
Member

Sure! I just pushed up a v0.28.0 tag

@redradist
Copy link
Contributor Author

@alexcrichton The basic wasmtime package almost merged ...
Do you plan have in sync versions of wasmtime-cpp and wasmtime ?

I need it to know for making next package for wasmtime-cpp ...
If yes, then we need extra tag for release 0.29 ?

@redradist
Copy link
Contributor Author

redradist commented Sep 6, 2021

@alexcrichton I have create PR in wasmtime for C Conan package bytecodealliance/wasmtime#3307
Soon I will create C++ Conan package ...

@redradist
Copy link
Contributor Author

@alexcrichton I've started working on wasmtime-cpp packaging conan-io/conan-center-index#7188

@alexcrichton
Copy link
Member

Awesome, thanks! Let me know if you need anything from my end.

The plan is indeed to have the wasmtime-cpp and wasmtime versions sync'd. I don't know really where to specify a version in this repository beyond tags, but I can create a 0.29 tag if so necessary.

@redradist
Copy link
Contributor Author

redradist commented Sep 12, 2021

@alexcrichton Seems like you added wasmtime_memorytype_minimum, wasmtime_memorytype_is64, wasmtime_memorytype_new functions those are missed in release of wasmtime https://github.com/bytecodealliance/wasmtime/releases/tag/v0.29.0

It means you need to change commit for tag v0.29.0 otherwise dependency do not works

@alexcrichton
Copy link
Member

Oops sorry yeah I'll try to find a better 0.29.0 tag later this week.

@redradist
Copy link
Contributor Author

@alexcrichton There are only two possible commits Fix including wasmtime.hh in multiple CGUs (#9) 3ee46ce and Instead of converting wasmtime::Span added ifdef with type alias for … 69829db

@alexcrichton
Copy link
Member

Ok I've updated the v0.29.0 tag to be the same as the v0.28.0 tag since that's the latest commit that works.

@redradist
Copy link
Contributor Author

@alexcrichton Thanks a lot !!
Receipt was updated, I expect it soon to be merged

@redradist
Copy link
Contributor Author

redradist commented Sep 17, 2021

@alexcrichton PR merged conan-io/conan-center-index#7188

@redradist
Copy link
Contributor Author

@alexcrichton Now when you will have new release for wasmtime just create PR to https://github.com/conan-io/conan-center-index and add new version to receipt https://github.com/conan-io/conan-center-index/tree/master/recipes/wasmtime in config.yml and conandata.yml

The same for wasmtime-cpp https://github.com/conan-io/conan-center-index/tree/master/recipes/wasmtime-cpp

@alexcrichton
Copy link
Member

Ok great! I'll probably lean on you to update the conan packages, but would you like me to notify you on releases/tags?

@redradist
Copy link
Contributor Author

Ok great! I'll probably lean on you to update the conan packages, but would you like me to notify you on releases/tags?

Yeah, would be nice ... I will provide PR and you will learn how to update package version by yourself

@alexcrichton
Copy link
Member

Ok, I'm going to close this because I think this is done now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants