fix(cmake): resolve build error due to malformed if() expression on Android platform.#4444
fix(cmake): resolve build error due to malformed if() expression on Android platform.#4444jackluo923 wants to merge 1 commit into
Conversation
17d1d45 to
daeceb8
Compare
| set(ZSTD_MULTITHREAD_SUPPORT_DEFAULT OFF) | ||
| # Handle old Android API levels | ||
| if((NOT ANDROID_PLATFORM_LEVEL) OR (ANDROID_PLATFORM_LEVEL VERSION_LESS 24)) | ||
| if((NOT ANDROID_PLATFORM_LEVEL) OR ANDROID_PLATFORM_LEVEL VERSION_LESS 24) |
There was a problem hiding this comment.
So the patch removes one parenthesis level,
and now it works better for cmake ?
There was a problem hiding this comment.
Yes. Removing the pair of parenthesis makes cmake happy.
Here's an explanation provided by ChatGPT: https://poe.com/s/6bcOKmapQY5A2mpfW5sc
|
Probably a parallel concern to this PR, |
This repo already appears to support Android cross-compilation through this workflow, albeit by directly invoking the make command rather than using |
is expecte to fail, due to #4444
|
I just tried to add a Is there anything different in this test, compared to yours ? |
is expecte to fail, due to #4444
We’re using the default CMake version (v3.22.1) provided by Ubuntu Jammy (22.04 LTS) package manager. Your CI run uses version v3.31.6. I’ll try upgrading and do some experiment. |
|
Here's the output using cmake version Here's the output using cmake version This means some how between v1.5.7 and the latest dev branch, the build problem disappeared. I am not sure what the root cause for this yet. |
|
That's weird. |
|
Wait, it did change, the line used to be : and now it's this: That's a small difference, but it was enough to create the problem you observed.
|
I guess this PR is no longer needed. Do we want to leave the current code as-is or improve it some how? |
|
I think indeed that the current code is fine, and the proposed change doesn't improve it. |
Description
When building Zstd v1.5.7 for the Android platform using CMake, the following error occurs during configuration:
This regression was introduced in 49fe2ec (refactor: modularize CMakeLists.txt for better maintainability). The problematic conditional logic is:
CMake is picky about parentheses for logical grouping inside if() statements. This leads to a parsing failure on valid Android configurations.
Fix
The parentheses surrounding the version check is removed to comply with CMake syntax rules.
Validation
Manually triggered a successful cross-compiled build with Android NDK26.