Skip to content
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

giflib: Fix build issue on recent Android NDK versions #25956

Merged
merged 4 commits into from
Nov 18, 2024

Conversation

eigenraven
Copy link
Contributor

Summary

Changes to recipe: giflib/5.2.x

Sets a maximum CMake policy version of 3.31.0 alongside the existing minimum of 3.1 to fix build issues when using recent Android NDKs that require CMake policies from 3.3.

Motivation

Without this patch, trying to use giflib in an Android ndk r27 project results in the following error:

CMake Warning (dev) at /home/runner/work/ubq/ubq/.conan2/p/androcf6124664d526/p/bin/build/cmake/flags.cmake:46 (if):
  Policy CMP0057 is not set: Support new IN_LIST if() operator.  Run "cmake
  --help-policy CMP0057" for policy details.  Use the cmake_policy command to
  set the policy and suppress this warning.

  IN_LIST will be interpreted as an operator when the policy is set to NEW.
  Since the policy is not set the OLD behavior will be used.
Call Stack (most recent call first):
  /home/runner/work/.../.venv/lib/python3.11/site-packages/cmake/data/share/cmake-3.30/Modules/Platform/Android-Clang.cmake:23 (include)
  /home/runner/work/.../.venv/lib/python3.11/site-packages/cmake/data/share/cmake-3.30/Modules/Platform/Android-Clang-C.cmake:1 (include)
  /home/runner/work/.../.venv/lib/python3.11/site-packages/cmake/data/share/cmake-3.30/Modules/CMakeCInformation.cmake:48 (include)
  CMakeLists.txt:2 (project)
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Error at /home/runner/work/ubq/ubq/.conan2/p/androcf6124664d526/p/bin/build/cmake/flags.cmake:46 (if):
  if given arguments:

    "hwaddress" "IN_LIST" "ANDROID_SANITIZE"

  Unknown arguments specified
Call Stack (most recent call first):
  /home/runner/work/.../.venv/lib/python3.11/site-packages/cmake/data/share/cmake-3.30/Modules/Platform/Android-Clang.cmake:23 (include)
  /home/runner/work/.../.venv/lib/python3.11/site-packages/cmake/data/share/cmake-3.30/Modules/Platform/Android-Clang-C.cmake:1 (include)
  /home/runner/work/.../.venv/lib/python3.11/site-packages/cmake/data/share/cmake-3.30/Modules/CMakeCInformation.cmake:48 (include)
  CMakeLists.txt:2 (project)


-- Configuring incomplete, errors occurred!

Details

Added a backwards-compatible "maximum policy version" as recommended by https://cmake.org/cmake/help/latest/command/cmake_minimum_required.html. The CMake file is really simple, and testing it with current CMake 3.31 revealed no compatibility warnings or issues after bumping the version. I've tested a local conan build on macOS 14.7 + Xcode 16 and successfully cross-compiled to Android using NDK r27.


@perseoGI perseoGI self-assigned this Nov 15, 2024
perseoGI
perseoGI previously approved these changes Nov 15, 2024
Copy link
Contributor

@perseoGI perseoGI left a comment

Choose a reason for hiding this comment

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

I was really confused at first sight because I thought you were defining a maximum CMake version but I see now that the ... defines the maximum CMake policy (and not version). And it is also backwards compatible!

Looks good to me, thank you!

Copy link
Contributor

@valgur valgur left a comment

Choose a reason for hiding this comment

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

Need to use a newer minimum policy version instead of capping the maximum one.

Co-authored-by: Martin Valgur <martin.valgur@gmail.com>
@eigenraven
Copy link
Contributor Author

@valgur I've updated to the minimum of 3.15 as suggested, but my previous solution didn't cap the maximum to 3.31 any more than the old code capped it to 3.1 - the second version in the version range is used to apply newer CMake policies up to a certain version to indicate wider tested compatibility across a range of versions and doesn't in any way limit using newer versions.

@perseoGI
Copy link
Contributor

I have made some recipe refactor.
This kind of changes are restricted to conan maintainers.
Also this changes are needed right now to force the revision export.
Ignore them!

@jcar87 jcar87 merged commit 5fecff8 into conan-io:master Nov 18, 2024
10 checks passed
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.

5 participants