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

cmake: bump minimum cmake version to 3.0 + add target_include_directories #42

Merged
merged 4 commits into from
Jun 15, 2022

Conversation

madebr
Copy link
Contributor

@madebr madebr commented Jun 6, 2022

  1. Bumping CMake minimum version to 3.0 gets rid of these warnings:
    CMake Deprecation Warning at CMakeLists.txt:12 (cmake_minimum_required):
      Compatibility with CMake < 2.8.12 will be removed from a future version of
      CMake.
    
      Update the VERSION argument <min> value or use a ...<max> suffix to tell
      CMake that the project does not need compatibility with older versions.
    
    and
    CMake Warning (dev) at CMakeLists.txt:15 (project):
      Policy CMP0048 is not set: project() command manages VERSION variables.
      Run "cmake --help-policy CMP0048" for policy details.  Use the cmake_policy
      command to set the policy and suppress this warning.
    
      The following variable(s) would be set to empty:
    
        CMAKE_PROJECT_VERSION
        CMAKE_PROJECT_VERSION_MAJOR
        CMAKE_PROJECT_VERSION_MINOR
        CMAKE_PROJECT_VERSION_PATCH
    This warning is for project developers.  Use -Wno-dev to suppress it.
    
  2. use list(APPEND) where applicable.
  3. use WIN32.
    Also, the comment "we're about to move to Win64" is outdated.
  4. conditionally enable c++
  5. Create PhysFS::PhysFS and PhysFS::PhysFS-static targets, and export those via the installed cmake packages.
  6. Use target_include_directories(tgt PUBLIC "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}")
    This fixes CMake: Use target_include_directories() #14
    (You need to use a generator expression because you don't want to make build machine paths to appear in the exported cmake package)

Fixes #14

Copy link
Owner

@icculus icculus left a comment

Choose a reason for hiding this comment

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

I confess that CMake scares the crap out of me, so I'm taking it on faith that the "PhysFS::" namespace stuff isn't going to cause problems...minor questions and notes aside, most of this PR looks pretty good.

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
@madebr
Copy link
Contributor Author

madebr commented Jun 14, 2022

I've added MinGW to the ci matrix (using a pre-installed toolchain on the windows image)

CMakeLists.txt Outdated Show resolved Hide resolved
@sezero sezero merged commit 2a90b1f into icculus:main Jun 15, 2022
@madebr madebr deleted the cmake-tweaks branch June 15, 2022 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CMake: Use target_include_directories()
4 participants