Refactor and add rdkit-postgresql output

skearnes commented 3 weeks ago

Checklist

[x] Used a personal fork of the feedstock to propose changes
[x] Bumped the build number (if the version is unchanged)
[ ] Reset the build number to 0 (if the version changed)
[x] Re-rendered with the latest conda-smithy (Use the phrase code>@<space/conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
[x] Ensured the license file is being packaged.

Resolves #128

This PR refactors the recipe to contain multiple outputs:

librdkit (C++ library)
librdkit-dev (librdkit headers; renamed from rdkit-dev)
rdkit (Python bindings)
rdkit-postgresql (PostgreSQL cartridge)

There is also an implicit metapackage called rdkit-meta (note that the top-level package has been renamed from rdkit to rdkit-meta to avoid conflicting with the rdkit subpackage; I couldn't find another way to get the inter-subpackage dependencies and build requirements to work).

RDKit is built with both Python bindings and the PostgreSQL cartridge in build.sh, and then each output has a section in install.sh that installs the appropriate targets from the build into the corresponding subpackage.

Notes:

I'm marking this as draft to get feedback on my implementation of the overall split strategy suggested https://github.com/conda-forge/rdkit-feedstock/issues/128#issuecomment-1732907145. Once we get to something we like, I'll probably need some help with the Windows side of the builds.
Tested locally with build-locally.py on a single linux_64 target.
This is my first try at conda-forge; suggestions are welcome!

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) and found it was in an excellent condition.

skearnes commented 3 weeks ago

I think the osx_arm64 builds are failing because the pgsql rdkit.so isn't being converted into a dylib; I'm getting ERROR: could not access file "$libdir/rdkit": No such file or directory even though $libdir/rdkit.so exists?

Update: fixed by creating a symbolic link rdkit->rdkit.so

skearnes commented 2 weeks ago

pg_config is failing on arm64, but only in the context of cmake (if I run it separately in build.sh it's fine):

/Users/runner/miniforge3/conda-bld/rdkit-meta_1719518403904/work/Code/PgSQL/rdkit/CMakeLists.txt(73):  if(NOT Unknown system error -86 EQUAL 0 OR NOT PG_BINDIR )

https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=964328&view=logs&j=1b8be447-c2bd-5772-b66a-a1146441bf88&t=6d156886-9c65-5a39-5edc-896ebd7da557&l=3545

skearnes commented 2 weeks ago

pg_config is failing on arm64, but only in the context of cmake (if I run it separately in build.sh it's fine):
/Users/runner/miniforge3/conda-bld/rdkit-meta_1719518403904/work/Code/PgSQL/rdkit/CMakeLists.txt(73):  if(NOT Unknown system error -86 EQUAL 0 OR NOT PG_BINDIR )
https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=964328&view=logs&j=1b8be447-c2bd-5772-b66a-a1146441bf88&t=6d156886-9c65-5a39-5edc-896ebd7da557&l=3545

Looks like I'm not the first to encounter this: https://github.com/conda-forge/pgvector-feedstock/blob/main/recipe/arm64_pg_config

skearnes commented 2 weeks ago

Update: Windows builds are nearly there; librdkit, rdkit, and rdkit-dev are passing and I need to fix a remaining issue for rdkit-postgresql: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=967075&view=logs&j=171a126d-c574-5c8c-1269-ff3b989e923d&t=623c2ba8-c4f1-582d-6c78-1c7699e2e0a2&l=11825

greglandrum commented 2 weeks ago

Thanks for continuing to work through this @skearnes

skearnes commented 1 week ago

Thanks for continuing to work through this @skearnes

Finally passing!

skearnes commented 1 week ago

@conda-forge-admin, please rerender

greglandrum commented 1 week ago

I've built and tested this on both linux and windows and have done some basic verification that the rdkit python package and rdkit-postgresql package can be installed and work after being installed.

Since I'm not an expert on the overall functioning of conda-forge packages it would be really helpful if @jaimergp could also take a look at this.

conda-forge-webservices[bot] commented 1 week 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) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

You should not need to depend on libboost directly. If your package is linking to boost libraries, use libboost-devel in the host environment (which will create the correct dependence at runtime through a run-export), or use libboost-headers if you only need the boost headers.

skearnes commented 1 week ago

In the current state, this shouldn't be merged. Basic tests and run_exports are missing, and there are some strange dependencies added to pin things down (imo) unnecessarily.

Thanks @jaimergp for all the comments! Summary of my changes:

Pinned the boost version, allowing me to remove the extra host deps in the librdkit subpackage
Added simple tests for librdkit
Skipped the build for librdkit-dev except on Windows

Other changes are reflected in the various threads. PTAL; thanks!

conda-forge-webservices[bot] commented 1 week 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) and found it was in an excellent condition.

skearnes commented 1 week ago

@conda-forge-admin, please rerender

skearnes commented 1 week ago

@conda-forge-admin, please rerender

github-actions[bot] commented 1 week 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/9860865504.

skearnes commented 6 days ago

Ok. I have hunch that this might actually be due to the values in the .ci_support files. I tried every combination of dependencies in a clean env and I always got 1.85, so the mismatch may have been a false alarm due to the pinning in those files being applied to the base build but not the outputs.

On Wed, Jul 10, 2024, 7:16 AM jaimergp @.***> wrote:

@.**** commented on this pull request.

In recipe/meta.yaml https://github.com/conda-forge/rdkit-feedstock/pull/158#discussion_r1672088288 :

{% set version = "2024.03.4" %} +{% set boost_version = "1.85" %}

Unfortunately pinning boost like this is going to prevent you from receiving the bot updates. Whatever's going on with the boost versions needs to be figured out, but we can't override the global pinnings https://github.com/conda-forge/conda-forge-pinning-feedstock/blob/446c2a56a2caf948bc9e0fcd7662ac1da26d9385/recipe/conda_build_config.yaml#L468-L471 .

We currently have 1.84 there. Is there any reason to go for 1.85?

— Reply to this email directly, view it on GitHub https://github.com/conda-forge/rdkit-feedstock/pull/158#pullrequestreview-2168795316, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHITGOOYWLN2UIPSCHAJU3ZLUJYBAVCNFSM6AAAAABJY5VXC2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNRYG44TKMZRGY . You are receiving this because you were mentioned.Message ID: @.***>

skearnes commented 6 days ago

@conda-forge-admin, please rerender

skearnes commented 6 days ago

Using libboost-devel in the librdkit host reqs instead of pin_compatible seems to fix the boost version mismatch. But now the mac builds are failing due to a boost linking error (no idea why; nothing has changed in the base build).

skearnes commented 6 days ago

I fixed the mac builds by removing some custom CXXFLAGS in build.sh:

# Workaround for clang++ and boost functional.
if [ "${target_platform}" == "osx-arm64" ] || [ "${target_platform}" == "osx-64" ]; then
    export CXXFLAGS="-D_LIBCPP_DISABLE_AVAILABILITY -D_HAS_AUTO_PTR_ETC=0 $CXXFLAGS"
fi

@greglandrum does that surprise you? I have no idea why it started failing just before now.

skearnes commented 6 days ago

@jaimergp thanks again! I think we're close?

skearnes commented 6 days ago

@conda-forge-admin, please rerender

github-actions[bot] commented 6 days 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/9883125158.

jaimergp commented 5 days ago

I fixed the mac builds by removing some custom CXXFLAGS in build.sh:

The first one was needed for this gotcha, I guess. But if removing them fixes the build and everything passes, I guess we are good to go 🚀

skearnes commented 5 days ago

@conda-forge-admin, please rerender

github-actions[bot] commented 5 days 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/9897250557.

github-actions[bot] commented 5 days 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/9897269658.

jaimergp commented 4 days ago

meta.yaml and build scripts look awesome now, thanks for the patience.

I'm now checking the logs and I see a few warnings like this in the rdkit and rdkit-postgresql outputs:

WARNING (rdkit-postgresql,lib/rdkit.so): $RPATH/libRDKitRDInchiLib.so.1 not found in packages, sysroot(s) nor the missing_dso_whitelist.

That's strange because librdkit does ship that library, so I'm not sure if this a bug in conda-build or something. I wonder if we can silence those by adding the suggestion I'm commenting below.

jaimergp commented 4 days ago

In the artifact contents I also see we are distributing the whole RDKit book in librdkit. Is this a good opportunity to remove that or put in a different output? rdkit-docs or something?

skearnes commented 4 days ago

@conda-forge-admin, please rerender

jaimergp commented 4 days ago

Is this stuff in librdkit more dev than lib?

?rw-rw-r-- 0/0       1201 2024-07-12 16:01:14 share/RDKit/Projects/CTestTestfile.cmake 
?rw-rw-r-- 0/0      20886 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/CreateDb.py 
?rw-rw-r-- 0/0      25224 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/SearchDb.py 
?rw-rw-r-- 0/0      21735 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/UnitTestDbCLI.py 
?rw-rw-r-- 0/0       1633 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/moe_like.dsc 
?rw-rw-r-- 0/0        312 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/testData/README 
?rw-rw-r-- 0/0        460 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/testData/bzr.2.smi 
?rw-rw-r-- 0/0     322884 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/testData/bzr.sdf 
?rw-rw-r-- 0/0        523 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/testData/bzr.smi 
?rw-rw-r-- 0/0       2174 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/testData/bzr_q1.mol 
?rw-rw-r-- 0/0     433172 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/testData/pubchem.200.sdf 
?rw-rw-r-- 0/0       9120 2024-07-12 16:01:14 share/RDKit/Projects/Makefile 
?rw-rw-r-- 0/0       1654 2024-07-12 16:01:14 share/RDKit/Projects/cmake_install.cmake 
?rw-rw-r-- 0/0        150 2024-07-01 03:48:34 share/RDKit/Projects/pytest.ini 
?rw-rw-r-- 0/0        621 2024-07-01 03:48:34 share/RDKit/README 
?rw-rw-r-- 0/0        309 2024-07-01 03:48:34 share/RDKit/Scripts/FeatFinderCLI.py 
?rw-rw-r-- 0/0       2154 2024-07-01 03:48:34 share/RDKit/Scripts/PythonFormat.py 
?rw-rw-r-- 0/0        314 2024-07-01 03:48:34 share/RDKit/Scripts/README.md 
?rw-rw-r-- 0/0        797 2024-07-01 03:48:34 share/RDKit/Scripts/create_deb_packages.sh 
?rw-rw-r-- 0/0      22995 2024-07-01 03:48:34 share/RDKit/Scripts/gen_rdkit_stubs/__init__.py 
?rw-rw-r-- 0/0       2506 2024-07-01 03:48:34 share/RDKit/Scripts/gen_rdkit_stubs/__main__.py 
?rw-rw-r-- 0/0       3026 2024-07-01 03:48:34 share/RDKit/Scripts/gen_rdkit_stubs/worker.py 
?rw-rw-r-- 0/0      75632 2024-07-01 03:48:34 share/RDKit/Scripts/patch_rdkit_docstrings/__init__.py 
?rw-rw-r-- 0/0       3261 2024-07-01 03:48:34 share/RDKit/Scripts/patch_rdkit_docstrings/__main__.py

skearnes commented 4 days ago

Is this stuff in librdkit more dev than lib?

?rw-rw-r-- 0/0       1201 2024-07-12 16:01:14 share/RDKit/Projects/CTestTestfile.cmake 
?rw-rw-r-- 0/0      20886 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/CreateDb.py 
?rw-rw-r-- 0/0      25224 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/SearchDb.py 
?rw-rw-r-- 0/0      21735 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/UnitTestDbCLI.py 
?rw-rw-r-- 0/0       1633 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/moe_like.dsc 
?rw-rw-r-- 0/0        312 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/testData/README 
?rw-rw-r-- 0/0        460 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/testData/bzr.2.smi 
?rw-rw-r-- 0/0     322884 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/testData/bzr.sdf 
?rw-rw-r-- 0/0        523 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/testData/bzr.smi 
?rw-rw-r-- 0/0       2174 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/testData/bzr_q1.mol 
?rw-rw-r-- 0/0     433172 2024-07-01 03:48:34 share/RDKit/Projects/DbCLI/testData/pubchem.200.sdf 
?rw-rw-r-- 0/0       9120 2024-07-12 16:01:14 share/RDKit/Projects/Makefile 
?rw-rw-r-- 0/0       1654 2024-07-12 16:01:14 share/RDKit/Projects/cmake_install.cmake 
?rw-rw-r-- 0/0        150 2024-07-01 03:48:34 share/RDKit/Projects/pytest.ini 
?rw-rw-r-- 0/0        621 2024-07-01 03:48:34 share/RDKit/README 
?rw-rw-r-- 0/0        309 2024-07-01 03:48:34 share/RDKit/Scripts/FeatFinderCLI.py 
?rw-rw-r-- 0/0       2154 2024-07-01 03:48:34 share/RDKit/Scripts/PythonFormat.py 
?rw-rw-r-- 0/0        314 2024-07-01 03:48:34 share/RDKit/Scripts/README.md 
?rw-rw-r-- 0/0        797 2024-07-01 03:48:34 share/RDKit/Scripts/create_deb_packages.sh 
?rw-rw-r-- 0/0      22995 2024-07-01 03:48:34 share/RDKit/Scripts/gen_rdkit_stubs/__init__.py 
?rw-rw-r-- 0/0       2506 2024-07-01 03:48:34 share/RDKit/Scripts/gen_rdkit_stubs/__main__.py 
?rw-rw-r-- 0/0       3026 2024-07-01 03:48:34 share/RDKit/Scripts/gen_rdkit_stubs/worker.py 
?rw-rw-r-- 0/0      75632 2024-07-01 03:48:34 share/RDKit/Scripts/patch_rdkit_docstrings/__init__.py 
?rw-rw-r-- 0/0       3261 2024-07-01 03:48:34 share/RDKit/Scripts/patch_rdkit_docstrings/__main__.py

Maybe, but that's an issue for the upstream cmake configuration. I can't do much about it here (and I'm not willing to carve out globs for things like this).

jaimergp commented 4 days ago

WARNING (rdkit-postgresql,lib/rdkit.so): Needed DSO lib/libboost_serialization.so.1.84.0 found in ['conda-forge/linux-64::libboost==1.84.0=hba137d9_3']
WARNING (rdkit-postgresql,lib/rdkit.so): .. but ['conda-forge/linux-64::libboost==1.84.0=hba137d9_3'] not in reqs/run, (i.e. it is overlinking) (likely) or a missing dependency (less likely)
WARNING (rdkit-postgresql,lib/rdkit.so): Needed DSO lib/libstdc++.so.6 found in ['conda-forge/linux-64::libstdcxx-ng==14.1.0=hc0a3c3a_0']
WARNING (rdkit-postgresql,lib/rdkit.so): .. but ['conda-forge/linux-64::libstdcxx-ng==14.1.0=hc0a3c3a_0'] not in reqs/run, (i.e. it is overlinking) (likely) or a missing dependency (less likely)
WARNING (rdkit-postgresql,lib/rdkit.so): Needed DSO lib/libgcc_s.so.1 found in ['conda-forge/linux-64::libgcc-ng==14.1.0=h77fa898_0']
WARNING (rdkit-postgresql,lib/rdkit.so): .. but ['conda-forge/linux-64::libgcc-ng==14.1.0=h77fa898_0'] not in reqs/run, (i.e. it is overlinking) (likely) or a missing dependency (less likely)
   INFO (rdkit-postgresql,lib/rdkit.so): Needed DSO x86_64-conda-linux-gnu/sysroot/lib64/libc.so.6 found in CDT/compiler package conda-forge/noarch::sysroot_linux-64==2.17=h4a8ded7_16

We are going to need some stuff in host for the postgresql too, I think. Adding a couple of suggestions in a moment.

jaimergp commented 4 days ago

I see some headers in librdkit too:

------------------------------------------
osx-64/librdkit-2024.03.4-h51a49a3_1.conda
------------------------------------------
-- Size: 18.3MB
-- SHA256: 1c4b57d8e532a75724413aa59792553d8d26ff7d1093ed39ed2d516a3f43832e
-- Contents:
?rw-rw-r-- 0/0       4451 2024-07-01 03:48:34 include/rdkit/RDGeneral/hash/detail/float_functions.hpp 
?rw-rw-r-- 0/0       4929 2024-07-01 03:48:34 include/rdkit/RDGeneral/hash/detail/hash_float.hpp 
?rw-rw-r-- 0/0       4138 2024-07-01 03:48:34 include/rdkit/RDGeneral/hash/extensions.hpp 
?rw-rw-r-- 0/0      15699 2024-07-01 03:48:34 include/rdkit/RDGeneral/hash/hash.hpp 
?rw-rw-r-- 0/0       1240 2024-07-01 03:48:34 include/rdkit/RDGeneral/hash/hash_fwd.hpp

Are these expected there?

And what about these cmake files?

?rw-rw-r-- 0/0       1865 2024-07-12 17:03:36 lib/cmake/rdkit/rdkit-config-version.cmake 
?rw-rw-r-- 0/0       1245 2024-07-12 17:03:36 lib/cmake/rdkit/rdkit-config.cmake 
?rw-rw-r-- 0/0      37741 2024-07-12 17:03:36 lib/cmake/rdkit/rdkit-targets-release.cmake 
?rw-rw-r-- 0/0      27646 2024-07-12 17:03:36 lib/cmake/rdkit/rdkit-targets.cmake

jaimergp commented 4 days ago

Also librdkit contains a few Contrib modules, including Python stuff 🤔

?rw-rw-r-- 0/0      14150 2024-07-01 03:48:34 share/RDKit/Contrib/AtomAtomSimilarity/AtomAtomPathSimilarity.py

?rw-rw-r-- 0/0      23160 2024-07-01 03:48:34 share/RDKit/Contrib/FreeWilson/freewilson.py

And some sources and config files that I'm not sure they belong there

?rw-rw-r-- 0/0        378 2024-07-01 03:48:34 share/RDKit/Contrib/ConformerParser/CMakeLists.txt 
?rw-rw-r-- 0/0       4405 2024-07-01 03:48:34 share/RDKit/Contrib/ConformerParser/ConformerParser.cpp 
?rw-rw-r-- 0/0       2738 2024-07-01 03:48:34 share/RDKit/Contrib/ConformerParser/ConformerParser.h 
?rw-rw-r-- 0/0        333 2024-07-01 03:48:34 share/RDKit/Contrib/ConformerParser/Wrap/CMakeLists.txt 
?rw-rw-r-- 0/0       3342 2024-07-01 03:48:34 share/RDKit/Contrib/ConformerParser/Wrap/rdConformerParser.cpp 
?rw-rw-r-- 0/0       1238 2024-07-01 03:48:34 share/RDKit/Contrib/ConformerParser/Wrap/testConformerParser.py 
?rw-rw-r-- 0/0       4378 2024-07-01 03:48:34 share/RDKit/Contrib/ConformerParser/test.cpp

Btw this all comes from the Inspecting artifacts section at the end of the logs, in case you want to take a look.

skearnes commented 4 days ago

@jaimergp if it's ok with you I'd rather not try to get all the file partitioning just right---these are issues that should be handled upstream with the cmake configs and doing everything here seems unnecessary. I'm happy to fix warnings in the conda build but I don't want this PR to stall much longer while we strive for perfection.

jaimergp commented 4 days ago

In that case, we might be lucky with the latest change being the last :D

jaimergp commented 15 hours ago

With https://github.com/conda-forge/rdkit-feedstock/pull/160, we need to resolve the conflicts in meta.yaml and rerender. You can ignore the changes in .ci_support/*.yaml because they will be regenerated with the rerender anyway.

skearnes commented 10 hours ago

@conda-forge-admin, please rerender

conda-forge / rdkit-feedstock

Refactor and add rdkit-postgresql output #158

@.**** commented on this pull request.