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

Raise the minimum CMake version to 3.10 #2523

Merged
merged 2 commits into from Sep 16, 2022
Merged

Conversation

dimztimz
Copy link
Contributor

The CMake file states 3.5 as minimal version, but 3.8 was actually minimal, see patch 47d56f2. Before that patch 3.9 was needed. This patch really fixes compatibility with 3.5.

@codecov
Copy link

codecov bot commented Sep 14, 2022

Codecov Report

Merging #2523 (7c4e3b2) into devel (359542d) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##            devel    #2523      +/-   ##
==========================================
+ Coverage   91.54%   91.56%   +0.02%     
==========================================
  Files         183      183              
  Lines        7562     7560       -2     
==========================================
  Hits         6922     6922              
+ Misses        640      638       -2     

@horenmar
Copy link
Member

No, thanks. When I have time for Catch2 again, I am going to instead bump up the minimum required version of CMake.

@dimztimz
Copy link
Contributor Author

Which version of Cmake do you want? My intention was to clean the file of two calls to project() inside if condition. That looks kinda hacky. Is 3.8 Acceptable?

@dimztimz
Copy link
Contributor Author

This patch does two things:

  1. Cleans some code, looks better
  2. Actually fixes a small bug. The bug is that the call to cmake_minimum_version currently is incorrect.

@horenmar
Copy link
Member

Do you have good reason for wanting to bump it up to 3.8 specifically? I looked at default packages in LTS ubuntu as a benchmark, bionic has 3.10.2, focal 3.16.3. Based on this I would target 3.16 on my own, but I am willing to hear arguments for other versions.

The cleanup makes sense, if it is still needed after the bump. I did the fastest thing I could when making the change.

@dimztimz
Copy link
Contributor Author

Well, I will list useful features that can be used.

  • v3.6 Adds support for Clang-tidy static analysis
  • v3.8 Adds support for C++17, support for Cpplint static analysis
  • v3.9 Improves command project()
  • v3.10 Adds support for Cppcheck static analysis.
  • v3.12 Improves command project().
  • v3.14 Simplifies calls to command install().

Personally, I'd bump it to 3.8 now, 3.10 in couple of months and I'd go above 3.10 when Ubuntu 18.04 goes out of support (precisely, in Extended Security Maintenance).

@horenmar
Copy link
Member

Ok, I am willing to keep it at 3.10 for now.

@dimztimz dimztimz changed the title Fix compatibility with CMake 3.5 Raise the minimum CMake version to 3.10 Sep 16, 2022
@dimztimz
Copy link
Contributor Author

I changed the PR so it raises the CMake minimum version to 3.10, and also it cleans the calls to project().

@horenmar
Copy link
Member

Thanks.

@horenmar horenmar merged commit 98d37da into catchorg:devel Sep 16, 2022
@horenmar horenmar added the Building and Packaging Issues affecting build/packaging scripts and utilities label Sep 16, 2022
@dimztimz dimztimz deleted the cmake branch September 16, 2022 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Building and Packaging Issues affecting build/packaging scripts and utilities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants