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

Fix hiredis_cluster_ssl build on Windows #253

Merged
merged 5 commits into from
Jan 15, 2025
Merged

Conversation

ffratila
Copy link
Contributor

Because hiredis_cluster_ssl does not export any functions, building it on Windows produces just the .dll and not the .lib file (this stackoverflow question describes a similar problem).

Made changes that mirror what is done for hiredis_cluster (adding a .def file with the exports for the WIN32 build)

Copy link
Collaborator

@bjosv bjosv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I have a suggestion and that is to add a CMake property instead of adding the hiredis_cluster_ssl.def files (see below).
We could do the same change for the other lib and remove the hiredis_cluster.def file at the same time. WDYT?

CMakeLists.txt Outdated Show resolved Hide resolved
@zuiderkwast zuiderkwast requested review from bjosv and removed request for bjosv January 15, 2025 09:18
@ffratila
Copy link
Contributor Author

Ended up doing a bit more (enabling the non-const connect cb API on windows) when searching for references to the hiredis_cluster.def file.

Copy link
Collaborator

@bjosv bjosv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@bjosv bjosv merged commit 5ac51f4 into Nordix:master Jan 15, 2025
34 checks passed
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

Successfully merging this pull request may close these issues.

2 participants