Add fastjet

[lgray]

Checklist

• Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
• License file is packaged (see here for an example).
• Source is from official source.
• Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
• If static libraries are linked in, the license of the static library is packaged.
• Build number is 0.
• A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
• GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
• When in trouble, please check our knowledge base documentation before pinging a team.

@conda-forge/help-python
@conda-forge/help-c-cpp

@jpivarski @aryan26roy

[conda-forge-linter]

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/fastjet) and found some lint.

Here's what I've got...

For recipes/fastjet:

• There are 1 too many lines. There should be one empty line at the end of the file.

[conda-forge-linter]

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/fastjet) and found it was in an excellent condition.

[xhochy]

Do you plan to change this to a release version before the release?

[lgray]

The rc's will continue for a few more until a full release version. @jpivarski and @aryan26roy can tell you more about that path.

I'd like to have it in conda from the release candidates since we're already using the package in conda environments via pip (and it's a benefit to have everything in conda).

[xhochy]

Then please use a dev/rc channels as we do in https://github.com/conda-forge/lief-feedstock/blob/dev/recipe/conda_build_config.yaml to mark them as such. The version numbers in conda don't differentiate between release and pre-release. Therefore pre-release should be put into labels.

[xhochy]

@jpivarski Please 👍 to being a maintainer

[jpivarski]

Yes, I +1 to being a maintainer.

[xhochy]

@aryan26roy Please 👍 to being a maintainer

[jpivarski]

@aryan26roy, I suppose we could make a non-RC release in PyPI. Take the current state, do a final RC release, get @lgray's group to test it, and, passing that, do a single non-RC release. This first conda-forge release could be based on that (rather than adding the complication of getting to other conda channels). What does everyone think of that?

[xhochy]

All the complication with RCs is to upload them to a label. This is done by adding the mentioned file with the respective two lines. Happy to merge that already. You can simply remove that file and rerender on the feedstock when you do an actual release. Shouldn't be that much work, user will need to use {conda,mamba} install -c conda-forge/label/fastjet_rc fastjet to install it until (there is no notion of --pre in conda as it is in pip, this is the alternative way to do it).

The choice is up to you all, I'm happy to merge RC+label or actual release-without-a-label.

[lgray]

@xhochy I'm going to go ahead and use the label system for now, it's pretty reasonable.

@jpivarski @aryan26roy this is ok with you? I'm fine to manage the conda repo a bit to avoid pushing releases too quickly.

[aryan26roy]

@lgray . I'm ok with this. This saves us from putting out releases too quickly.

[lgray]

@jpivarski can I get your 👍 as well? Then please feel free to merge, @xhochy!

[jpivarski]

@jpivarski can I get your as well? Then please feel free to merge, @xhochy!

Yes, if the label thing is easy, let's go for it!

[lgray]

Hmmm the osx build is complaining about libtool not existing despite the package very clearly being installed. Any idea @xhochy ?

[conda-forge-linter]

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/fastjet) and found some lint.

Here's what I've got...

For recipes/fastjet:

• Selectors are suggested to take a <two spaces>#<one space>[<expression>] form. See lines [14]

[conda-forge-linter]

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/fastjet) and found it was in an excellent condition.

[lgray]

Hmmm looks like something is still ugly with the RPATHs in conda. The build was finishing successfully on my home machine, ah well.

Any suggestions if @jpivarski or @henryiii have some time to look at it?

[lgray]

@chrisburr thanks!

[lgray]

@chrisburr - it now fails in the build step claiming it cannot find libgmp?

[chrisburr]

That will be a bug in the package's build system as it's finding it successfully during the configure stage:

  checking if /home/conda/staged-recipes/build_artifacts/fastjet_1629816133851/_build_env/bin/x86_64-conda-linux-gnu-c++ -O3 -Bstatic -lgmp -Bdynamic -g  considers atd::auto_ptr as deprecated... yes
  checking if /home/conda/staged-recipes/build_artifacts/fastjet_1629816133851/_build_env/bin/x86_64-conda-linux-gnu-c++ -O3 -Bstatic -lgmp -Bdynamic -g  -Werror=deprecated-declarations supports atd::unique_ptr... yes

