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

CMake need to improve, record ideas #18717

Open
drelaptop opened this Issue Mar 8, 2018 · 38 comments

Comments

Projects
None yet
4 participants
@drelaptop
Collaborator

drelaptop commented Mar 8, 2018

  • cocos2d-x version: latest in github

  • 1. info.plist is always created when -GXcode, it's bad friendly for custom info.plist

    • can't find a method to solve this for now, most cmake use template info.plist
  • 2. iOS target and macOS can't be in one project, some complex for switch

  • 3. write cmake docs, how to use step by step, add link in main readme.md

  • 4. sub-dir tree of header files isn't include in Xcode or VS project

  • 5. supply simple python script to execute pre-defined shell command, let cmake easy to use

    • plan to do it, in the future, it's better finish this before 3.17 next milestone released.
  • 6. delete the old project build files or not if cmake can do build well

    • the cmake build isn't so good as the older way for iOS platform, need to find a solution
    • might have to wait 4.0 version to do this
  • 7. cmake support pre-compiled headers, to increase the compile speed

    • no plan to do it recently, hope cocos2d-x forum help
  • 8. What about cmake and cocos2d-x-3rd-party-libs?

  • 9. Lua Template project on win32, exist different behavior between run with VS and double-click run on Explorer

  • 10. using prebuilt libs on Android Gradle Build, we must set COCOS_PREBUILT_ROOT to finger the prebuilt libs root library.

  • 11. [High Priority] Resource files didn't update before build a target in Xcode, need to solve this issue,

    • temp solution is re-execute cmake .. in the generated Xcode project folder
  • 12. [HighLight] cmake build support mingw on Windows, more than 2 developers asked this

    • no plan yet, might do it in future
  • 13. build APK for android on pure cmake build

    • I think it's impossiable until Google support this firstly.
@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented Mar 9, 2018

What about cmake and cocos2d-x-3rd-party-libs?

@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented Mar 10, 2018

It's a good idea to let cmake support 3rd-party-libs finally. We can finish it in the future, but I have no plan to do it recently.

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented Mar 15, 2018

@drelaptop Once again great job!
I reviewed current implementation below my comments:

  1. Add CI travis/appveyor build for cmake mac/win32
    Without that sooner or later something will stop working for cmake

  2. Remove from cocos2d-x\cmake\Modules files Find*.cmake which are delivered with cmake by default (e.g. FindCURL.cmake)
    CMake installation will have more upto-date version.

  3. Remove duplicated defines (e.g. remove USE_BOX2D use CC_ENABLE_BOX2D_INTEGRATION everywhere)

  if (USE_BOX2D)
    add_definitions(-DCC_ENABLE_BOX2D_INTEGRATION=1)
  else()
    add_definitions(-DCC_ENABLE_BOX2D_INTEGRATION=0)
  endif()
  1. Reduce number of functions (e.g. get_target_depends_ext_dlls). Simplify current implementation
    Current logic is too complicated. Too many variables, functions
    Dll dependencies should be setup by FindPackage.cmake or add_subdirectory
    CMakeLists.txt should looks like below:
if(ANDROID)
    add_library(foo )
else   
    add_executable(foo)
endif()

target_sources(foo file1.cpp ....)
if(ANDROID)
    target_sources(foo android_specific.cpp)
endif()

add_subdirectory(sub)
add_subdirectory(...)

find_package(CURL)
find_package(....)

target_include_directories(foo PRIVATE ${CURL_INCLUDE_DIRS}   .....)
target_link_libraries(foo PRIVATE ${CURL_LIBRARIES} sub ......)

if(MSVC)
    find_package(msvc_specific)
    target_link_libraries(foo msvc_specific)
endif()
  1. Do we really need XCode, Visual Studio support?
    Maintaining list of resources and headers and other XCode/Visual Studio specific files make CMakeLists.txt more complicated

  2. Modernize current implementation
    We use old syntax, obsolete commands like:
    INCLUDE_DIRECTORIES(), ADD_DEFINITIONS(), LINK_LIBRARIES()

Please see below two documents (there are also videos on youtube)
https://github.com/boostcon/cppnow_presentations_2017/blob/master/05-19-2017_friday/effective_cmake__daniel_pfeifer__cppnow_05-19-2017.pdf

https://github.com/CppCon/CppCon2017/blob/master/Tutorials/Using%20Modern%20CMake%20Patterns%20to%20Enforce%20a%20Good%20Modular%20Design/Using%20Modern%20CMake%20Patterns%20to%20Enforce%20a%20Good%20Modular%20Design%20-%20Mathieu%20Ropert%20-%20CppCon%202017.pdf

