rdkit / homebrew-rdkit

Homebrew formula for rdkit
44 stars 19 forks source link

Update formula from 2019_09_1 to 2020_03_3 release #91

Closed cthoyt closed 4 years ago

UnixJunkie commented 4 years ago

Why are the CI tests not running for this PR? Where are the results?

cthoyt commented 4 years ago

I've been noticing Travis and GitHub have been communicating poorly lately. Conspiracy theory: GitHub is trying to push them out to make everyone use its own CI service.

Here's the latest build shown on Travis on my branch for this PR: https://travis-ci.com/github/cthoyt/homebrew-rdkit/builds/172202773.

Seems to be forgetting to install numpy (or maybe it's going to the wrong place)

UnixJunkie commented 4 years ago

I have other priorities currently, so this will have to wait. In the meantime, if you interact with the mailing list to have it compiling locally, that would be great. Since, we cannot merge this until it compiles.

UnixJunkie commented 4 years ago

superseded by https://github.com/rdkit/homebrew-rdkit/pull/92

UnixJunkie commented 4 years ago

Does this one work on a Mac? Could you test locally?

UnixJunkie commented 4 years ago
[ 18%] Built target coordgen
make[1]: *** [Code/DataStructs/Wrap/CMakeFiles/cDataStructs.dir/all] Error 2
[ 18%] Linking CXX shared module ../../../rdkit/rdBase.so
cd /tmp/rdkit-20200708-11720-143tsj2/rdkit-Release_2020_03_4/Code/RDBoost/Wrap && /usr/local/Cellar/cmake/3.17.3/bin/cmake -E cmake_link_script CMakeFiles/rdBase.dir/link.txt --verbose=1
/usr/local/Homebrew/Library/Homebrew/shims/mac/super/clang++ -std=c++11 -stdlib=libc++ -Wno-parentheses -Wno-logical-op-parentheses -Wno-format -mpopcnt -Wall -Wextra -Wno-deprecated -Wno-unused-function -fno-strict-aliasing -Wno-format -Wno-logical-op-parentheses -fPIC -stdlib=libc++ -DNDEBUG -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk -bundle -Wl,-headerpad_max_install_names -bundle -undefined dynamic_lookup -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk -o ../../../rdkit/rdBase.so CMakeFiles/rdBase.dir/RDBase.cpp.o  -Wl,-rpath,/tmp/rdkit-20200708-11720-143tsj2/rdkit-Release_2020_03_4/lib ../../../lib/libRDKitRDBoost.2020.03.4.dylib /usr/local/lib/libboost_python3.dylib /usr/local/lib/libboost_serialization-mt.dylib ../../../lib/libRDKitRDGeneral.2020.03.4.dylib 
[ 18%] Built target rdBase
make: *** [all] Error 2
UnixJunkie commented 4 years ago

I merged the superseding PR. Also, this other one was not touching the Travis things.

cthoyt commented 4 years ago

Sorry I did not get a chance to follow up on this.

I've identified that boost-python is installing the python3.8 package on homebrew, which is explicitly not the same as python3. Since they want to make sure all dependencies in the entire ecosystem are upgraded to python3.8-compatible first, they have not allowed python3.8 to be installed by python3. The only solution to making it compile was to ensure that rdkit is also using python3.8. Since you can't point to specific older versions of boost or boost-python, this solution requires forcing users to get on the new python3.8 if they want the install to be simple. Also because this requires a bit of compiling, I added caching to the travis build and a link explaining how it works into the travis.yml

I realize that this goes beyond the original title of the PR to just upgrade the version. However, I'm curious why you merged #92 (whose build fails for the same reason) when my original PR made the same succinct upgrade?

Anyway I will submit another PR with a better explanation with the remaining changes from this one

UnixJunkie commented 4 years ago

I merged it because it worked on my Mac (after some manual uninstalls, though).

If you send another PR, please separate any Travis improvement from an rdkit release update.

I think in the Travis things, we should force brew to uninstall rdkit and all its dependencies first. In that case, it seems that the rdkit brew formula is more reproducible.