[lgray]

Looks like we may want to convince the fastjet authors to switch to cmake sooner rather than later. Fun.

[chrisburr]

This looks suspicious to me: https://github.com/scikit-hep/fastjet/blob/4fb2d3d587da13fad431412773c8ab9a4f1cc078/setup.py#L69

Overriding CXXFLAGS is also prone to causing issues.

[jpivarski]

This looks suspicious to me: https://github.com/scikit-hep/fastjet/blob/4fb2d3d587da13fad431412773c8ab9a4f1cc078/setup.py#L69

Overriding CXXFLAGS is also prone to causing issues.

You're right to be suspicious of that override. :) That was necessary to tell the compiler to build the shared libraries in fastjet-core (everything except the _ext pybind11 module) with an RPATH that includes $ORIGIN, so that they can be moved as long as they're moved together, which has to happen because the Python package may be installed in different absolute paths on different systems. Even setting this $ORIGIN took some finagling because fastjet-core's build system sometimes shell-evaluates that argument and sometimes shell-evaluates the shell-evaluation. Including two dollar signs kept it from being expanded in the first case, defining an environment variable ORIGIN='$ORIGIN' kept it from being expanded in the second case.

When building for PyPI, we only managed to get the Linux build to complete successfully. MacOS is almost possible; @henryiii was going to look at that. Based on our conversations with the fastjet-core authors, building on Windows sounds like a lost cause, none of the original authors had success in doing that—there's little chance we would. The ROOT package in conda-forge excludes a Windows build; presumably this, could too.

And yet the tests seem to be saying that Linux fails but MacOS and Windows do not? That doesn't make sense to me, especially the Windows case.

[henryiii]

@jpivarski the Windows and macOS builds are just processing the skip and passing.

[lgray]

@jpivarski yes - sorry I turned off those builds to keep things simple for now.

[henryiii]

@chrisburr would you like to make that PR against upstream? We can get it in.

[henryiii]

Well, I guess it depends if it works. Though still seems reasonable. We need to do some work on packaging soon....

[lgray]

Is that "safe" ?

[chrisburr]

Is that "safe" ?

Yes and no, it's disabling one of conda-builds safety checks but you're intentionally doing things in a non-standard way here so that's not inherently a problem. (Looks the whitelist is not working correctly, I'll take a look shortly. Actually it is, GitHub was showing me the previous pipeline)

A more important question is if this is what you want to do. A more conventional way to do this would be to install the fastjet library into $PREFIX/ (possibly as part of a separate libfastjet package that could be a separate output in the same recipe) and the Python bindings would then build against that rather that vendoring the libraries inside the site-packages directory.

[stale]

Hi friend!

We really, really, really appreciate that you have taken the time to make a PR on conda-forge/staged-recipes! conda-forge only exists because people like you donate their time to build and maintain conda recipes for use by the community.

In an effort to maintain this repository and increase the signal-to-noise for open PRs, the maintainers of staged-recipes close excessively old PRs after six months. This PR will remain open for another month, and then will be closed.

If you'd like to keep it open, please comment/push and we will be happy to oblige! Note that very old PRs will likely need to be rebased on main so that they can be rebuilt with the most recent CI scripts. If you have any trouble, or we missed reviewing this PR in the first place (sorry!), feel free to ping the team using a special command in a comment on the PR to get the attention of the staged-recipes team.

Cheers and thank you for contributing to this community effort!

[stale]

Hi again! About a month ago, we commented on this PR saying it would be closed in another month if it was still inactive. It has been a month and so now it is being closed. Thank you so much for making it in the first place and contributing to the community project that is conda-forge. If you'd like to reopen this PR, please feel free to do so at any time!

Cheers and have a great day!