Skip to content
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

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

Open
matthewfeickert opened this issue Aug 15, 2024 · 4 comments
Open

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

matthewfeickert opened this issue Aug 15, 2024 · 4 comments

Comments

@matthewfeickert
Copy link
Member

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

Originally posted by @lgray in #301 (comment)

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
Copy link
Member Author

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
Copy link
Member Author

matthewfeickert commented Sep 11, 2024

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
Copy link
Member Author

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
Copy link
Member Author

matthewfeickert commented Sep 12, 2024

PR #4 provides a good summary (especially this bit #4 (comment)) of why this is needed as well and then also overviews why cgal is currently installed in such a terrible manner.

fastjet/setup.py

Lines 52 to 88 in 28ec47c

zip_filename = DIR / pathlib.Path(CGAL_ZIP).parts[-1]
with urllib.request.urlopen(CGAL_ZIP) as http_obj:
with open(zip_filename, "wb") as file_obj:
shutil.copyfileobj(http_obj, file_obj)
with zipfile.ZipFile(zip_filename) as zip_obj:
cgal_dir = DIR / zip_obj.namelist()[0]
zip_obj.extractall(DIR)
# Patch for segfault of LimitedWarning
# For more info see https://github.com/scikit-hep/fastjet/pull/131
subprocess.run(
["patch", "src/ClusterSequence.cc", DIR / "patch_clustersequence.txt"],
cwd=FASTJET,
)
# RPATH is set for shared libraries in the following locations:
# * fastjet/
# * fastjet/_fastjet_core/lib/
# * fastjet/_fastjet_core/lib/python*/site-packages/
_rpath = "'$$ORIGIN/_fastjet_core/lib:$$ORIGIN:$$ORIGIN/../..'"
env = os.environ.copy()
env["PYTHON"] = sys.executable
env["PYTHON_INCLUDE"] = f'-I{sysconfig.get_path("include")}'
env["CXXFLAGS"] = "-O3 -Bstatic -lgmp -Bdynamic -std=c++17"
env["LDFLAGS"] = env.get("LDFLAGS", "") + f" -Wl,-rpath,{_rpath}"
env["ORIGIN"] = "$ORIGIN" # if evaluated, it will still be '$ORIGIN'
args = [
f"--prefix={OUTPUT}",
"--enable-thread-safety",
"--disable-auto-ptr",
"--enable-allcxxplugins",
"--enable-cgal-header-only",
"--enable-cgal",
f"--with-cgaldir={cgal_dir}",

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

No branches or pull requests

1 participant