@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented Mar 16, 2018

@crazyhappygame thank you for so detail suggestion, I like you.

  1. Add CI travis/appveyor build for cmake mac/win32

    Not need do it now. we should do that after the cmake can replace the older build system. I hope in 3.17 the cmake and older build system both works well.

  2. Remove from cocos2d-x\cmake\Modules files Find*.cmake

    Good idea, I will do that in the next days

  3. Remove duplicated defines

    Yes, we should do that.

  4. Reduce number of functions (e.g. get_target_depends_ext_dlls)

    We should reduce, but about the logic of collect dlls, I can't find a simpler implementation. When consider this issue, we should consider the situation of using prebuilt libs.
    For example, build the cpp-empty-test and use the cocos2d.a prebuilt libs, in this situation we didn't need to add cocos/CMakeLists.txt, and then how we know the dll list of cocos2d lib needed? for we need to copy the dll into app binary dir. The current logic, CocosPickLibs.cmake, CocosUseLibs.cmake and cocos_use_pkg() do this work.

  5. Do we really need XCode, Visual Studio support?

    We really need Xcode, I think many users depend Xcode to develop a iOS game, how to Debug if we didn't support IDE, Debug by command line is really hard for most users.

  6. Modernize current implementation

    Thanks for this instructions, it's very useful when doing CMake Code refactoring. I will read the document, however I have the plan to research "creator_to_cocos2dx" plugin, and then do some bug fix and improvements. so I am afraid I won't do this work in next days, might do it in recent future. I will ask @minggo suggestion about this task.

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented Mar 16, 2018

ad1. I was thinking about adding new cmake CI configuration (old CI configs should stay unchanged)

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented Mar 18, 2018

Could you at least add Travis CI test build for android with prebuilds?
i.e. 3 new projects

cocos new --agreement n -l cpp -p my.pack.test1 test1
cocos new --agreement n -l cpp -p my.pack.test2 test2
cocos new --agreement n -l cpp -p my.pack.test3 test3
.....
@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented Apr 29, 2018

@crazyhappygame finish 2 items below at PR #18799 , could you review it when you are free?

2. Remove from cocos2d-x\cmake\Modules files Find*.cmake

Good idea, I will do that in the next days

3. Remove duplicated defines

Yes, we should do that.
@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented May 5, 2018

@drelaptop
2. Remove from cocos2d-x\cmake\Modules files Find*.cmake - looks good, Linux compilation passed
3. Remove duplicated defines - this looks bad, my idea was about renaming options/defines. In PR #18799 it seems like options are not used at all ..... I think that we should rollback this change and add all add_definitions
Currently it is impossible to enable/disable options/defines by cmake command line. For example below do nothing:
cmake -DUSE_CHIPMUNK=OFF
before it disabled chipmunk and physics integration

@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented May 7, 2018

Thanks for review

For example below do nothing:
cmake -DUSE_CHIPMUNK=OFF

cmake .. -DUSE_CHIPMUNK=OFF this is NOT do nothing, it disable the USE_CHIPMUNK in CMake build, you can find this in CMakeCache.txt

//Use chipmunk for physics library
USE_CHIPMUNK:BOOL=OFF

So I think the issue not can't enable/disable options by cmake command line , but the USE_*** logic. I think the USE_*** logic is awful, for example I can't enable the Box2D Tests in cpp-tests by set USE_BOX2D=ON ONLY. the logic should improved later. rollback this change isn't necessary, for those changes didn't make the cmake build worse.

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented May 7, 2018

@drelaptop Did you run win32/mac/ios/android cmake build after you change? I just run cmake for cocos2d-x\tests\cpp-empty-test for win32 and it fails for me. I think that it worked before... It seems like currently it tries to links to VS2013 libbullet.lib with VS2015 win32 app.

I have following proposition: in travis/appveyor there are android_cocos_new_test, windows32_cocos_new_test CI build configuration. Maybe we may run for them cmake build?

I have below error:

