p4est icon indicating copy to clipboard operation
p4est copied to clipboard

Continue CMake build system

Open cburstedde opened this issue 3 years ago • 14 comments

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 the make dist tarball by automake.

cburstedde avatar Mar 12 '21 13:03 cburstedde

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.

cburstedde avatar Mar 12 '21 13:03 cburstedde

Another point:

  • The tests on make check run with 2 MPI ranks. Make ctest behave the same if not so.

cburstedde avatar Mar 16 '21 17:03 cburstedde

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.

cburstedde avatar Mar 23 '21 09:03 cburstedde

  • make install creates uppercase SC and P4EST files/directories. Preferred convention would be lowercase.

cburstedde avatar Apr 28 '21 11:04 cburstedde

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.

scivision avatar Apr 29 '21 14:04 scivision

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.

scivision avatar Apr 29 '21 14:04 scivision

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: ...

scivision avatar Apr 29 '21 14:04 scivision

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.

cburstedde avatar Apr 29 '21 14:04 cburstedde

* The tests on make check run with 2 MPI ranks. Make ctest behave the same if not so.

This is done.

cburstedde avatar Aug 31 '22 20:08 cburstedde

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.

cburstedde avatar Sep 30 '22 18:09 cburstedde

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.

PatrickThomasma avatar Dec 22 '22 16:12 PatrickThomasma

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.

cburstedde avatar Dec 18 '23 19:12 cburstedde

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.

cburstedde avatar Dec 18 '23 19:12 cburstedde

* 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.

cburstedde avatar Feb 15 '24 09:02 cburstedde

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.

cburstedde avatar Feb 15 '24 09:02 cburstedde

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.

cburstedde avatar Feb 15 '24 09:02 cburstedde