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

Android template project doesn't build on `PROP_BUILD_TYPE=ndk-build` #19049

Closed
pyrosphere opened this Issue Sep 15, 2018 · 9 comments

Comments

Projects
None yet
4 participants
@pyrosphere
Copy link
Contributor

pyrosphere commented Sep 15, 2018

The Android template project, created from cocos new fails to build if PROP_BUILD_TYPE is changed to ndk-build.

The error it throws is Android NDK: Module MyGame_shared depends on undefined modules: uv_static probably from pull request #19015

@v1993

This comment has been minimized.

Copy link
Contributor

v1993 commented Sep 15, 2018

Question: do we still need ndk-build support on new projects? CMake seem much better at all (almost no separate build configuration, at least for files and flags) and works just fine.

@pyrosphere

This comment has been minimized.

Copy link
Contributor Author

pyrosphere commented Sep 15, 2018

New projects should probably use CMake, but this breaks old projects that already use ndk-build.

@v1993

This comment has been minimized.

Copy link
Contributor

v1993 commented Sep 15, 2018

old projects that already use ndk-build.

The Android template project, created from cocos new

I didn't get this.

P.S.: we should do one of things: fix it or remove ndk-build support at all (as not working).

@pyrosphere

This comment has been minimized.

Copy link
Contributor Author

pyrosphere commented Sep 15, 2018

Every project using ndk-build is broken - the option is broken, but to open an issue I wasn't going to submit "my project is broken", I tested it using the template and confirmed it wasn't from any of my own custom configurations.

But yes, either ndk-build should be fixed or support removed.

@v1993

This comment has been minimized.

Copy link
Contributor

v1993 commented Sep 15, 2018

Ok, get it now. Now just wait cocos team response.

@drelaptop

This comment has been minimized.

Copy link
Contributor

drelaptop commented Sep 16, 2018

It's a error on Android.mk, to fix this issue all you need to do is add this line

$(call import-module,uv/prebuilt/android)

also rembmer to update external libs.


I think the reason is all the CI change the build type to cmake, so this ndk-build error didn't been found. we should add a ndk-build task at least. I will do it.

this issue have been solved at #19045 (ndk-build CI passed, Not merged)


Question: do we still need ndk-build support on new projects?

keep ndk-build supported on v3 branch will perform a good compatibility, we should do that

@crazyhappygame

This comment has been minimized.

Copy link
Contributor

crazyhappygame commented Sep 16, 2018

@drelaptop I do not insist but I think that we should not support ndk-build
https://developer.android.com/ndk/guides/
Android Studio's default build tool to compile native libraries is CMake. Android Studio also supports ndk-build due to the large number of existing projects that use the build toolkit. However, if you are creating a new native library, you should use CMake.
I think that for new Android projects CMake should be used.
Support both cmake and ndk-build will be only source of problems like that one: " if PROP_BUILD_TYPE is changed to ndk-build/cmake"
Or something work on cmake/ndk-build does not work ndk-build/cmake ....

@drelaptop

This comment has been minimized.

Copy link
Contributor

drelaptop commented Sep 16, 2018

thanks for this advice, I hope so, but we can't do this on v3 branch. it will trouble developers when he upgrade engine release from 3.15 to 3.18.

as google explained, large number of existing projects that use the build toolkit, cocos2d-x projects are same with that. We have changed the default build-type to cmake, that means we hope developers to use cmake for new project.

@drelaptop

This comment has been minimized.

Copy link
Contributor

drelaptop commented Sep 17, 2018

#19045 merged, remember to execute download-deps.py to update external.

@drelaptop drelaptop closed this Sep 17, 2018

@drelaptop drelaptop added this to the 3.18 milestone Sep 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.