7>libchipmunk.lib(cpArbiter.obj) : MSIL .netmodule or module compiled with /GL found; restarting link with /LTCG; add /LTCG to the link command line to improve linker performance
7>LINK : warning LNK4075: ignoring '/INCREMENTAL' due to '/LTCG' specification
7>libbullet.lib(btAlignedAllocator.obj) : warning LNK4075: ignoring '/EDITANDCONTINUE' due to '/OPT:LBR' specification
7>libbullet.lib(btCollisionDispatcher.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btCollisionWorld.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btDefaultCollisionConfiguration.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btDbvtBroadphase.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btDiscreteDynamicsWorld.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btSequentialImpulseConstraintSolver.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btCollisionObject.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btRigidBody.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btGhostObject.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btManifoldResult.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btCollisionShape.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btConvexShape.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btBvhTriangleMeshShape.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btQuickprof.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btConvexConvexAlgorithm.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btEmptyCollisionAlgorithm.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btConvexConcaveCollisionAlgorithm.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btCompoundCollisionAlgorithm.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btCompoundCompoundCollisionAlgorithm.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btConvexPlaneCollisionAlgorithm.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btBoxBoxCollisionAlgorithm.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btSphereSphereCollisionAlgorithm.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btSphereTriangleCollisionAlgorithm.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btDbvt.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btOverlappingPairCache.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btSimulationIslandManager.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btTypedConstraint.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btConeTwistConstraint.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btGeneric6DofConstraint.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btQuantizedBvh.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btOptimizedBvh.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btPolyhedralConvexShape.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btActivatingCollisionAlgorithm.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btPolyhedralContactClipping.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btCollisionAlgorithm.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btHashedSimplePairCache.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btUnionFind.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btConvexPolyhedron.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btConvexHullComputer.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>libbullet.lib(btGeometryUtil.obj) : error LNK2038: mismatch detected for '_MSC_VER': value '1800' doesn't match value '1900' in AppDelegate.obj
7>   Creating library G:/cocos_latest/subrepos/external/cocos2d-x/tests/cpp-empty-test/build2/Debug/cpp-empty-test.lib and object G:/cocos_latest/subrepos/external/cocos2d-x/tests/cpp-empty-test/build2/Debug/cpp-empty-test.exp
7>LINK : warning LNK4098: defaultlib 'LIBCMTD' conflicts with use of other libs; use /NODEFAULTLIB:library
7>glfw3.lib(init.c.obj) : error LNK2001: unresolved external symbol __imp__vsnprintf
7>MSVCRTD.lib(vsnprintf.obj) : error LNK2001: unresolved external symbol __imp__vsnprintf
7>libjpeg.lib(jerror.obj) : error LNK2001: unresolved external symbol ___iob_func
7>libtiff.lib(tif_unix.obj) : error LNK2001: unresolved external symbol __imp____iob_func
7>libpng.lib(pngerror.obj) : error LNK2001: unresolved external symbol __imp____iob_func
7>libchipmunk.lib(chipmunk.obj) : error LNK2001: unresolved external symbol __imp____iob_func
7>libtiff.lib(tif_unix.obj) : error LNK2001: unresolved external symbol __imp__vfprintf
7>libchipmunk.lib(chipmunk.obj) : error LNK2001: unresolved external symbol __imp__vfprintf
7>MSVCRTD.lib(vsnprintf.obj) : error LNK2001: unresolved external symbol __imp___vsnprintf
7>G:\cocos_latest\subrepos\external\cocos2d-x\tests\cpp-empty-test\build2\bin\cpp-empty-test\Debug\cpp-empty-test.exe : fatal error LNK1120: 5 unresolved externals
@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented May 8, 2018

cpp-tests project is tested on mac/iOS/Android. win32 is tested on Windows10 + VS2017.

Did you update you external libs? VS2013 libbullet.lib have been removed 6 months ago. and all vs2013 libs have been removed. @crazyhappygame

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented May 9, 2018

@drelaptop it works for me. I am sorry for false alarm. I had broken build environment.

@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented May 17, 2018

#18816 might fixed

  1. [High Priority] Resource files didn't update before build a target in Xcode, need to solve this issue,

not tested yet

@v1993

This comment has been minimized.

Contributor

v1993 commented May 18, 2018

Some my ideas (in order of importance decreasing):

  1. Add MinGW support (I'm not shure how hard is it, but would be very helpful for me).
  2. Icons support for cmake (windows game.ico and add icon for linux (window icon)).
  3. Not very important and probably not to be done, but still an idea: build APK for android on pure cmake build (in this case also implement 2nd idea for android).
@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented May 19, 2018

Add MinGW support (I'm not shure how hard is it, but would be very helpful for me).

no plan yet, I will research it if am free between 3.17 and 3.18. and very glad if one of cocos2d-x developers to add this feature.

Icons support for cmake (windows game.ico and add icon for linux (window icon)).

Icons for windows I prepare to fix it, I think it's not a hard things when cmake generated VS project.
Icons for Linux, I am not a linux native developer, no ideas yet, hope forum gives help

Not very important and probably not to be done, but still an idea: build APK for android on pure cmake build (in this case also implement 2nd idea for android).

We have the ability to add the cmake support for Android( NDK) build in Gradle, because Google support this, we can't implement pure cmake build (Not Needed Android Studio) if Google doesn't support this.

AFAIK, Google didn't support pure cmake build for Android, Android Studio is necessary.

@v1993

This comment has been minimized.

Contributor

v1993 commented May 19, 2018

Icons support for linux is impossible on compile time, only on runtime.

I've implemented runtime solution in #18833.

@v1993

This comment has been minimized.

Contributor

v1993 commented May 19, 2018

I just met bug: if there is symlink in Resources, CMake crashes:

CMake Error at /home/v/compile/cocos2d-x-mine/cmake/Modules/CocosBuildHelpers.cmake:38 (configure_file):
  configure_file input location

    /home/v/compile/cocos2d-x-mine/tests/cpp-empty-test/Resources/icons

  is a directory but a file was expected.
Call Stack (most recent call first):
  CMakeLists.txt:141 (cocos_copy_res)

@drelaptop drelaptop added this to the 3.18 milestone May 22, 2018

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented May 24, 2018

@drelaptop Please check below repository https://github.com/crazyhappygame/modern_cmake
As example I setup Box2D and recast (two cocos2d-x-3rd-party-libs) to compile and use cmake. There is also set travis linux/mac and windows CI build for that repo. (to check that it really works).

Please check how nice and simple are all CMakeLists.txt files when you use modern cmake and you compile all from source code.

I would like to persuade you to migrate as first cocos2d-x-3rd-party-libs to modern cmake. Having static libs for all cocos2d-x-3rd-party-libs and for all platforms will significantly simplify others CMakeLists.txt files.

@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented May 25, 2018

@crazyhappygame that's wonderful, I want to improve the cmake using modern cmake in 3.18 milestone.

I find you use file(GLOB_RECURSE SOURCES Box2D/*.cpp) to collect the source files, but it's not a way that cmake official suggest us to use.

cmake_minimum_required(VERSION 3.9) is 3.9+ release necessary for using modern cmake? or only 3.1+

if we can find a better way to use generated libs by cmake, are you satisfied with the GEN_COCOS_PREBUILT and USE_COCOS_PREBUILT implement?

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented May 25, 2018

@drelaptop
I change GLOB_RECURSE to list of files.

CMake 3.9 was used because this is in Travis and appveyor. CMake 3.1 should be fine.

About prebuilds. Prebuilds are for free with this folder structure(CMakeLists.txt in root do prebuilds). "game1" "game2" reuse engine build. Please check e.g Travis build output engine is compiled only once.

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented May 25, 2018

About cmake version. Cmake official Android support was added in cmake 3.7. Cmake 3.9 was released one year ago.

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented May 29, 2018

@drelaptop I added android appveyor and Travis CI configuration to: https://github.com/crazyhappygame/modern_cmake

I have problems with setting up iOS...

I really think that we need to setup cmake for cocos2d-x-3rd-party-libs and compile cocos2d-x from source code.

Once again.... Please check how nice and simple are all CMakeLists.txt files when you use modern cmake and you compile all from source code.

Can we plan cmake for cocos2d-x-3rd-party-libs in release 3.18?

@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented May 29, 2018

@crazyhappygame thanks for this great work. I will set it higher priority into 3.18 milestone.

according to @minggo 's suggestion, at this time I am researching a critical audio crash issue (#18597). after this I think I need to solve some network critical issues. and then do the cmake for 3rd-party-libs, cocos CLI support cmake (for example #18862 ).

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented May 29, 2018

@drelaptop If you do not mind I propose the following plan:

  1. Create new repo e.g. "cocos2d/cocos2d-x-external"
  2. Copy https://github.com/crazyhappygame/modern_cmake to "cocos2d/cocos2d-x-external"
  3. Setup CI travis, appveyor (fix iOS CI build configuration, add more configurations for android: e.g.: arm64-v8a, x86)
  4. Copy all source code from cocos2d-x-3rd-party-libs (current version of cocos2d-x-3rd-party-libs source code for compilation, but source code, no downloading, no unzipping etc..) to "cocos2d/cocos2d-x-external"
  5. In parallel (you, me and maybe someone will join us :) ) PR by PR add CMakeLists.txt (modern cmake using add_subdirectory, add_library, target_* only :) ) for each library (of course each time check that CI travis, appveyor pass)

Please let me know what do you think about this plan. First 3 points you will have to do.

@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented May 30, 2018

@crazyhappygame thanks, it's really a good plan.

I want to start this work now, but at this time I have to solve some critical issues. for I can't spend all my time into improving the cocos2d-x build. After some issues solved, I will discuss this with @minggo .

Anyway adding cmake support for all 3rd-party libs is meaningful. it will make custom 3rd libs easier, and upgrade 3rd libs quicker. what you have done is a great things for cocos2d-x.

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented Jun 4, 2018

@drelaptop for you info: added android x86, x86_64, arm64-v8a ci configs, and iOS (unfortunetlt works on my mac , fails on Travis) to repo https://github.com/crazyhappygame/modern_cmake

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented Jun 10, 2018

@drelaptop for you info: fixed iOS travis build and added ctest to https://github.com/crazyhappygame/modern_cmake

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented Jun 17, 2018

@drelaptop ping :) Did you have a chance to talk with @minggo about cocos2d-x-3rd-party-libs and cmake build from soruces?

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented Jun 21, 2018

@drelaptop @slackmoehrle @minggo Could you read: #18717 (comment) ?
I propose to setup CMake for all cocos2d-x-3rd-party-libs and built always from source code cocos2d-x+deps.

In https://github.com/crazyhappygame/modern_cmake as example I set CMake for Box2D and recast with CI travis/appveyor for all currently supported platforms: android (arm, x86, x86_64, arm64-v8a), iOS, mac, linux, win32.

Some advantages:

  1. No manual recompilation for new Android NDK/SDK, compiler versions, system version etc ...
  2. No mismatches for prebuilds. See e.g.: Android https://android.googlesource.com/platform/ndk/+/master/docs/user/common_problems.md#using-mismatched-prebuilt-libraries
  3. Easy update or addition new libraries (for example now we need support metal for Mac/iOS, later probably vulcan and Android etc. )

I think that workload related to "Migrate cocos2d-x-3rd-party-libs to CMake" will be small and advantages will be huge!

Please add "Migrate cocos2d-x-3rd-party-libs to CMake" to cocos2d-x 3.18 roadmap.
From my side I can offer commitment and help.

@minggo

This comment has been minimized.

Contributor

minggo commented Jul 18, 2018

It is good if it can replace current building system in https://github.com/cocos2d/cocos2d-x-3rd-party-libs-src. Current building system is a little complex, and does not support windows.

@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented Jul 24, 2018

now we answered the item 8

quite grateful to @crazyhappygame , lots of work to push the cmake process on cocos2d-x.

@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented Nov 6, 2018

  1. using prebuilt libs on Android Gradle Build, we must set COCOS_PREBUILT_ROOT to finger the prebuilt libs root library.

Item 10 became invalid, for #19109

@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented Nov 7, 2018

About the item 6

  1. delete the old project build files or not if cmake can do build well
    the cmake build isn't so good as the older way for iOS platform, need to find a solution
    might have to wait 4.0 version to do this

I think we can delete the VS project files, keep cmake build only on win32 now.

  • VS project files existed compatibility issues with only tested on VS2015 but run on VS2017. Intellisense error, fixed toolset.
  • cmake will auto find valid compile tools, and generate a proper visual studio project.
  • we want to push cmake to the only build tools in cocos2d-x, it's better to do it step by step.
  • the reason for only remove VS project files, not include Xcode is that VS project supplied isn't works well and stable like Xcode.

what's your options? @minggo @PatriceJiang @crazyhappygame @v1993

@minggo

This comment has been minimized.

Contributor

minggo commented Nov 7, 2018

I think we should keep VS project, but use cmake to generate new VS project files. We should discuss in the forum before we do this change.

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented Nov 7, 2018

I think we should keep VS projects. Most users still use VS projects.
Also cmake still needs some improvements:

  1. No support for multiple configs (debug, release)
  2. Complicated *.dll copy logic
@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented Nov 7, 2018

  1. No support for multiple configs (debug, release)

I will try to use generator-expressions in multiple configs related soon

  1. Complicated *.dll copy logic

sorry, I don't hold same idea with you. the *.dll copy logic is complicated, but it's properly works. the logic copy dlls depend on target property, meanwhile consider libraries dependence.

@drelaptop drelaptop modified the milestones: 3.17.1, next Nov 12, 2018

@drelaptop

This comment has been minimized.

Collaborator

drelaptop commented Nov 29, 2018

maybe we can find a proper way to realize prebuilt libs feature, and still keep cmake scripts simple, easy understand. @crazyhappygame

refer to thread

https://discuss.cocos2d-x.org/t/not-elegant-cmake-prebuilt-feature-was-removed-in-3-17-1-release/44688

@crazyhappygame

This comment has been minimized.

Contributor

crazyhappygame commented Dec 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment