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

WIP: Add support to use system libraries #803

Merged
merged 4 commits into from
Mar 26, 2024

Conversation

guillemj
Copy link
Contributor

These patches add support to optionally use system libraries instead of the vendored (or embedded) projects. This was requested in #493. I'm sending what I've got for now to avoid duplicated work, even though this does not seem to be passing the tests yet. I'll try to check which commit makes it fail next week. I've left out for now the lua vendored code, because I cannot test that with what is currently packaged in Debian as there are at least two lua modules missing there. My current WIP for that is in guillemj@5bbff3c.

This is something that is required by most major distributions as a matter of policy, and something that will need to be carried over when packaging for Debian (or Fedora I guess), see https://bugs.debian.org/1067413.

Add a new USE_SYSTEM_CONCURRENTQUEUE make variable to select whether
to include the system concurrentqueue headers.
Add a new USE_SYSTEM_JEMALLOC make variable to select whether to link
against the system libjemalloc.
Add a new USE_SYSTEM_ROCKSDB make variable to select whether to link
against the system librocksdb.
…_ssl

Add a new USE_SYSTEM_HIREDIS make variable to select whether to link
against the system libhiredis (and libhiredis_ssl if BUILD_TLS is
enabled).

Move the sdscompat.h header from the vendored hiredis directory to src/,
as this file is not and has never been part of the upstream hiredis
project, it got added in commit bffbbea
in redis itself.
@JohnSully
Copy link
Collaborator

Looks reasonable. CI failure is unrelated (flaky test)

@JohnSully JohnSully merged commit 0731a05 into Snapchat:main Mar 26, 2024
3 of 4 checks passed
@guillemj
Copy link
Contributor Author

Thanks for merging! Unfortunately the hiredis change was not entirely correct, which I detected after bisecting for the test failures I mentioned in my initial message. :) I'm sending a new MR to fix that up.

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