Skip to content

Conversation

@rlalik
Copy link
Contributor

@rlalik rlalik commented Jun 5, 2023

This is mainly rework of the build system to simplify things and take advantage of modern cmake features.

Significant changes:

  • Split sources/headers to separate src/inlude dirs
  • Use cmake macros to create plugins instead of operating with string variables
  • Install in GNU-like manner, install all headers and plugin headers, install also macros
  • Installation creates also cmake-like targets for cmake projects

rlalik added 3 commits June 23, 2023 20:46
* Split sources/headers to separate src/inlude dirs
* Use cmake macros to create plugins instead of operating with string variables
* Install in GNU-like manner, install all headers and plugin headers, install also macros
* Installation creates also cmake-like targets for cmake projects
@rlalik rlalik force-pushed the fix_build_system branch from 210e6ea to 6ff3b1b Compare June 23, 2023 18:50
@rlalik
Copy link
Contributor Author

rlalik commented Jun 23, 2023

This PR fixes many issues with cmake build system although breaks one thing. Before $PLUTO_DIR was the directory where libPluto.so was to be found. But it wasn't following general Linux schema of install dirs. This PR install libPluto.so into $PLUTODIR/lib or $PLUTODIR/lib64. What is nice with this is that, for Hades one can set PLUTO_DIR=$MYHADDIR. And it will work. Or, replace LIB_DIR += $PLUTO_DIR with LIB_DIR += $PLUTO_DIR/lib{64}. This will make it consistent with typical install systems. It all assumes that PLUTO_DIR points to installation dir, not build source/build location like often it was done before. Source/Build/Installation separation should always be used. Now it is.

Another fix is to make plugins actually installable. Old make install was only installing public headers into $PLUTODIR/include (here the correct GNU dir was used). Now, also plugin headers are installed into $PLUTODIR/include/plugins. In the user code plugins should be included with distinct plugin path, e.g. #include <plugins/PStrangenessPlugin.h> (however, with cmake projects it will work also without).

I fixed the build system also, like:

  • removed GLOB with is strongly discouraged and the explicit file names are given
  • separated src/include locations for core sources and plugins, it follows the rule that include contains the public installable headers, and in the future we can place, if needed, private headers and sources in appropriate directories.
  • there is a cmake macro to handle the plugins
  • exported cmake Config/Targets files which properly import Pluto into other cmake projects.

Example use of new build system:

export PLUTO_DIR=/cvfms/.../Pluto-6.2.0
mkdir build && cd build
cmake .. -DCMAKE_INSTALL_PREFIX=$PLUTO_DIR -DCMAKE_CXX_STANDARD=14 # or other ROOT standard, as the new ROOT are not properly passing the expected standard but relay on compile features
make install -j30

@PlutoUser
Copy link
Owner

I think we should first conclude the Bugfixes, then make a bugfix release and come finally to the new Features. For the new build System I have to check if it is still working with the QA Script. This is quite important.

This was referenced Jun 23, 2023
@rlalik
Copy link
Contributor Author

rlalik commented Jun 25, 2023

From my side only already emerged #7 and #8 were bug fixes. This and the #6 and #9 are new features and they can go to the new release.

@PlutoUser
Copy link
Owner

I will continue working on this one... right?

@rlalik
Copy link
Contributor Author

rlalik commented Jul 11, 2023

Go ahead

@PlutoUser
Copy link
Owner

I think one should still allow an in-source build. With your new build system the includes of the plugins are copied to include/plugins, and the macros are copied to share. So things are duplicated.

Also, I don't think that "share" is the best wording, as it is not where people who are used to it are expecting the macros

@PlutoUser
Copy link
Owner

Something changed already:
Warning in <PStaticData::GetParticleKey>: id 70 not found
I see that the order of plugins (and also the order of allocated decays) changed, so this must be investigated first. Maybe the change of the order unraveled something

@PlutoUser PlutoUser merged commit c3b70c3 into PlutoUser:master Jul 12, 2023
@rlalik
Copy link
Contributor Author

rlalik commented Jul 12, 2023

I think one should still allow an in-source build. With your new build system the includes of the plugins are copied to include/plugins, and the macros are copied to share. So things are duplicated.

Also, I don't think that "share" is the best wording, as it is not where people who are used to it are expecting the macros

If the installation would go into standard linux location (some users may want this) then putting macros somewhere else than share would be weird and mess with the directory structure. But one can introduce something like STANDALONE mode (or use LEGACY mode which already exists) which install it into separate location and then macros can be on the / level with include/ and lib/ and so on.

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