Fix libjpeg-turbo build for Android #8592
Conversation
This comment has been minimized.
This comment has been minimized.
|
Here is a more robust approach suggested in abseil: #8577 (comment) |
| @@ -117,7 +117,7 @@ def _configure_cmake(self): | |||
| if tools.Version(self.version) <= "2.1.0": | |||
| self._cmake.definitions["CMAKE_MACOSX_BUNDLE"] = False # avoid configuration error if building for iOS/tvOS/watchOS | |||
|
|
|||
| if tools.cross_building(self): | |||
| if tools.cross_building(self) and not self.settings.os == "Android": | |||
There was a problem hiding this comment.
This is not an actual fix.
The real bug is conan not respecting CONAN_CMAKE_SYSTEM_PROCESSOR when cross building.
See this bug report: conan-io/conan#9736
There was a problem hiding this comment.
Does conan automagically set CONAN_CMAKE_SYSTEM_PROCESSOR?
There was a problem hiding this comment.
Yes, to self.settings.arch.
Overriding it in the profiles does not work when cross building.
You can find a short repro in the issue (without requiring downloading the android ndk).
There was a problem hiding this comment.
Yes, to self.settings.arch.
But it's wrong anyway in some cases.
There was a problem hiding this comment.
Indeed, my issue is about being unable to override CONAN_CMAKE_SYSTEM_PROCESSOR.
It didn't work for Android/armv7 because the recipe specifies CMAKE_SYSTEM_PROCESSOR=armv7 which is not known to Android NDK CMake scripts. However the NDK toolchain does not require this variable (it sets the variable itself), so it does not need to be specified at all.
b13e177 to
5a122ee
Compare
|
Updated the MR using the suggested approach from the abseil recipe. |
This comment has been minimized.
This comment has been minimized.
|
All green in build 3 (
|
| if(NOT CMAKE_SYSTEM_PROCESSOR) | ||
| set(CMAKE_SYSTEM_PROCESSOR ${CONAN_LIBJPEG_SYSTEM_PROCESSOR}) | ||
| endif() |
There was a problem hiding this comment.
After some experimentations in libpng and using the same trick, I've found it should be:
if(NOT DEFINED CMAKE_SYSTEM_PROCESSOR OR CMAKE_SYSTEM_PROCESSOR STREQUAL "")
set(CMAKE_SYSTEM_PROCESSOR ${CONAN_LIBJPEG_SYSTEM_PROCESSOR})
endif()Indeed, CMAKE_SYSTEM_PROCESSOR seems to always be defined, just empty by default if cross-building.
There was a problem hiding this comment.
Oops, no it's fine, it doesn't check if CMAKE_SYSTEM_PROCESSOR is defined, so it works.
There was a problem hiding this comment.
"STREQUAL" check is surely more strict, but I can't imagine any sane value for this variable which could be rejected by simple "NOT".
However there are cases in CMake when its necessary to check a variable for emptiness. It's done simply by taking it's value in the quotes:
if("${VAR}" STREQUAL "")There's no need to check if it's defined.
There was a problem hiding this comment.
Yes, this check is perfectly fine, my bad, I thought the check was if(NOT DEFINED CMAKE_SYSTEM_PROCESSOR).
There was a problem hiding this comment.
Really, I tried NOT DEFINED first, but it didn't work, so I switched to just NOT.
It didn't work for Android/armv7 because the recipe specifies CMAKE_SYSTEM_PROCESSOR=armv7 which is not known to Android NDK CMake scripts. However the NDK toolchain does not require this variable (it sets the variable itself), so it does not need to be specified at all.
It didn't work for Android/armv7 because the recipe specifies
CMAKE_SYSTEM_PROCESSOR=armv7 which is not known to Android NDK CMake scripts.
However Conan specifies enough details to CMake in Android build, so there's
no need to specify CMAKE_SYSTEM_PROCESSOR manually.
Specify library name and version: libjpeg-turbo/2.1.2
conan-center hook activated.