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

[redis-plus-plus] Support async interface with features #25248

Closed
elmanelman opened this issue Jun 15, 2022 · 7 comments · Fixed by #25272
Closed

[redis-plus-plus] Support async interface with features #25248

elmanelman opened this issue Jun 15, 2022 · 7 comments · Fixed by #25272
Assignees
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist

Comments

@elmanelman
Copy link

Is your feature request related to a problem? Please describe.
For now it isn't possible to choose whether to install and build async part of redis-plus-plus with vcpkg. These features are enabled with cmake variables: REDIS_PLUS_PLUS_BUILD_ASYNC=libuv and, optionally, REDIS_PLUS_PLUS_ASYNC_FUTURE=boost, if you want to use boost::future instead of std::future.

Proposed solution
Add library features that, being selected, will set ...BUILD_ASYNC and ...ASYNC_FUTURE variables.

For instance, redis-plus-plus[async] (or [async-std]) could set ...BUILD_ASYNC=libuv only, and redis-plus-plus[async-boost] could set both ...BUILD_ASYNC=libuv and ...ASYNC_FUTURE=boost.

Describe alternatives you've considered
I couldn't find any other concise ways to solve this.

@elmanelman elmanelman changed the title [redis_plus_plus] Support async interface with features [redis-plus-plus] Support async interface with features Jun 15, 2022
@JackBoosY JackBoosY added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Jun 16, 2022
@JackBoosY
Copy link
Contributor

According to our policy, we refuse to accept adding conflicting features. So do you need std or boost?

@elmanelman
Copy link
Author

elmanelman commented Jun 16, 2022

It would be nice to set ...BUILD_ASYNC and ...ASYNC_FUTURE separately, as library users may want to choose which futures to use and, looking ahead, choose event loop lib.

I suggest adding two features (meanwhile, I will be glad of better solutions):

  • async-libuv (or async-event-loop-libuv) that sets REDIS_PLUS_PLUS_BUILD_ASYNC=libuv
  • async-futures-std that sets REDIS_PLUS_PLUS_ASYNC_FUTURE=std
  • async-futures-boost that sets REDIS_PLUS_PLUS_ASYNC_FUTURE=boost

So, you could pick redis-plus-plus[async-libuv,async-futures-std] for libuv event loop & std futures, and redis-plus-plus[async-libuv,async-futures-boost] for libuv & boost futures.

@JackBoosY
Copy link
Contributor

Which boost components is required by async-futures-boost?

@JackBoosY
Copy link
Contributor

@elmanelman According to our doc, we can only pick one feature, std or boost.

@elmanelman
Copy link
Author

Hmm, I got it and think we can leave async & async-std for now, as you implemented in #25272.

But what is the right way to give a choice between std and boost futures?

@JackBoosY
Copy link
Contributor

If you need to use async-futures-boost, you may need to modify vcpkg.json(add feature and add dependency items boost components) and portfile.cmake(check the feature whether it's enabled and set -DREDIS_PLUS_PLUS_ASYNC_FUTURE=boost to vcpkg_configure_make to enable it) using overlay port feature.
See https://github.com/microsoft/vcpkg/blob/master/docs/specifications/ports-overlay.md.

@JackBoosY JackBoosY removed the requires:more-information This Issue requires more information to solve label Jun 20, 2022
@elmanelman
Copy link
Author

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants