Skip to content

Makefile: correctly handle version suffixes on macOS #1257

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

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

ZhongRuoyu
Copy link
Contributor

@ZhongRuoyu ZhongRuoyu commented Apr 8, 2024

On macOS, the version suffix of a shared library comes before the .dylib suffix, which is different from the convention on other systems. Currently, the Makefile only overrides DYLIB_MINOR_NAME for macOS, but does not handle the other similar variables, namely DYLIB_MAJOR_NAME, SSL_DYLIB_MINOR_NAME, and SSL_DYLIB_MAJOR_NAME.

This PR fixes the issue by overriding all these variables for macOS.

On macOS, the version suffix of a shared library comes before the
`.dylib` suffix, which is different from the convention on other
systems. Currently, the Makefile only overrides `DYLIB_MINOR_NAME` for
macOS, but does not handle the other similar variables, namely
`DYLIB_MAJOR_NAME`, `SSL_DYLIB_MINOR_NAME`, and `SSL_DYLIB_MAJOR_NAME`.

This commit fixes the issue by overriding all these variables for macOS.

Signed-off-by: Ruoyu Zhong <zhongruoyu@outlook.com>
@ZhongRuoyu ZhongRuoyu force-pushed the macos-dylib-suffix branch from cba081a to f6dc8fc Compare April 8, 2024 16:44
@ZhongRuoyu
Copy link
Contributor Author

Gentle ping @michael-grunder -- mind having a look? It'd be nice to have this shipped, and we can apply the fix to Homebrew's hiredis.

@michael-grunder michael-grunder merged commit 77bcc73 into redis:master Feb 10, 2025
@michael-grunder
Copy link
Collaborator

As far as I can tell this change is correct. Thanks!

@ZhongRuoyu
Copy link
Contributor Author

Great, thank you @michael-grunder!

@ZhongRuoyu ZhongRuoyu deleted the macos-dylib-suffix branch February 11, 2025 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants