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

Support CMake 3.10-3.20 #422

Merged
merged 3 commits into from May 15, 2021
Merged

Conversation

carstene1ns
Copy link
Member

Add github action to check for breakage.

- Set minimal version to 3.10, remove older modules
- Support cmake older than 3.12 (no list transform)
- Explain missing doxygen (avoid user confusion)
- Set all policies to NEW (for tested versions)
to check for compilation failures on older cmake/compilers
Adapt .editorconfig
@carstene1ns carstene1ns added this to the 0.6.3 milestone May 14, 2021
cmake_minimum_required(VERSION 3.10...3.20 FATAL_ERROR)
if(${CMAKE_VERSION} VERSION_LESS 3.12)
# Use most recent policies, even for older cmake
cmake_policy(VERSION ${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION})
Copy link
Member Author

Choose a reason for hiding this comment

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

The above is to specify "supported versions" and to enable all policies.
(See https://cliutils.gitlab.io/modern-cmake/chapters/basics.html)

@carstene1ns carstene1ns changed the title Support CMake 3.10 Support CMake 3.10-3.20 May 14, 2021
@Ghabry
Copy link
Member

Ghabry commented May 14, 2021

  • The module FindEXPAT was added in 3.10 and can be removed
  • The module FindICU was added in 3.7 and can be removed. There is an improvement in 3.11 for Windows but on Windows we usually have the version VIsual Studio bundles, so one of the latest
  • FindIconv was added in 3.11, so must stay (for now ;))

apt-get update
apt-get install -y --no-install-recommends --no-install-suggests \
build-essential cmake ninja-build git \
libicu-dev libexpat1-dev
Copy link
Member

Choose a reason for hiding this comment

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

This is also great for an updated "how to install" guide because I see the package names to install :D

Copy link
Member Author

Choose a reason for hiding this comment

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

Debian still has funny names for some libs. For player it is worse.

Copy link
Member

@Ghabry Ghabry left a comment

Choose a reason for hiding this comment

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

Modules must be deleted, otherwise: LGTM

@carstene1ns carstene1ns force-pushed the bugfix/old-cmake branch 2 times, most recently from 9f7b33e to 14606cd Compare May 14, 2021 22:27
@Ghabry Ghabry merged commit de724ca into EasyRPG:master May 15, 2021
@@ -309,11 +296,7 @@ set(LCF_HEADERS
src/lcf/third_party/string_view.h
)

list(TRANSFORM LCF_SOURCES PREPEND ${CMAKE_CURRENT_SOURCE_DIR}/)
list(TRANSFORM LCF_HEADERS PREPEND ${CMAKE_CURRENT_SOURCE_DIR}/)
list(APPEND LCF_HEADERS ${CMAKE_CURRENT_BINARY_DIR}/src/lcf/config.h)
Copy link
Member Author

Choose a reason for hiding this comment

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

cc #423.

@carstene1ns carstene1ns deleted the bugfix/old-cmake branch May 15, 2021 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants