search

scikit-hep / fastjet

Jet-finding in the Scikit-HEP ecosystem.
https://fastjet.readthedocs.io
BSD 3-Clause "New" or "Revised" License
21 stars 13 forks source link

Abandon fastjet's build system and rewrite it in CMake #310

Open matthewfeickert opened 3 months ago

matthewfeickert commented 3 months ago

I think the time has come to CMake fastjet and submit a PR.

Originally posted by @lgray in https://github.com/scikit-hep/fastjet/issues/301#issuecomment-2189541168

While I'm not sure if the FastJet authors would be on-board with this, we would have the ability to just add CMake support which people can then use as they want I don't think that would require a restructure?

matthewfeickert commented 2 months ago

I see that there's something at least CMake related already on https://gitlab.com/fastjet/fastjet/-/merge_requests/4 which seems to be adding cmake.in configure files for downstream use. I think(?) that would be helpful for builds here, as that would allow for pulling in the FastJet config from a top level CMakeLists.txt file, but I would assume that it would still be good to have FastJet just support CMake in general.

@henryiii thoughts here?

matthewfeickert commented 2 months ago

An email response from the FastJet team RE: the idea of a contribution of adding CMake support:

It's something we've discussed as being of interest, but you guessed correctly regarding bandwidth! The main issue we see is that there are quite a few options in the current build system, and replicating them all would take some work and validation. Added to that, even though we're using CMake in other projects, I wouldn’t say we're as proficient in it as we would like to be. If at some point some of you are interested in contributing on this front, we'd be happy to try it out and give our thoughts.

matthewfeickert commented 2 months ago

First thing would probably be to migrate https://gitlab.com/fastjet/siscone to CMake and then to step through and migrate all the FastJet plugins https://gitlab.com/fastjet/fastjet/-/tree/master/plugins?ref_type=heads. Once those all work then FastJet itself could be migrated.

matthewfeickert commented 2 months ago

PR https://github.com/scikit-hep/fastjet/pull/4 provides a good summary (especially this bit https://github.com/scikit-hep/fastjet/pull/4#issuecomment-824377855) of why this is needed as well and then also overviews why cgal is currently installed in such a terrible manner.

https://github.com/scikit-hep/fastjet/blob/28ec47c78d5042142278cecbc09d1acec0c8a34e/setup.py#L52-L88