Add CMake-based build support on stable/2.11 #116
Conversation
|
I am packaging CoinUtils up to google-or tools for conda right now and I am basing it off of your CMake files. However, they don't seem to install a pkgconfig file, right? I think it would be nice to also configure a pkg-config file so that users of that toolchain can still use it. |
|
This patch was mainly to add Coin-OR to the CMake world (e.g. in order to consume it in google/or-tools when using the CMake build system, have a clean vcpkg package)... Sure you can add pkg-config generation using CMake but:
|
|
I understand what and why you're doing this -- but IMO this current CMake is wrong in this regard and it doesn't generate a "clean" recipe if it doesn't additionally install the pkg-config file since (especially in a package manager) other downstream packages might need / expect the file. Btw. do you want to be part of the maintainers for the conda packages? I am adding or-tools next, just waiting for the (lib) protobuf build on conda to be based on CMake as well: |
|
|
| target_compile_definitions(CoinUtils | ||
| PUBLIC HAVE_CONFIG_H | ||
| PRIVATE COINUTILS_BUILD) | ||
| if(CMAKE_VERSION VERSION_LESS "3.8.2") |
There was a problem hiding this comment.
Either this should be removed, or the toplevel cmake_minimum_required(VERSION 3.15) should be adjusted.
| if(NOT TARGET ZLIB::ZLIB) | ||
| find_package(ZLIB) | ||
| endif() | ||
| if(ZLIB_FOUND OR TARGET ZLIB::ZLIB) |
There was a problem hiding this comment.
Later on, ZLIB::ZLIB is required, thus ZLIB_FOUND is not relevant.
| set_target_properties(CoinUtils PROPERTIES | ||
| PUBLIC_HEADER "${_HDRS};${CMAKE_CURRENT_BINARY_DIR}/config_coinutils.h" | ||
| VERSION ${PROJECT_VERSION} | ||
| SOVERSION ${PROJECT_VERSION_MAJOR}) |
There was a problem hiding this comment.
This is incorrect, the current SOVERSION for libtool builds is 3.
You get the SOVERSION by calculating current - age from the libtool version triplett [current:revision:age], i.e. 14:6:11 -> SOVERSION = 14 - 11 = 3
Mizux
force-pushed
the
stable/2.11
branch
3 times, most recently
from
d820685
to
6422e15
Compare
- Fix ZLIB usage - Fix super build (e.g. FetchContent) integration - Add install_rpath
- Based on Docker for environment and Make for orchestration - Add amd64 docker cmake based build workflows
Add CMake based build + Makefile/docker CI + Travis-CI