acts-project / algebra-plugins

Mozilla Public License 2.0
3 stars 10 forks source link

External Build Re-Design, main branch (2023.05.25.) #104

Closed krasznaa closed 11 months ago

krasznaa commented 1 year ago

I'm absolutely opening a can of worms with this one, but let's see where it goes...

With CMake 3.24 a new behaviour was introduced for FetchContent and find_package. They now implement a very similar logic for building or finding a needed external to what we implemented in these R&D projects. (And in Acts itself.)

Basically, FetchContent can either build an external dependency as it did before, or it can look for it with find_package. The default behaviour of the code is to silently look for the package first, and if it's found in the user's build environment, the found version is used. While if it's not found, it is built instead. But one can also force the build to either always look for an existing external, or to always build it. See: FETCHCONTENT_TRY_FIND_PACKAGE_MODE

The behaviour of the code is not really changed with all these modifications. The reasons that doing these modifications could be a good idea are the following:

But one of the main things that sold this for me is how one would be able to work on multiple projects at the same time in the new setup. With our current setup one needs to for instance:

In this new setup one can instead clone vecmem and this project side-by-side, and when setting up the build of this project, use -DFETCHCONTENT_SOURCE_DIR_VECMEM=<foo> to tell the build to pick up vecmem from that location. So all of a sudden one can modify both projects freely, and just issue make commands in a single build directory.

But... Because there is a big but... :frowning: We don't use CMake 3.24+ in our Docker images at the moment. :frowning: So to make all this work, we would need to put in a fair amount of effort... :thinking: So we may want to think it through properly before committing to it. Even if I think this could be helpful in the long term.

krasznaa commented 1 year ago

I like the fact that this reduces the number of configuration parameters, but does this mean that it is no longer possible to disable the remote fetching of packages (because ALGEBRA_PLUGINS_USE_SYSTEM_LIBS is gone)? worried

The direct replacement for -DALGEBRA_PLUGINS_USE_SYSTEM_LIBS=TRUE is: -DFETCHCONTENT_TRY_FIND_PACKAGE_MODE=ALWAYS.

Though I myself am also worried a bit about the lack of individual settings. :frowning: We can only force this behaviour globally now. But with FETCHCONTENT_SOURCEDIR\<FOO> I believe we would still have enough flexibility for all the use cases that we had so far.

wermos commented 11 months ago

The direct replacement for -DALGEBRA_PLUGINS_USE_SYSTEM_LIBS=TRUE is: -DFETCHCONTENT_TRY_FIND_PACKAGE_MODE=ALWAYS.

You can still keep the option ALGEBRA_PLUGINS_USE_SYSTEM_LIBS by simply setting the FETCHCONTENT_TRY_FIND_PACKAGE_MODE to ALWAYS whenever the former is passed on the command line 🤔

krasznaa commented 11 months ago

Let me explicitly close this one. In the end I don't think we need to fundamentally re-think the build setup at this stage of the project.

Once things get included into Acts, we can come back to optimising the build setup. 🤔