-
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] set CMAKE_SYSTEM_PROCESSOR for M1 #6187
Conversation
Signed-off-by: SSE4 <tomskside@gmail.com>
This comment has been minimized.
This comment has been minimized.
All green in build 2 (
|
if self.settings.os == "Macos" and self.settings.arch == "armv8": | ||
cmake.definitions["CMAKE_SYSTEM_PROCESSOR"] = "aarch64" |
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.
It fails also for iOS, but I don't know which CMAKE_SYSTEM_PROCESSOR to set.
ok: #2366, maybe this recipe should use autotools for non-Windows platform
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.
Well, in fact, this is only required when cross-building (because we are not using toolchain file)
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.
iOS has different architectures (armv7, armv8 and their variants), so CMAKE_SYSTEM_PROCESSOR will have different values
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.
Sure, my suggestion is to force this value only for the cross-building scenario, otherwise, in a native build I would expect it to work out of the box. wdyt?
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.
as I understand, in native builds CMAKE_SYSTEM_PROCESSOR
defaults to CMAKE_HOST_SYSTEM_PROCESSOR
which should be auto-detected to aarch64
by cmake itself? still, I don't have ARM M1 laptop to check it, so no idea if it works without explicit definition...
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.
It works for iOS:
if tools.is_apple_os(self.settings.os):
if "arm" in self.settings.arch:
cmake.definitions["PNG_ARM_NEON"] = "on" if self.settings.os == "Macos" else "off"
if self.settings.arch == "armv8":
cmake.definitions["CMAKE_SYSTEM_PROCESSOR"] = "aarch64"
I would expect conan to handle this automatically (except PNG_ARM_NEON
of course, here we should switch to autotools, it seems to properly handle neon for iOS).
* - [libpng] set CMAKE_SYSTEM_PROCESSOR for M1 Signed-off-by: SSE4 <tomskside@gmail.com> * - argh, annoying hook!
see https://www.gitmemory.com/issue/glennrp/libpng/372/808830443
Specify library name and version: libpng/all
This is also a good place to share with all of us why you are submitting this PR (specially if it is a new addition to ConanCenter): is it a dependency of other libraries you want to package? Are you the author of the library? Thanks!
conan-center hook activated.