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

Add initial CMake build support. #185

Closed
wants to merge 7 commits into from

Conversation

drewnoakes
Copy link
Contributor

This CMakeLists.txtprovides targets forunittests,perftests` and all example programs.

Note that this file does not copy the bin folder contents to the target directory, which only applies if you do out-of-source builds. In which case you may copy the folder manually, or ideally update CMakeLists.txt to perform this. I didn't manage to get this to work.

It's possible the minimum CMake version required is actually lower than that published, but I cannot test. Users of this file on earlier versions should modify this if earlier versions do in fact work.

To use:

rapidjson$ mkdir out
rapidjson$ cd out
rapidjson/out$ cmake ..
rapidjson/out$ make unittests

This CMake file has also been tested with beta builds of the new CLion IDE and works well.

You may also configure the build using a tool such as ccmake.

rapidjson/out$ ccmake .

@miloyip
Copy link
Collaborator

miloyip commented Oct 31, 2014

Thank you for this. We would like to have CMAKE build.

I am not familiar with CMAKE. Is it possible to do some wildcards of files instead of explicitly listing them? Otherwise it may be easily broken if a file is added or removed, isn't it?

@drewnoakes
Copy link
Contributor Author

That does seem attractive, but it doesn't seem to be a common practice. I think this explains why:

It is tempting to use this command to avoid writing the list of source files for a library or executable target. While this seems to work, there is no way for CMake to generate a build system that knows when a new source file has been added. Normally the generated build system knows when it needs to rerun CMake because the CMakeLists.txt file is modified to add a new source. When the source is just added to the directory without modifying this file, one would have to manually rerun CMake to generate a build system incorporating the new file.

This is pretty minimal as far as CMake configurations go, in my experience.

What other compile flags would you like? For example I expect we should add -Wall and maybe -Wextra and -Werror.

@miloyip
Copy link
Collaborator

miloyip commented Oct 31, 2014

Currently gmake version has -march=native -Wall -Wextra -Werror -Weffc++ -Wswitch-default for unit test.

This CMakeLists.txt provides targets for unittests, perftests and all
example programs.

Note that this file does not copy the `bin` folder contents to the
target directory, which only applies if you do out-of-source builds. In
which case you may copy the folder manually, or ideally update
CMakeLists.txt to perform this. I didn't manage to get this to work.

It's possible the minimum CMake version required is actually lower than
that published, but I cannot test. Users of this file on earlier
versions should modify this if earlier versions do in fact work.

To use:

    rapidjson$ mkdir out
    rapidjson$ cd out
    rapidjson/out$ cmake ..
    rapidjson/out$ make unittests

This CMake file has also been tested with beta builds of the new CLion
IDE and works well.

You may also configure the build using a tool such as `ccmake`.

    rapidjson/out$ ccmake .
-march=native -Wall -Wextra -Werror -Weffc++ -Wswitch-default
@drewnoakes
Copy link
Contributor Author

Some improvements, rebased on top of master.

  • Using the same compiler flags as those @miloyip quoted from gmake
  • Google test now built as a lib using its own CMake file
  • Removal of all header files -- CMake can deduce these dependencies itself

The file is much simpler now, fitting on one screen.

Otherwise it may be easily broken if a file is added or removed, isn't it?

In practice this isn't an issue, so long as someone is using the CMake file. It might be a good idea to have the CI server run an additional build using CMake, just to ensure everything is alright with its configuration.

@miloyip
Copy link
Collaborator

miloyip commented Oct 31, 2014

Can you also help adding instructions in readme?

@spl spl mentioned this pull request Nov 3, 2014
@jollyroger jollyroger mentioned this pull request Nov 5, 2014
7 tasks
@miloyip miloyip closed this Apr 11, 2015
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.

2 participants