iamantony / qtcsv

Library for reading and writing csv-files in Qt.
MIT License
265 stars 141 forks source link

add CMake project handling #28

Closed cguentherTUChemnitz closed 7 years ago

cguentherTUChemnitz commented 7 years ago

I want to create a conan.io package of your project. It easier to handle this, when the project builds with cmake. Therefore i implemented cmake project handling. I tried to adopt as most as possible of your qmake setup. Including your lib-versioning and symlinking.

You can test with example build:

git clone https://github.com/cguentherTUChemnitz/qtcsv
cd ./qtcsv
mkdir build && cd ./build
cmake ..
make

I added two options, to handle the shared or static lib compilations, as well as the testing as a post-build step. This can be switched by:

cmake .. -DSHARED_LIB=OFF -DBUILD_AND_RUN_TESTS=OFF

I added also the installations commands. The default installation prefix is /usr/local for cmake. The installation can be started with:

make install

iamantony commented 7 years ago

Hi @cguentherTUChemnitz ! This is very good idea. I thought to add support of cmake, but unfortunately I'm not familiar with it.

Several thoughts on your code:

  1. Shared and static Maybe instead of one option SHARED_LIB we could use two options: SHARED_LIB and STATIC_LIB. So user can build either one of the lib variants (shared or static) or both of them at the same time. And by default cmake will set option for shared library.

  2. Qt4 and Qt5 In cmake file you set Qt5Core package as required. But what if I want to build qtcsv with Qt4?

cguentherTUChemnitz commented 7 years ago

Yeah i understand your thoughts.

  1. shared and static i agree with your point and implemented special handling of building both or only one at once.
  2. Qt4 and Qt5 This point is a good question. The found_package statement has different package names for Qt5 and Qt4. Even the imported targets have different names with Qt5::Core or Qt4::Core. So i do not know your desired behavior in detail. Should i expose additional cmake options to switch between the find_package statements of Qt4 and Qt5, or should i search first for Qt5 and if its not found then i search for Qt4? But what is when the user has both Qt versions installed and he would build for both or one specific? What is here the specific desired behavior?
iamantony commented 7 years ago
  1. shared and static Yeah, I saw you commit. Do not tested it yet, but looks good.

  2. Qt4 and Qt5 Let's expose two new options: QT4 and QT5. By default they are both OFF.

    • if user does not specify any of the options, then we set QT5 = ON and search fot Qt5::Core
    • if user specifies one of the options, then we search for specified QtX::Core
    • if user specifies both of the options, then we print error message like "Please specify only one option"
cguentherTUChemnitz commented 7 years ago

I simplified a bit your Qt4 and Qt5 configuration suggestion.

I implemented only one option "USE_QT4", which defaults to OFF. In the disabled / default case Qt5 is used. We do not have to check against double definition of Qt Version, because only one option exists. If the user explicitly want to set either to Qt4 or Qt5, than he can switch by -DUSE_QT4=ON or -DUSE_QT4=OFF.

iamantony commented 7 years ago

I merged your commits into dev branch. But work on cmake support still in progress... Thank you for your time and code!)