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

C++ requirements are excessive #341

Open
Falmarri opened this issue Dec 14, 2023 · 4 comments
Open

C++ requirements are excessive #341

Falmarri opened this issue Dec 14, 2023 · 4 comments
Assignees
Labels
enhancement New feature or request package: sdk/client Issues affecting the C++ Client SDK waiting for feedback Indicates LaunchDarkly is waiting for customer feedback before issue is closed due to staleness.

Comments

@Falmarri
Copy link

This library is going to be very hard for a lot of enterprise customers to use. C++17 and boost 1.81 is much more modern than I'm required to support. Even Ubuntu jammy only has boost 1.74 at most. And I have to support bionic, which is the latest of 1.55.

@Falmarri Falmarri added bug Something isn't working package: sdk/client Issues affecting the C++ Client SDK labels Dec 14, 2023
@cwaldren-ld cwaldren-ld added enhancement New feature or request and removed bug Something isn't working labels Dec 14, 2023
@cwaldren-ld
Copy link
Contributor

cwaldren-ld commented Dec 14, 2023

Hi @Falmarri , we are aware that a 1 year old Boost is not very old in the C++ world, and therefore may cause some adoption issues. If this is a hard barrier to adoption, please reach out to support for escalation.

The main requirement of Boost 1.81 is Boost.URL, which we use for URL handling.

It may be possible for us to replace this dependency, which could relax the requirement to Boost 1.75 (we make extensive use of the JSON library introduced in 1.75, rather than pulling in another 3rd party library).

May I ask if jammy and bionic are your build environment or your runtime environment (or both?)

@cwaldren-ld cwaldren-ld self-assigned this Dec 14, 2023
@cwaldren-ld cwaldren-ld added the waiting for feedback Indicates LaunchDarkly is waiting for customer feedback before issue is closed due to staleness. label Dec 14, 2023
@Falmarri
Copy link
Author

@cwaldren-ld You're not going to be excited about our system 🤦 . We still have to build for jessie, though it's OK for us to disable features there, but it means build complexity. We build much of our toolchain from source, including cmake and gcc-10. We have bionic and focal as our main runtimes, and we build with the same system and package for each supported distro. If I had to I could build boost from source in our toolchain, however we are still on c++14. So I saw the API requires c++17 which we're still a little bit away from being able to upgrade to.

To be clear I wouldn't necessarily assume support for any of this ancient tooling, especially for a static library. I just know we're not the worst in terms of being on really old toolchains. Just thought I would give you my experience taking a couple hours seeing if I could get this running, as I already have a solution for what we want to use it for via env variables and it's not really worth the time if it takes more than a day or so (we use launch darkly elsewhere in our ecosystem, fwiw).

If you wanted to make it easy for us, supporting back to c++14, and a compile time option to use https://github.com/nlohmann/json instead of boost for json as we use that elsewhere as well.

I also notice that when i cmake --install, you don't install a .cmake config that lets me run find_package. It's possible I missed something, but I think your examples assume this is being built within the consume project's cmake file rather than separately installed.

I appreciate the response with the specific reasons though. Modernizing our system is definitely something we're working towards.

@cwaldren-ld
Copy link
Contributor

cwaldren-ld commented Dec 18, 2023

Hi @Falmarri , I appreciate how difficult it can be to maintain mature codebases like yours, especially when trying to pull in a library that could help gradually modernize it.

Thank you for clarifying your requirements.

For C++14 and nlohmann/json, the main issue here is that we make extensive use of Boost.JSON and also of C++17 features std::optional, std::variant, if constexpr (all those variants!) and some structured bindings.

While it would be possible for us to refactor the codebase to C++14, we'd need to receive broader feedback to prioritize that.

On a more useful note: we have put in the time to support a stable C API (for both the client-side and server-side libraries.)

If you're able to cross-compile in an environment that supports C++17 + recent boost, then your main project should only need a C99 compiler to interface with our SDK.

About the .cmake config - you're 100% correct and that's an omission that we need to fix. We currently don't support the find_package workflow.

@cwaldren-ld
Copy link
Contributor

Just an update, the latest release of the SDKs contains preliminary .cmake package config, supporting the find_package workflow.

Currently, the package contains both the server and client SDKs but this is subject to change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request package: sdk/client Issues affecting the C++ Client SDK waiting for feedback Indicates LaunchDarkly is waiting for customer feedback before issue is closed due to staleness.
Projects
None yet
Development

No branches or pull requests

2 participants