-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
libpng: fix cross-build to iOS arm + cache cmake + modernize #6285
libpng: fix cross-build to iOS arm + cache cmake + modernize #6285
Conversation
when NEON is enable for iOS builds, there are missing symbols in generated lib
we don't package this file in CCI
recipes/libpng/all/conanfile.py
Outdated
cmake.definitions["CMAKE_SYSTEM_PROCESSOR"] = "aarch64" | ||
cmake.configure() | ||
return cmake | ||
self._cmake.definitions["PNG_BUILD_ZLIB"] = "ON" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean a vendored zlib
is built by libpng when building for emscripten?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like no zlib is used at all:
https://github.com/glennrp/libpng/blob/a37d4836519517bdce6cb9d956092321eca3e73b/CMakeLists.txt#L38-L45
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, maybe there was an issue with Emscripten in conan long time ago.
It's the same thing for M_LIBRARY, it's useless to force empty value, it is overridden by upstream anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed in 2ebb94f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a pr on cci for emscripten at #6163
Once it gets merged, some recipes should be tested with it.
All green in build 4 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All holes are covered now. 😝 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Declined ✖️
self._cmake.definitions["PNG_DEBUG"] = self.settings.build_type == "Debug" | ||
if tools.is_apple_os(self.settings.os): | ||
if "arm" in self.settings.arch: | ||
# FIXME: Neon should work on iOS (but currently if leads to undefined symbols), see if autotools build is better |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# FIXME: Neon should work on iOS (but currently if leads to undefined symbols), see if autotools build is better | |
# FIXME: Neon should work on iOS (but currently it leads to undefined symbols), see if autotools build is better |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I saw that after pushing the last commit, but was too lazy to fix the typo 😩
…dernize * fix undefined symbols in iOS arm builds when NEON is enable for iOS builds, there are missing symbols in generated lib * cleanup imports * no os.rename * reorder methods * conventions * cache cmake * declare patch files in conandata.yml * use find_package() in test_package * cleanup * use pure C test package * explicit pkg_config name * remove useless patch in .pc file we don't package this file in CCI * consistent quotes * empty string api_prefix default value * debug output for Debug build_type only * use boolean for CMake options * add comment about iOS build and Neon * remove Emscripten specific stuff
Specify library name and version: lib/1.0
closes #2366
conan-center hook activated.