p4est
p4est copied to clipboard
Continue CMake build system
Description
The CMake build system is supported on branch develop, but not yet complete.
Proposed solution
- The archive created by
cpack
is lacking some.c
source files as well as example and test. - The archive created by
cpack
shall include all files required to run./bootstrap && configure && make -j check V=0
in the unpacked tree. - Generally, the
cpack
archive should be as similar as possible to themake dist
tarball by automake.
Additionally:
- Add a new CI job that runs cpack, unpacks the archive und then runs the autotools chain.
- Add a new CI job that calls make dist, unpacks and then builds, tests and packs using cmake.
Another point:
- The tests on make check run with 2 MPI ranks. Make ctest behave the same if not so.
A couple more items to align the CMake to the automake build:
- Currently, make install without explicit prefix installs into sc/local. Change to local.
- When installing and sc is a submodule, the install of sc should go into the p4est install prefix, not under sc.
- The CMake build creates libp4est.a, libp6est.a and libp8est.a. To align with autotools, it should be all in libp4est.a.
-
make install
creates uppercase SC and P4EST files/directories. Preferred convention would be lowercase.
I think to make "examples" be a full test of the "installed" p4est from a single project instead of two separate projects as I have now would require making CMake be a "superbuild" where a new top-level CMakeLists.txt contains two ExternalProjects--one for p4est, one for p4est examples.
The reason for examples as a separate project or subproject is to test that CMake packaging is correct--it is a common pitfall to have problems with linking, compiling or finding the installed library due to something wrong with p4est CMake package. These problems only show up when treating p4est as a separate package upon a separate project or subproject, but are how virtually all non-p4est-dev users will use p4est.
For the lowercase SC/P4EST install subdirectory naming, yes I think that can be done with a few extra lines of code
To make one libp4est.{a,so} regardless of whether or not p8est,p6est are also built can be done a couple different ways. To do it most like a typical Makefile:
add_library(p8est OBJECT ...)
add_library(p6est OBJECT ...)
...
target_link_libraries(p4est PRIVATE p6est p8est)
this creates object files for building into p4est, like a traditional Makefile.
For legacy compatibility reasons, target_link_libraries
is overloaded in cases like this to mean "transitively include this object file and its associated CMake properties" instead of strictly "linking"
There might be a couple additional touchups as I might assume in some other places (like for install/packaging) that there are up to 3 library files instead of only 1.
To specifically control tests MPI rank, lines like
${MPIEXEC_EXECUTABLE} ${MPIEXEC_NUMPROC_FLAG} ${Ncpu} $<TARGET_FILE: ...
become like (assuming fixed rank 2 is desired)
${MPIEXEC_EXECUTABLE} ${MPIEXEC_NUMPROC_FLAG} 2 $<TARGET_FILE: ...
I think to make "examples" be a full test of the "installed" p4est from a single project instead of two separate projects as I have now would require making CMake be a "superbuild" where a new top-level CMakeLists.txt contains two ExternalProjects--one for p4est, one for p4est examples.
The reason for examples as a separate project or subproject is to test that CMake packaging is correct--it is a common pitfall to have problems with linking, compiling or finding the installed library due to something wrong with p4est CMake package. These problems only show up when treating p4est as a separate package upon a separate project or subproject, but are how virtually all non-p4est-dev users will use p4est.
Sounds all very good -- removed change of examples from issue.
* The tests on make check run with 2 MPI ranks. Make ctest behave the same if not so.
This is done.
Just noting some differences before I forget: the test programs shall be named p4est_test_partition, p8est_test_partition, etc., and cmake links static by default while autotools link dynamic by default. What to do about it we shall consider another day.
I am using the p4est develop branch right now and I was wondering if there was a way to build the source files inside of p4est/examples ? The CMakeLists inside there has problems building it and im not sure if there's something I need to add in order to get them to build.
The develop branch is current again. If you think the examples are not built, please feel free to have them built by default, the same way it is done with the autoconf build.
I've updated the p4est tests such that they survive if P4EST_HAVE_ZLIB or SC_HAVE_ZLIB are not defined.
The new downside is, however, that a missing ZLIB define now goes unnoticed. Last I've seen, there are several CMake builds in the CI that appear to not set these defines. This would be nice to have fixed so the defines are always consistent.
* Add a new CI job that runs cpack, unpacks the archive und then runs the autotools chain. * Add a new CI job that calls make dist, unpacks and then builds, tests and packs using cmake.
This would be the ultimate test; cc @dutkalex for reference.
I am using the p4est develop branch right now and I was wondering if there was a way to build the source files inside of p4est/examples ? The CMakeLists inside there has problems building it and im not sure if there's something I need to add in order to get them to build.
The examples in both sc and p4est should be built by default on make. Only the tests are built separately and run by make check. If this is not so with the current CMake builds, please feel free to fix.
Just noting some differences before I forget: the test programs shall be named p4est_test_partition, p8est_test_partition, etc., and cmake links static by default while autotools link dynamic by default. What to do about it we shall consider another day.
The file names should be aligned to those generated by automake. About static/dynamic builds, I'm fine using the CMake defaults and only override when specified explicitly.