Skip to content

Fix Android NDK 22 build error#19501

Merged
opencv-pushbot merged 1 commit intoopencv:masterfrom
komakai:android-ndk22
Feb 14, 2021
Merged

Fix Android NDK 22 build error#19501
opencv-pushbot merged 1 commit intoopencv:masterfrom
komakai:android-ndk22

Conversation

@komakai
Copy link
Copy Markdown
Contributor

@komakai komakai commented Feb 11, 2021

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work

Fix for #19500

set(ANDROID_GRADLE_PLUGIN_VERSION "4.0.1" CACHE STRING "Android Gradle Plugin version (4.0.1)")
message(STATUS "Android Gradle Plugin version: ${ANDROID_GRADLE_PLUGIN_VERSION}")

set(GRADLE_VERSION "6.1.1" CACHE STRING "Gradle version (6.1.1)")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What prevent from using most recent 6.8.2 ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What prevent from using most recent 6.8.2 ?

Nothing - I'm just proposing a PR to fix this one particular issue. If you want to propose a PR to bring the OpenCV Android implementation up to the bleeding edge (maybe add some Kotlin extensions as well) then please go ahead 😃

@@ -1,7 +1,10 @@
# https://developer.android.com/studio/releases/gradle-plugin
set(ANDROID_GRADLE_PLUGIN_VERSION "3.2.1" CACHE STRING "Android Gradle Plugin version (3.0+)")
set(ANDROID_GRADLE_PLUGIN_VERSION "4.0.1" CACHE STRING "Android Gradle Plugin version (4.0.1)")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What prevent from using most recent 4.1.2 ?

@alalek
Copy link
Copy Markdown
Member

alalek commented Feb 11, 2021

relates #19048


android_sdkmanager = os.path.join(os.environ['ANDROID_SDK'], 'tools', 'bin', 'sdkmanager')
accept_licenses_proc = subprocess.Popen(['yes | ' + android_sdkmanager + ' --licenses'], shell=True)
log.debug('yes | ' + android_sdkmanager + ' --licenses')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. This must be done by User before running this script. At least not implicitly in automatic mode.
  2. This would not work on Windows.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@alalek I wasn't intending to leave this code in. It was just an attempt to get the failing build to pass. (Which anyway doesn't seem to have worked.) I will revert it.

@alalek
Copy link
Copy Markdown
Member

alalek commented Feb 11, 2021

My suggestion is to keep "existed" versions in CMake file and put new versions values into the new ndk-22.config.py file (through cmake_vars). They will override values stored in CMake scripts.

@komakai komakai reopened this Feb 13, 2021
@zchrissirhcz
Copy link
Copy Markdown
Contributor

There is a related issue:

Android NDK 22 may link to libz.a instead of libz.so for imgcodecs module (static lib), leading to link error (libz.a not found) on machines without same NDK version installed in same directory.

PR #19522 fix that issue.

To finally build a correct OpenCV Android package, I think #19521 #19522 and current PR #19501 are all required and should be merge in this order.

Correct me if I made any mistake guess.

Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Thank you 👍

@opencv-pushbot opencv-pushbot merged commit ad66b07 into opencv:master Feb 14, 2021
@alalek alalek mentioned this pull request Feb 21, 2021
@alalek alalek mentioned this pull request Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants