Skip to content

Conversation

@kring
Copy link
Member

@kring kring commented Oct 17, 2025

This PR cleans up some weirdness I noticed while investigating the Android build failure in CesiumGS/cesium-unreal#1748.

We had our own custom find script for zlib-ng, even though it has its own. Perhaps we thought we needed it because we had the name of the import library wrong? Or perhaps the built-in one was added in a more recent vcpkg release? Either way, it's not needed anymore, so I removed it and fixed the library name (zlib-ng::zlib-ng -> zlib-ng::zlib).

Other changes:

  • Removed a duplicate find_package for curl. Looks like a merge error.
  • Fixed a typo.
  • Sorted the find_package lines alphabetically.
  • Updated Config.cmake.in with some recent third-party library changes. @timoore I'm not especially well equipped to test this one, so you should take a look and make sure I haven't broken something.

There's a cesium-unreal branch with the same name to demonstrate this still works ok in Unreal.

Copy link
Contributor

@j9liu j9liu left a comment

Choose a reason for hiding this comment

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

Looks good @kring ! Though before I merge, I want to open a similar branch to check that Unity handles these changes well.

EDIT: branch here is passing 🙌

@j9liu
Copy link
Contributor

j9liu commented Oct 20, 2025

Actually @timoore, per the PR description, you should take a look at this as well before merging.

@j9liu j9liu requested a review from timoore October 20, 2025 14:10
@timoore
Copy link
Contributor

timoore commented Oct 20, 2025

@kring and @j9liu , this looks fine to me. It's too bad that library dependencies have to be tracked in both CMakeLists.txt and Config.cmake.in, but that's just the way it is... it's probably premature to have a common source for both.

Copy link
Contributor

@timoore timoore left a comment

Choose a reason for hiding this comment

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

Looks good, a positive change.

@j9liu
Copy link
Contributor

j9liu commented Oct 20, 2025

Thanks @timoore !

@j9liu j9liu merged commit 4f96d33 into main Oct 20, 2025
22 checks passed
@j9liu j9liu deleted the zlib-ng-cleanup branch October 20, 2025 19:46
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