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 Windows CMake builds with shared zlib #8846

Merged
merged 1 commit into from Oct 21, 2020

Conversation

nyanpasu64
Copy link
Contributor

@nyanpasu64 nyanpasu64 commented Jun 5, 2020

On Linux, if shared zlib is present, zlib.h is always available and -lz links to zlib, even if you don't run find_package(ZLIB).

For some reason I have zlib installed on Windows (possibly from vcpkg), so find_package(ZLIB) succeeds and ZLIB_FOUND is true. When Dolphin uses shared zlib on Windows, the problem is that zlib.h is not in the default include path, and the CMake target is called ZLIB::ZLIB and there's neither a target nor a library called z.

However, both find_package(ZLIB) and add_subdirectory(Externals/zlib) create a target called ZLIB::ZLIB, so I'll switch to that instead. Hopefully this change doesn't break anyone's build.

CMakeLists.txt Outdated Show resolved Hide resolved
On Linux, if shared zlib is present, zlib.h is always available and -lz
links to zlib, even if you don't run find_package(ZLIB).

For some reason I have zlib installed on Windows (possibly from vcpkg),
so find_package(ZLIB) succeeds and ZLIB_FOUND is true.
When Dolphin uses shared zlib on Windows, the problem is that zlib.h
is not in the default include path, and the CMake target is called
ZLIB::ZLIB and there's neither a target nor a library called z.

However, both find_package(ZLIB) and add_subdirectory(Externals/zlib)
create a target called ZLIB::ZLIB, so I'll switch to that instead.
Hopefully this change doesn't break anyone's build.
@nyanpasu64
Copy link
Contributor Author

Just rebased to master and removed the unnecessary global ZLIB::ZLIB. Building Dolphin works on my branch and fail on master.

I haven't checked if there are any other libraries that can break this way.

Copy link
Member

@leoetlino leoetlino left a comment

Choose a reason for hiding this comment

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

sgtm

@leoetlino leoetlino merged commit 08f9ed0 into dolphin-emu:master Oct 21, 2020
10 checks passed
@nyanpasu64 nyanpasu64 deleted the fix-cmake-zlib branch October 22, 2020 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants