conda-forge / rdkit-feedstock

A conda-smithy repository for rdkit.
BSD 3-Clause "New" or "Revised" License
8 stars 22 forks source link

add python, numpy, and boost deps to librdkit #171

Open greglandrum opened 3 weeks ago

greglandrum commented 3 weeks ago

This adds python, numpy, and libboost-python-devel as requirements to the build and host requirements of the librdkit target. I hope this is what's required for us to end up with one librdkit per python version.

This is a bit gross since there are no actual python dependencies in there, but librdkit installs the cmake configuration files for the RDKit and those end up with the python version in them (for include paths and libraries), so I think it's necessary.

Hopefully fixes #170

Checklist

conda-forge-webservices[bot] commented 3 weeks ago

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 (recipe/meta.yaml) and found it was in an excellent condition.

greglandrum commented 3 weeks ago

@conda-forge-admin please rerender

github-actions[bot] commented 3 weeks ago

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/rdkit-feedstock/actions/runs/10421465884.

greglandrum commented 2 weeks ago

@jaimergp @skearnes, could one of you please take a look at this?

jaimergp commented 2 weeks ago

I hope this is what's required for us to end up with one librdkit per python version.

Doesn't this undo some of the benefits of splitting librdkit to begin with? 🤔 Is there a fix that can be made in the CMake files instead?

greglandrum commented 2 weeks ago

I hope this is what's required for us to end up with one librdkit per python version.

Doesn't this undo some of the benefits of splitting librdkit to begin with? 🤔 Is there a fix that can be made in the CMake files instead?

Maybe, but I certainly don't know how to do it. The cmake configuration files are there for use by downstream projects so you can do FindPackage(rdkit) in a CMakeLists.txt. There's only one of those configuration files and it needs to be installed whether the python bindings are there or not.

What I think we can do, however, is add the requirements in a run_constrained section (https://docs.conda.io/projects/conda-build/en/latest/resources/define-metadata.html#run-constrained) so that you don't have to install python if you get librdkit, but if you do get it the version number is constrained. I will try that out locally and, if it works, update the PR

greglandrum commented 2 weeks ago

What I think we can do, however, is add the requirements in a run_constrained section (https://docs.conda.io/projects/conda-build/en/latest/resources/define-metadata.html#run-constrained) so that you don't have to install python if you get librdkit, but if you do get it the version number is constrained. I will try that out locally and, if it works, update the PR

This does not seem to work as I hoped it would, but I pushed the commit anyway just to see if we do actually end up with separate builds for the different python versions.

greglandrum commented 2 weeks ago

Unfortunately I am unable to download the build artifacts from the CI builds via the published artifacts page (https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=1013581&view=artifacts&pathAsName=false&type=publishedArtifacts) I'm pretty sure this worked in the past.

greglandrum commented 2 weeks ago

The problem we have at the moment is that we are producing something which is "clean", i.e. one librdkit build, but which is broken: it's no longer possible to install librdkit and reliably use that to bulid dependent software. I am happy to spend some time trying to fix the problem here or upstream so that we can go back to having something clean - one librdkit build which is completely independent on python version - but it would be nice to have functional builds available while we are waiting for that.