Skip to content
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 to v2018.03.1 #12

Merged
merged 11 commits into from
May 11, 2018
Merged

Update to v2018.03.1 #12

merged 11 commits into from
May 11, 2018

Conversation

mcs07
Copy link
Contributor

@mcs07 mcs07 commented Apr 24, 2018

  • Update version to 2018.03.1
  • Skip builds for python 2.7 on Windows which is no longer supported by RDKit
  • Minor update to VS2015 patch to work with v2018.03.1
  • Lots of feedstock changes from a re-render after the big conda-smithy/conda-build update.

@conda-forge-linter
Copy link

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.

@mcs07
Copy link
Contributor Author

mcs07 commented Apr 24, 2018

Looks like Mac builds are failing with this error:

[  5%] Building CXX object Code/RDGeneral/CMakeFiles/RDGeneral.dir/LocaleSwitcher.cpp.o
/Users/travis/miniconda3/conda-bld/rdkit_1524573971640/work/Code/RDGeneral/LocaleSwitcher.cpp:73:10: error: thread-local storage is not supported for the current target
  static thread_local int recursion = 0;
         ^
1 error generated.

A google search suggests that this might be a bug in some versions of Apple-provided clang. conda-forge intentionally uses a very old version (osx_image: xcode6.4) to ensure the resulting packages work on old versions of Mac OS X. I think we can override this manually (probably to osx_image: xcode7.3?) but we'd probably lose support for OS X versions before 10.11 and we'd have to be careful that conda-smithy doesn't revert it back during the next re-render.

https://docs.travis-ci.com/user/reference/osx/#OS-X-Version

@mcs07
Copy link
Contributor Author

mcs07 commented Apr 25, 2018

Alternatively we could disable RDK_THREADSAFE_SSS, but that's not a great solution either.

@pstjohn
Copy link
Contributor

pstjohn commented Apr 25, 2018

Any thoughts on whether or not this would relate to rdkit/rdkit#1617 or #9?
Solving that would probably warrant breaking older mac builds. What are the drawbacks to disabling threadsafe?

@mcs07
Copy link
Contributor Author

mcs07 commented Apr 25, 2018

I think those issues are unrelated to this, and they should hopefully be resolved in v2018.03.1.

With RDK_THREADSAFE_SSS disabled, RDKit won't be able to do certain tasks in parallel on multiple threads, leading to worse performance when multiple CPU cores are available.

@mcs07
Copy link
Contributor Author

mcs07 commented Apr 25, 2018

Adding clangdev and libcxx requirements from conda-forge seems to have fixed the Mac builds. This seems to give us newer clang while still keeping the older MacOSX10.9.sdk.

Would anyone from @conda-forge/core be able to advise if this is a good solution and if there are any caveats?

I think this may all be solved eventually by the planned move to the anaconda compilers, but we will be waiting on Boost and everything else to switch also.

@jakirkham
Copy link
Member

We're going to be migrating to Anaconda's compilers. If you are already at the limits of what the CI provided compilers provide and are adding a new recipe (as is the case here), would advise just switching to Anaconda's compilers. This will save you work later as you will have completed this step already. Details in the link below.

cc @conda-forge/core

ref: https://conda.io/docs/user-guide/tasks/build-packages/compiler-tools.html#anaconda-compilers-and-conda-build-3

@mcs07
Copy link
Contributor Author

mcs07 commented Apr 25, 2018

Looks like we can't use the anaconda compilers on linux until boost is rebuilt using them?

@djsutherland
Copy link

djsutherland commented May 10, 2018

@mcs07: The setup has changed a bit since #12 (comment): using {{ compiler('cxx') }} on conda-forge currently just pulls in the toolchain package on Mac. You could do something like

  build:
    - {{ compiler('cxx') }}  # [not osx]
    - clangxx_osx-64  # [osx]

to start using the Anaconda ones now. (I think that's all you need, but not 100% positive.)

@mcs07
Copy link
Contributor Author

mcs07 commented May 10, 2018

@dougalsutherland Thanks I'll give it a go.

@pstjohn
Copy link
Contributor

pstjohn commented May 11, 2018

Try triggering a re-build.. that circleci build got hit by #6.
Is this more or less RTM? Awesome job btw, this looks great.

@mcs07
Copy link
Contributor Author

mcs07 commented May 11, 2018

OK yes, I think we are good to merge!

@pstjohn pstjohn merged commit 52d85b5 into conda-forge:master May 11, 2018
@pstjohn
Copy link
Contributor

pstjohn commented May 11, 2018

Fantastic! Thanks for the work on this.

@mcs07 mcs07 deleted the v2018.03.1 branch June 4, 2018 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants