search

commonmark / cmark

CommonMark parsing and rendering library and program in C
Other
1.62k stars 539 forks source link

Update CMakeLists.txt #393

Closed compnerd closed 3 years ago

compnerd commented 3 years ago

Bump the minimum required CMake to 3.7. CMake 3.1 is required for the CMAKE_C_STANDARD and CMAKE_C_STANDARD_REQUIRED options to take effect. Relying on CMake to control the compiler this way allows us to avoid having to rebuild the logic for detecting the appropriate flags and ensuring that the compiler is able to support the necessary language version.

compnerd commented 3 years ago

@jgm this is a follow up from the discussion that occurred in #392. By bumping the CMake version, we can ensure that the compiler is properly used by the build.

jgm commented 3 years ago

Are there associated changes you'd make if the version is bumped? Maybe they should be added here? Otherwise I don't see why we should gratuitously bump the version; that just makes it not work for certain users without providing any advantage to anyone.

compnerd commented 3 years ago

No, the desired change is already in the tree here: https://github.com/commonmark/cmark/blob/master/CMakeLists.txt#L17-L19

Id say that we could bump to 3.1 if you prefer, but it seems unnecessarily low. I would actually suggest that we should bump to 3.18 and then roll back to the more declarative based CMake with generator expressions. However, this change conservatively bumps the version to ensure that the compiler flags for the language version are setup properly.

jgm commented 3 years ago

got it, thanks