rdkit / conda-rdkit

Conda build recipe for the rdkit
50 stars 30 forks source link

update the rdkit-postgresql recipe for release 2018.03 #65

Closed rvianello closed 6 years ago

rvianello commented 6 years ago

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+).

greglandrum commented 6 years ago

Thanks Riccardo. I will try and take a look on the Mac either this evening or tomorrow morning.

greglandrum commented 6 years ago

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.

greglandrum commented 6 years ago

I did a Mac build this morning and everything worked fine, but I'm kind of curious about the output. I got the following:

anaconda upload /Users/glandrum/anaconda5/conda-bld/osx-64/rdkit-postgresql-2018.03.1.1-h4feeb9f_1.tar.bz2
anaconda upload /Users/glandrum/anaconda5/conda-bld/osx-64/rdkit-postgresql-2018.03.1.1-hec0650b_1.tar.bz2
anaconda upload /Users/glandrum/anaconda5/conda-bld/osx-64/rdkit-postgresql-2018.03.1.1-hb8657ce_1.tar.bz2

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.

rvianello commented 6 years ago

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).

rvianello commented 6 years ago

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.

greglandrum commented 6 years ago

@rvianello : do you want me to wait a bit longer before merging this?

rvianello commented 6 years ago

@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).

greglandrum commented 6 years ago

Sounds good. I agree that doing the other cleanups in smaller PRs makes sense.