Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,23 @@
*.app

# Actual Project Executables

# Created by https://www.gitignore.io/api/cmake

### CMake ###
CMakeCache.txt
CMakeFiles
CMakeScripts
Testing
Makefile
cmake_install.cmake
install_manifest.txt
compile_commands.json
CTestTestfile.cmake
build*


# End of https://www.gitignore.io/api/cmake

# Expected test results
!test-high/expected/*.out
54 changes: 54 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
language: cpp

matrix:
include:
- name: "Linux (gcc)"
os: linux
compiler: gcc
addons:
apt:
packages: valgrind
script:
- ctest --verbose --output-on-failure --script cmake/ctest-gcc.cmake
before_deploy:
- mkdir build
- cd build
- cmake -D CMAKE_BUILD_TYPE=Release ..
- cd ..
- cpack -G ZIP --config cmake/cpack-linux.cmake
deploy:
provider: releases
api_key: ${GITHUB_TOKEN}
file_glob: true
file: "*.zip"
skip_cleanup: true
on:
tags: true

- name: "Linux (clang)"
os: linux
compiler: clang
script:
- ctest --verbose --output-on-failure --script cmake/ctest-clang.cmake

- name: "OSX"
os: osx
script:
- ctest --verbose --output-on-failure --script cmake/ctest-osx.cmake

- name: "Windows"
os: windows
before_script:
- mkdir build
- cd build
- cmake ..
- cmake --build .
script:
- cmake --build . --target RUN_TESTS

allow_failures:
- os: windows
fast_finish: true

notifications:
email: false
9 changes: 9 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
cmake_minimum_required(VERSION 3.8.0)

project(pcg-super)

add_subdirectory(include)
add_subdirectory(sample)

enable_testing()
add_subdirectory(test-high)
Comment on lines +6 to +9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be guarded like so
https://cliutils.gitlab.io/modern-cmake/chapters/testing.html

And building the samples should be an option, perhaps an option for installing them too.

Copy link
Author

@aytekinar aytekinar Feb 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if you had time to read through the comments in the pull request thread. It's been almost a year since I made this pull request, and hence, I have forgot most of the code base. Yet, these concerns were discussed back then, and the summary could be found at least under this comment.

Have you tried, for instance

~> git clone https://github.com/aytekinar/pcg-cpp /tmp/pcg-cpp
~> cd /tmp/pcg-cpp
/tmp/pcg-cpp> git checkout 43-cmake
/tmp/pcg-cpp> mkdir build install
/tmp/pcg-cpp> cd build
# only install the header and cmake files
/tmp/pcg-cpp/build> cmake -D CMAKE_INSTALL_PREFIX=../install ../include
/tmp/pcg-cpp/build> cmake --build . && cmake --build . --target install
# super-build, i.e., build sample files as well as tests, and install both header, cmake and sample files
/tmp/pcg-cpp/build> rm -rf *
/tmp/pcg-cpp/build> cmake -D CMAKE_INSTALL_PREFIX=../install ../
/tmp/pcg-cpp/build> cmake --build . && cmake --build . --target install
# if you want to test
/tmp/pcg-cpp/build> ctest

Basically, I had followed Daniel Pfeifer's talk, which is also listed in "Effective Modern CMake" link under https://cliutils.gitlab.io/modern-cmake/.

IMHO, the simplest solution with CMake is not to use options at all. This is also proposed/suggested by many different people. If you do not want to install/test anything, simply make an out-of-source build of include. No need to remember any options, etc. If you want to include this project as a submodule, do so and point in your CMake to this include folder (e.g., add_subdirectory(external/pcg-cpp/include)).

39 changes: 0 additions & 39 deletions Makefile

This file was deleted.

54 changes: 39 additions & 15 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# PCG Random Number Generation, C++ Edition

[![Build Status][unix-img]][unix-link] ![License][license-img]

[unix-img]: https://img.shields.io/travis/com/imneme/pcg-cpp/master.svg
[unix-link]: https://travis-ci.com/imneme/pcg-cpp
[license-img]: https://img.shields.io/badge/license-MIT%2FApache--2.0-blue.svg

[PCG-Random website]: http://www.pcg-random.org

This code provides an implementation of the PCG family of random number
Expand All @@ -24,22 +30,40 @@ Visit [PCG-Random website] for information on how to use this library, or look
at the sample code in the `sample` directory -- hopefully it should be fairly
self explanatory.

## Building
## Building, Testing and Installing

The code is written in C++11, as an include-only library (i.e., there is
nothing you need to build). There are some provided demo programs and tests
however. On a Unix-style system (e.g., Linux, Mac OS X) you should be able
to just type

make

To build the demo programs.

## Testing

Run

make test
nothing you need to build).

There are some provided demo programs and tests however. We provide a
[CMake](https://cmake.org/)-based building and testing functionality. If you
do not have CMake installed on your system, you can
[download](https://cmake.org/download/) the latest version. Once installed, you
can run

```bash
mkdir build
cd build
cmake ../
cmake --build .
cmake --build . --target test
cmake --build . --target install
```

Above, CMake creates necessary files inside the `build` folder, builds the demo
programs, performs the tests, and finally installs the header files and demo
programs to the `include` and `bin` folders, respectively, in the default system
path. You can append `-D CMAKE_INSTALL_PREFIX=<folder_name>` to change the
default (system) path. For instance, if you would like to install the files in
the `install` folder under the current path, simply run

```bash
mkdir build install
cd build
cmake -D CMAKE_INSTALL_PREFIX=../install ../
cmake --build .
cmake --build . --target install
```

## Directory Structure

Expand All @@ -48,5 +72,5 @@ The directories are arranged as follows:
* `include` -- contains `pcg_random.hpp` and supporting include files
* `test-high` -- test code for the high-level API where the functions have
shorter, less scary-looking names.
* `sample` -- sample code, some similar to the code in `test-high` but more
* `sample` -- sample code, some similar to the code in `test-high` but more
human readable, some other examples too
10 changes: 10 additions & 0 deletions cmake/cpack-linux.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
include("build/CPackConfig.cmake")

set(CPACK_INSTALL_CMAKE_PROJECTS
"build;pcg-super;ALL;/"
)

set(CPACK_PACKAGE_FILE_NAME pcg-linux)
set(CPACK_PACKAGE_DESCRIPTION_SUMMARY
"PCG Random Number Generation, C++ Edition"
)
16 changes: 16 additions & 0 deletions cmake/ctest-clang.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
set(ENV{CC} "clang")
set(ENV{CXX} "clang++")

set(CTEST_PROJECT_NAME "pcg")
set(CTEST_SOURCE_DIRECTORY ".")
set(CTEST_BINARY_DIRECTORY "build-test-clang")
set(CTEST_CMAKE_GENERATOR "Unix Makefiles")

ctest_start("Continuous")
ctest_configure()
ctest_build()
ctest_test(RETURN_VALUE SUCCESS)

if(NOT ${SUCCESS} EQUAL 0)
message(SEND_ERROR "Testing failed.")
endif()
22 changes: 22 additions & 0 deletions cmake/ctest-gcc.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
set(ENV{CC} "gcc")
set(ENV{CXX} "g++")
set(ENV{CXXFLAGS} "--coverage")

set(CTEST_PROJECT_NAME "pcg")
set(CTEST_SOURCE_DIRECTORY ".")
set(CTEST_BINARY_DIRECTORY "build-test-gcc")
set(CTEST_CMAKE_GENERATOR "Unix Makefiles")

set(CTEST_COVERAGE_COMMAND "/usr/bin/gcov")
set(CTEST_MEMORYCHECK_COMMAND "/usr/bin/valgrind")

ctest_start("Continuous")
ctest_configure()
ctest_build()
ctest_test(RETURN_VALUE SUCCESS)
ctest_coverage()
ctest_memcheck()

if(NOT ${SUCCESS} EQUAL 0)
message(SEND_ERROR "Testing failed.")
endif()
13 changes: 13 additions & 0 deletions cmake/ctest-osx.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
set(CTEST_PROJECT_NAME "pcg")
set(CTEST_SOURCE_DIRECTORY ".")
set(CTEST_BINARY_DIRECTORY "build-test-osx")
set(CTEST_CMAKE_GENERATOR "Unix Makefiles")

ctest_start("Continuous")
ctest_configure()
ctest_build()
ctest_test(RETURN_VALUE SUCCESS)

if(NOT ${SUCCESS} EQUAL 0)
message(SEND_ERROR "Testing failed.")
endif()
52 changes: 52 additions & 0 deletions include/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
cmake_minimum_required(VERSION 3.8.0)

project(pcg VERSION 0.98.1 LANGUAGES CXX)
Comment on lines +1 to +3
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the root CMakeLists.txt needs this. I also think having a separate CMakeLists.txt for the include directory for a header only library to be a bit overkill.

Copy link
Author

@aytekinar aytekinar Feb 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, include is the "root" directory for the project. If you read the previous comments in the thread, my suggestion was to have include under yet another directory such as src. We can discuss the name, perhaps have include under pcg, etc., but the most important thing is to have a separate project directory.

Having more than 1 CMakeLists.txt files does not hurt anyone. README.md can easily document what you need to do. If you want to install the headers, run

mkdir build
cd build
cmake ../include # or, cmake ../pcg, cmake ../src, whatever that folder would be
cmake --build . && cmake --build . --target install

If you want to also install sample files, do the super build:

mkdir build
cd build
cmake ../
cmake --build && cmake --build . --target install

At the end of the day, you simplify the logic.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I just find it weird since I've not seen any header only library separate them like that.


We can discuss the name, perhaps have include under pcg, etc.

This should probably be raised as an issue and resolved before this PR is merged.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.


include(CMakePackageConfigHelpers)
include(GNUInstallDirs)

add_library(pcg INTERFACE)
add_library(pcg::pcg ALIAS pcg)

target_compile_features(pcg INTERFACE cxx_std_11)

target_include_directories(pcg
INTERFACE
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
)

install(TARGETS pcg EXPORT pcg)

install(EXPORT pcg
FILE pcg-targets.cmake
NAMESPACE pcg::
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/pcg
)

configure_package_config_file(pcg-config.cmake.in
${CMAKE_CURRENT_BINARY_DIR}/pcg-config.cmake
INSTALL_DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/pcg
)

write_basic_package_version_file(pcg-config-version.cmake
COMPATIBILITY SameMajorVersion
)

install(
FILES
${CMAKE_CURRENT_BINARY_DIR}/pcg-config.cmake
${CMAKE_CURRENT_BINARY_DIR}/pcg-config-version.cmake
DESTINATION
${CMAKE_INSTALL_LIBDIR}/cmake/pcg
)

install(
DIRECTORY
${CMAKE_CURRENT_SOURCE_DIR}/
DESTINATION
${CMAKE_INSTALL_INCLUDEDIR}
FILES_MATCHING PATTERN *.hpp
)

include(CPack)
4 changes: 4 additions & 0 deletions include/pcg-config.cmake.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
@PACKAGE_INIT@
# include(CMakeFindDependencyMacro)
# find_dependency(XXX)
include("${CMAKE_CURRENT_LIST_DIR}/pcg-targets.cmake")
39 changes: 0 additions & 39 deletions sample/.gitignore

This file was deleted.

Loading