C++ setup #246
C++ setup #246
Conversation
cpp/foxglove/CMakeLists.txt
Outdated
| ) | ||
|
|
||
| add_library(foxglove STATIC "${foxglove_srcs}") | ||
| set_property(TARGET foxglove PROPERTY CXX_STANDARD 20) |
There was a problem hiding this comment.
Are we using features which require C++20? I would have expected C++14 would be sufficient.
There was a problem hiding this comment.
2020 was already 5 years ago, do you think it's still too early to move forward with it?
There was a problem hiding this comment.
🤷 my experience is c/c++ moves a lot slower. Would having this make it hard to incorporate an older library into a build, or is it possible to mix and match?
When I was building the C++ runtime of a project a couple years ago, I remember deciding on 14 because it included enough of the modern stuff I wanted, and there weren't major features after that I actually needed.
There was a problem hiding this comment.
(this is as opposed to cmake version, where I found a huge benefit to requiring newer cmake)
There was a problem hiding this comment.
I think 17 is a good compromise
.github/workflows/c_cpp.yml
Outdated
| include: | ||
| - runner: ubuntu-24.04 | ||
| target: aarch64-unknown-linux-gnu | ||
| lib_name: libfoxglove_c.a |
There was a problem hiding this comment.
thoughts on calling it just libfoxglove instead of libfoxglove_c?
There was a problem hiding this comment.
what should the C++ library be named then?
There was a problem hiding this comment.
Grok and chatgpt suggested calling the c library libfoxglove and the c++ library libfoxglove++ or libfoxglove-cpp. They didn't think a suffix for the c variant was needed.
Not sure how I feel about special characters in the file name - it seems reasonably common (libxml++) but safer to avoid?
🤷 I don't feel strongly, feel free to stick to what you have if you prefer it.
There was a problem hiding this comment.
Could do libfoxglove_c and libfoxglove_cpp for consistency?
There was a problem hiding this comment.
This may be somewhat moot as I'm currently not planning to ship prebuilt c++ libraries; people will just integrate it into their own build system. I stuck with the foxglove_c name, but currently the CI artifacts also include the target in the filename so they don't conflict. We can change this later if we clarify our distribution needs.
There was a problem hiding this comment.
I like the idea of just libfoxglove. The fact that it's C is fairly obvious.
There was a problem hiding this comment.
If someone did want to build the c++ version into a shared library and install it, what would they call that?
There was a problem hiding this comment.
libfoxglove_cpp or something like that, but optimize for the happy path
So does this mean a header-only C++ library, that links to the C staticlib/dylib? |
| * # Safety | ||
| * `name` and `host` must be null-terminated strings with valid UTF8. | ||
| */ | ||
| struct foxglove_websocket_server *foxglove_server_start(const char *name, |
There was a problem hiding this comment.
This will quickly get unwieldy, it would be better to pass in a C version of the ServerOptions struct. Don't have to address it now.
Maybe call this foxglove_start_server (verb first)
| /** | ||
| * Stop and shut down a server. | ||
| */ | ||
| void foxglove_server_stop(struct foxglove_websocket_server *server); |
There was a problem hiding this comment.
maybe foxglove_stop_server (verb first?)
There was a problem hiding this comment.
Re verb first, my thinking was as we add more and more types, it’ll be nice to have related methods sorted near each other so that’s why I was thinking [type]_[method]
There was a problem hiding this comment.
Ah, that's a good point. Carry on.
I forget it's like a poor man's namespace in C
Currently it includes cpp sources too, we could make it header-only but I’m not sure there’s much advantage to that? |
I seem to recall header-only libs being easier to setup in my build files, but that was a long time ago. One less step, I'm not sure it matters that much. |
|
🚢 it |
Changelog
None
Docs
None
Description
Some initial setup for C/C++ builds. Overall structure is:
cfolder with a Rust crate that builds astaticlib,cdylib, and uses cbindgen to generate a C headercppfolder with a C++ wrapper around the C library, as well as tests and examplesThe actual functionality is limited to constructing/starting/stopping a server. Will continue to add functionality in future PRs.