-
Notifications
You must be signed in to change notification settings - Fork 115
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
3d examples are not built by cmake #206
Comments
With autoconf, we have --enable-2d and --enable-3d. These are default on but can be switched independently. Shall we add the analogous options to CMake? |
currently, as I understand, 2d is always built, 3d and p6est can be enabled/disabled on the cmake configure command line via option Another point is that, now a project that use p4est (via cmake find_package) is aware if p4est was or wasn't built with p8est enabled, because P4EST_ENABLE_P8EST is always defined. Additionnally, in PR #207 I modified also the generated pkgconfig, so that users that rely on pkgconfig to find p4est can also easily have access to the information, e.g. pkg-config --variable p4est_enable_p8est p4est currently this variable is a boolean, that can be either ON/OFF, but it could be 0/1; what do you think ? |
This sounds nice. Would you be able to put this logic into the pkgconfig file, independent of the build system used? |
This is currently possible, but a few minor adjustments need to be made for enabling that in a robust way. |
Just to let you know, I checked if it was possible e.g. to build p4est with cmake, using libsc build with autotools, already installed. One problem emerged: if libsc is built with mpi enabled, but the use tries to build p4est with mpi OFF, currently the build fails, because the mpi.h header is not found. The reason is that, p4est top-level CMakeList.txt only activate MPI ipon upon option "mpi", but it should also activate MPI if the libsc found in environment was itself built with MPI activated. I'll make a PR about that in libsc and p4est soon. Finally, this illustrates the need for libsc to expose its build features in the pkg-config file, as well as in SCConfig.cmake file |
One problem emerged: if libsc is built with mpi enabled, but the use tries to build p4est with mpi OFF, currently the build fails, because the mpi.h header is not found. The reason is that, p4est top-level CMakeList.txt only activate MPI ipon upon option "mpi", but it should also activate MPI if the libsc found in environment was itself built with MPI activated.
I'll make a PR about that in libsc and p4est soon.
We check that the *_ENABLE_MPI and *_ENABLE_DEBUG options are the same.
I think it is ok to require this, it just needs to be well documented.
|
What I meant, is that:
|
I like the proposition to put the MPI and DEBUG status into pkgconfig (both build systems) and SCConfig.cmake. I would prefer that no options are auto-enabled. If there is a mismatch, we can print a helpful message and suggest to the user how to reconfigure their build. |
It is possible to build only 3d with autoconf. It's a nice experiment for consistency.
I'd use whatever is the standard form of a boolean in pkgconfig. The values from autoconf are yes/no, is that compatible? Otherwise, we may need to translate to true/false or 0/1. Shall we thus add p4est_enable_p4est=yes in CMake, and with autoconf p4est, p8est as appropriate? With autoconf, p6est is automatically chosen as the logical "and" of enable-2d, enable-3d, and enable-p6est, and with CMake it becomes equal to enable_p8est. This way the pkgfiles converge even more between the build systems. |
Hi may I ask on whether we are with this issue? I know we had quite a few merges towards this. |
Let me ask @mkirilin who has been looking into the example build rules of CMake. Is there anything that remains to be done, or shall we close this issue? |
At the current state of the develop branch 8dfa831, with cmake, all the 2d examples always build by default and 3d examples build depending on |
At the current state of the develop branch 8dfa831, with cmake, all the 2d examples always build by default and 3d examples build depending on `enable_p8est` flag which is ON by default as well. I think the issue might be closed.
Thanks for adding this update. Shall we look at the pkgconfig files and
verify that the contents are what we want for both autoconf and cmake?
|
Description
Currently in
example/CMakeLists.txt
, 3d examples are only built when targetP4EST::P8EST
is defined.The problem is that this target is never defined in p4est, because p8est sources are compiled as an object library, and then directly added to p4est target sources.
So currently, 3d example can't be built by cmake.
This commit 523e736 removed target P4EST::P8EST
Proposed solution
As enabling / disabling p8est / p6est are features that can be enabled/disabled at top-level via cmake options, I think we should also export this information in
cmake/config.cmake.in
so that e.g. variable P4EST_ENABLE_P8EST is defined (true or false) when a downstream cmake project (like example pcmake project) is doingfind_package(P4EST CONFIG)
and use that variable to decide if 3d examples can be built.The text was updated successfully, but these errors were encountered: