-
Notifications
You must be signed in to change notification settings - Fork 28
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
update the rdkit-postgresql recipe for release 2018.03 #65
update the rdkit-postgresql recipe for release 2018.03 #65
Conversation
Thanks Riccardo. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small change request here. More thoughts/suggestions coming as comments.
+find_package(RDKit REQUIRED) | ||
+include_directories(${RDKit_INCLUDE_DIRS}) | ||
+ | ||
+find_package(PostgreSQL REQUIRED) | ||
+# Boost and Eigen libraries that are required at link time | ||
+find_package(Boost 1.56.0 COMPONENTS serialization thread) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to drop thread from the requirements here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tried, but cmake produced a warning reading [...] Target "rdkit" links to target "Threads::Threads" but the target was not found. Perhaps a find_package() call is missing for an IMPORTED target, or an ALIAS target is missing?
and the build failed with /home/ric/rdkb/rdkit-postgresql_1525713265598/_build_env/bin/../lib/gcc/x86_64-conda_cos6-linux-gnu/7.2.0/../../../../x86_64-conda_cos6-linux-gnu/bin/ld: cannot find -lThreads::Threads
If you were willing to do an actual RDKit build here, you could almost certainly get rid of the runtime RDKit and boost dependencies by linking against each statically. It might be worth thinking about since it seems like this could be quite useful. |
I did a Mac build this morning and everything worked fine, but I'm kind of curious about the output. I got the following:
I assume that each of the builds is against a different version of postgresql, but I'm surprised that the psql version doesn't somehow show up in the filename. If that could be added, I think it would make what has been built a bit clearer. |
many thanks for testing the recipe on osx! running a full rdkit build, and this way statically link some dependencies, would also get rid of ensuring that the dependency from rdkit is correctly tracked (which is currently quite difficult to do). I will give it a try. regarding the dependency of different build variants from distinct versions of postgres, it's actually already reflected in the filename, in fact it contributes to the hash string that conda-build 3 uses to summarize all the recipe metadata (e.g. h4feeb9f). this is definitely not human readable, and not as friendly as a different package name (e.g. rdkit-postgresql95) would be, but it's so by design, because a different package name would be interpreted as a completely unrelated component. with this (somewhat questionable) generalization of build strings, it is easy to create a package X that depends on rdkit-postgresql, without referencing any particular version of postgres, this way allowing the same package X to be used with any version of postgresql, as long as a suitable build variant of rdkit-postgresql is found (with the older approach, a different X-Y package would have been required to be built for every rdkit-postgresqlY). |
I adapted the recipe to use static linking. It now requires building the toolkit, and it's therefore quite slower. I'll review the cmake configuration for additional components that could be disabled. |
@rvianello : do you want me to wait a bit longer before merging this? |
@greglandrum merging would be fine for me, if you are ok with leaving the windows support on hold for now (in the current revision it's no longer functional), and I would be happy to use smaller, separate PRs to propose a cleanup of the recipes that these changes are obsoleting (e.g. rdkit-postgresql95). |
Sounds good. I agree that doing the other cleanups in smaller PRs makes sense. |
* updated patch file * fix issue #27 * comply with the formatting applied by #1078 * use cmake to build the postgresql cartridge * update the release tag * update release label * COPY the recipes instead of cloning the git repo * update main build to 2016.9.4 * update rdkit version * get boost builds working with py36 * rdkit build for windows and py36 * linux and mac update * psql update * make pandas and pillow dependencies explicit * bump release * - enable rdkit, rdkit-postgresql95 and rdkit-postgresql conda builds on all platforms * - added missing bld.bat files * - added recipes for Windows PostgreSQL recipes * - added patch to build postgresql <=9.5.2 with VS2015 * switch which version is being built * Update which code is built * Update which code is built * add cheto build * switch to master and the rdkit repo * update which boost libraries are built * update boost dependency to 1.61 for unix * switch cheto to no-arch * switch to allowing versions of numpy greater than 1.11 instead of requiring a particular version * add a docker setup to do linux builds of the RDKit code * this works on the mac, but needs more testing * works on linux, and the mac * switch back to Release branch * remove runtime conda deps * remove python 3.4 support * update rdkit-postgresql95 build * Updates conda version to conda 3.0 compatible version * builds on the mac * stop requiring a particular conda version stop building the c++ tests * add freesasa to linux/mac builds * switch to requiring numpy 1.12 at build and >= 1.12 at runtime * update * try restricting this to psql9.5 * get postgresql95 build updated and working remove python dependency switch to 9.5.10 * get rdkit-postgresql95 build updated and working * switch boost download location for unix * oops * backup * another unsuccessful attempt at a mac build * legacy cmake argument removed * start using conda-build 3 syntax * update cairo * update to reflect new layout * Switch to the production branch. * remove un-necessary dependencies * rename cairo_nox back to cairo, update the rdkit meta.yaml to reflect that * do not activate the root environment... <sigh> * remove 'source activate root' from cairo_nox recipe * update the rdkit-postgresql recipe for release 2018.03 (#65) * initial round of updates targeting rdkit 2018.03.1 * build multiple variants of the cartridge for different versions of postgres * build the cartridge using static linking * remove the obsolete patch files * speed up build (#67) * remove the now obsolete rdkit-postgresql95 recipe (#68) Yeah, this one’s easy. Thanks! * try to force cairo_nox on linux builds * force a pandas version on build * allow pandas <=0.22 * windows builds should not be so huge
this is still in progress, but the current changes should allow building the cartridge for linux using the postgresql 10.3 package from the anaconda distribution, so it might be also sufficient for performing some preliminary tests on osx too.
next steps should include proper versioning of the postgresql dependency and building for both postgres 10 and 9, plus some more in depth testing.
eventually, I would like to remove the bundled postgresql recipes, and the rdkit-postgresql95 recipe (no longer needed and made unnecessary by the improved support for build variants in conda-build 3+).