acts-project / acts

Experiment-independent toolkit for (charged) particle track reconstruction in (high energy) physics experiments implemented in modern C++
https://acts.readthedocs.io
Mozilla Public License 2.0
104 stars 168 forks source link

`common` CMake preset requires ccache #3537

Open stephenswat opened 2 months ago

stephenswat commented 2 months ago

I've been trying to use the github-ci preset as a sort of "build everything, so if it works locally it will most likely pass the CI" build. However, I have noticed that this sets ccache as the compiler launcher. Makes sense, of course, for a CI preset. However, it appears that ccache is required in the common preset, from which all others inherit.

In my mind, it would be useful to have an "everything" preset for local builds, which does not require ccache. Given the multiple inheritance that CMake presets allow, I'd suggest we could have a ccache preset which sets ccache, and then common could have the launcher flag removed. We could then also have some kind of everything flag which would just build everything, like the ACTS_BUILD_EVERYTHING flag used to do.

Looking to see if anyone has any opinions on this so we can come to a nice compromise that everyone agrees with.

stephenswat commented 2 months ago

For the record, my current workaround for local builds is this:

cmake -B build -S . --preset github-ci -DCMAKE_CXX_COMPILER_LAUNCHER=""
paulgessinger commented 2 months ago

@andiwand

Also, fyi, you can add a CMakeUserPresets.json with custom presets that can extend the original ones.

andiwand commented 2 months ago

Sounds good to me. I put it there because we (or maybe I) use it everywhere. In the CI and local (since I have multiple ACTS build folder).

I didn't know that there is multiple inheritance which felt like a limitations to me. If you have a nice separation in mind I am happy to review!

paulgessinger commented 2 months ago

I think the CI mode also enables -Werror, which you might not want for local development.

github-actions[bot] commented 1 month ago

This issue/PR has been automatically marked as stale because it has not had recent activity. The stale label will be removed if any interaction occurs.