-
Notifications
You must be signed in to change notification settings - Fork 180
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
Refactored CMake build scripts #2189
base: master
Are you sure you want to change the base?
Conversation
I'm not a cmake expert and can't say is everything done correctly or not. New version definitely looks more clear. If it works properly I'm ready to merge it. I put this PR into cmake_cleanup branch. @loganmc10, @fzurita could you test that branch? |
I'll test in a bit, isn't 3.15.5 a little high for a minimum version though? The current LTS version of Ubuntu comes with 3.10 |
We need 3.15.5 for target_link_directories |
I think 3.15.5 is too high, it's going to stop working for anyone with a Raspberry Pi I would guess https://cmake.org/cmake/help/git-stage/command/target_link_directories.html
|
current raspbian's cmake is 3.13.4, and raspbian is probably more up to date than some of GLideN64's targets. agreed that this cannot be a requirement. |
What are the libs that need to be linked with on raspbian? (aka the fill filepath) With that we can make them an import target and drop the version down. |
Are we cool with requiring Cmake 3.13? |
The current LTS version of Ubuntu (supported until 2023 I think) has cmake 3.10. if we require higher than that, I think it will stop working for a good number of users |
Android only has CMake only up to 3.10 as well. |
Travis successfully built your PR for Linux and Mac. It would be good to setup CI build for RPI and Android too, I don't know how.
Current Ubuntu LTS is 18.04.4 (Bionic Beaver), is not it? This PR successfully built with it, but it is probably because it uses updated repo for cmake, commit a31c711
Then we are stuck with 3.10 Bradley, can you cleanup cmake files using 3.10 version? |
@gonetz What platforms are hard vs soft supported? |
Raspbian is just a prebuilt distro for raspberry pis. it's based on debian buster: https://www.raspberrypi.org/downloads/raspbian/. i won't make a CI but happy to test this PR at my end once the changes are done. |
3ed59a9
to
3e8b50b
Compare
The branch now builds on raspbian |
@bradc6 if you make a pull request here it will automatically try to build it: https://github.com/mupen64plus-ae/mupen64plus-ae/pulls You can try your updates on this file: https://github.com/mupen64plus-ae/mupen64plus-ae/blob/master/mupen64plus-video-gliden64/upstream/src/CMakeLists.txt |
@fzurita Thank you for the link, I'll give that a try and see if I can add android CI directly onto this repo |
If I get windows also building in cmake is there any reason to keep the msvc project either? That way there is nothing to "fall out of sync" against |
If cmake will generate completely equivalent msvc project, no. |
28c4deb
to
3fe5f50
Compare
…dern version of cmake
… more modern version of cmake" This reverts commit 5989140.
…ndefined symbol. This is probably needed in a larger scope but I don't have the other platforms to test at this time.
3e8b50b
to
5e6e108
Compare
@gonetz It can be done |
Bump? |
Hi Bradley, I just tried to build your PR with Android Studio:
Android SDK uses CMake 3.10.2.4988404. I have Ubuntu 18.4 on my PC. It also uses 3.10.2 |
Is it possible to check CMake version first and fall back to current 3.10 variant if CMake 3.13.4 or higher is not available? |
@gonetz Updated to 3.10. Could you build again? |
Ubuntu 18.04, CMakeOutput.log attached ~/WS/GLideN64/projects/cmake$ cmake -DCMAKE_BUILD_TYPE=Debug -DVEC4_OPT=On -DUSE_SYSTEM_LIBS=On -DMUPENPLUSAPI=On ../../src/ -- Configuring incomplete, errors occurred! |
Android Studio: /home/sergey/WS/mupen64plus-ae/mupen64plus-video-gliden64/upstream/src/CMakeLists.txt : C/C++ debug|x86 : CMake Error at /home/sergey/WS/mupen64plus-ae/mupen64plus-video-gliden64/upstream/src/osal/CMakeLists.txt:26 (target_link_directories): |
… does leak context but there is nothing we can really do about that at the moment
cmake -DCMAKE_BUILD_TYPE=Debug -DVEC4_OPT=On -DUSE_SYSTEM_LIBS=On -DMUPENPLUSAPI=On ../../src/ |
a missed some, one moment |
FYI, Android Studio now supports CMake 3.18 |
We should be good now. @bradc6 could you re-test your work on Windows and Linux(es)? |
Refactored the cmake files to be easier to read, the main could use more work though. Thoughts?