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

Set LIBDIR for remaining cmake-built libraries. #538

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

freakboy3742
Copy link
Contributor

@freakboy3742 freakboy3742 commented Oct 28, 2024

#531 removed the CMAKE_INSTALL_LIBDIR configuration from libjpeg-turbo on the basis that it appeared to be redundant.

#537 restored that flag, after it became clear that it isn't redundant - on any platform that uses the lib64 convention, CMAKE_INSTALL_LIBDIR is needed

In the discussion on #537, I speculated that it might be advisable to add the same flag to the other cmake libraries. It turns out I was right - if you set CMAKE_INSTALL_LIB_NAME and don't set CMAKE_INSTALL_LIBDIR, you end up installing the library into lib64, requiring a manual extra step to move the library to the location that matches the lib name; plus additional complications when the linked library references other libraries in lib64. This became apparent during testing on python-pillow/pillow#8497, because openjpeg libraries weren't lining up without additional post-build steps.

This adds the CMAKE_INSTALL_LIBDIR configuration to openjpeg and blosc.

@mattip
Copy link
Collaborator

mattip commented Oct 28, 2024

@radarhere it seems you are both testing this with Pillow but getting different experiences. I am not a CMake specialist, I will go with "whatever works". Maybe if one platform needs the flag and another doesn't, we can hide it behind a if stanza?

@radarhere
Copy link
Collaborator

radarhere commented Oct 28, 2024

Oh, I've tested this PR with Pillow, and from what I've seen, if this makes sense to others then I raise no objections.

@freakboy3742
Copy link
Contributor Author

I'm not sure we're getting different results - I'm
compensating for an additional change that I presume @radarhere isn't making.

Pillow's built config currently has a hack copying openjpeg binaries from lib64 to lib, which is exactly what this change prevents. AFACT, with this change, the hacks can be removed, because the /lib folder is used consistently in both the library name and the location where the library is output.

@radarhere
Copy link
Collaborator

Pillow's built config currently has a hack copying openjpeg binaries from lib64 to lib, which is exactly what this change prevents. AFACT, with this change, the hacks can be removed

Yep, I agree with this.

@mattip mattip merged commit 9a9d127 into multi-build:devel Oct 28, 2024
4 checks passed
@mattip
Copy link
Collaborator

mattip commented Oct 28, 2024

Thanks all for the discussion and the work. In it goes.

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.

3 participants