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 pkg-config files being broken on MSVC-built static libraries #2754

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Cacodemon345
Copy link

When libjxl is built as a static library with MSVC, the pkg-config files are not configured properly to account for statically-built library names, causing link failures.

This PR makes those files account for statically-built libraries that are built by MSVC.

@google-cla
Copy link

google-cla bot commented Aug 28, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@mo271 mo271 added the CI:full Label to attach to a PR to run the full CI workflow and not just the regular PR workflows label Aug 28, 2023
@mo271 mo271 added the building/portability Platform-specific issues, build issues label Aug 28, 2023
lib/jxl/libjxl.pc.in Outdated Show resolved Hide resolved
@HappySeaFox
Copy link
Contributor

I propose to rename jxl-static back to jxl on all platforms. This will simplify searching for libjxl in CMake with find_library and possibly other non-pkg-config environments.

@kmilos
Copy link

kmilos commented Oct 7, 2023

I propose to rename jxl-static back to jxl on all platforms. This will simplify searching for libjxl in CMake with find_library and possibly other non-pkg-config environments.

Sadly does not work well for MSVC builds where the shared import library and static one cannot coexist with the same base name.

@HappySeaFox
Copy link
Contributor

HappySeaFox commented Oct 7, 2023

Sadly does not work well for MSVC builds where the shared import library and static one cannot coexist with the same base name.

This is a build system-related problem. One of the possible solutions could be putting shared and static libs into different folders.

Another possible solution is to provide CMake configs so find_package(jxl) always finds the correct library. I would vote for this solution even more than for the renaming.

P.S. I can name just a couple of libs I'm aware of that have different names for shared and static versions. It's always pain for a build system to deal with such libs as they frequently don't provide CMake configs and need runtime patching.

UPDATE #2851

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
building/portability Platform-specific issues, build issues CI:full Label to attach to a PR to run the full CI workflow and not just the regular PR workflows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants