conda-forge / vtk-feedstock

A conda-smithy repository for vtk.
BSD 3-Clause "New" or "Revised" License
13 stars 64 forks source link

Re-Enable MacOs Build #47

Closed rlizzo closed 6 years ago

rlizzo commented 6 years ago

Hey guys, I just wanted to give you a heads up that the timeout issues with building the Mac version of VTK should be fixed once the next conda build version is released. I got fed up with the overwhelmingly slow "fixing linking" phase (which on my benchmarks took up about 60% of build time), so i dug into the conda build process and found some massive inefficiencies in how the dylibs were being made relocatable (only for Mac builds). We just merged the changes in PR https://github.com/conda/conda-build/pull/2564 , and I'm seeing a 100x speedup on the second phase.

The next release will probably be in a few days, so once it goes through give it a shot. Would love to have VTK be available for all platforms on conda-forge (lack of Mac support is the only thing preventing me from submitting my library as a feedstock). Cheers!

msarahan commented 6 years ago

3.1.3 is out, but conda-forge is not using conda-build 3.x yet. Notes from our most recent meeting are at https://paper.dropbox.com/doc/Compilers-conda-build-3-update-20171211-JSvRO2Lk2Z7Bbf2zDaG9p

rlizzo commented 6 years ago

Thanks Mike! Thats really unfortunate to hear. I'd be happy to contribute a bit of time if you guys have any projects that would be good for a newcomer to the conda-forge codebase.

jakirkham commented 6 years ago

Thanks for doing that @rlizzo! Haven’t read the PR in detail, but from skimming it this looks great. Will likely help a lot of other packages struggling with long build times on macOS as well.

@msarahan is correct that we are not yet on conda-build 3, but we are working towards it. Your fixes should provide additional motivation to finish this transition.

rlizzo commented 6 years ago

Happy to help @jakirkham, I just identified the issue, the heavy lifting was really done by @msarahan and @mingwandroid.

Out of curiosity. Have you guys addressed how conda-forge will handle different c++ standards during buildtime/runtime? It's pretty easy for individuals to make the modifications in the build.sh script, and then just specify that varient in your packages meta.yaml file, but it doesn't seem scaleable for maintainers of feedstocks. I'm just asking because there are still a few projects out there (I'm looking at you ITK), that are still stuck on the c++11 standard (or earlier if you want to use prior versions), and it may be a bit of an issue if conda-forge imposes c++14 on them (let alone any other compiler flags people may wish to change).

msarahan commented 6 years ago

I think you can build C++11 code with the c++14/17 standard. We have new compilers, described at https://www.anaconda.com/blog/developer-blog/utilizing-the-new-compilers-in-anaconda-distribution-5/ that let us use current standards. I believe that is 14 on mac (clang 4.0.1), and 17 on Linux (gcc 7.2)

rlizzo commented 6 years ago

So yes, c++11 code is absolutely valid for compilation with c++14. And as far as i know, there isn't a language spec that requires APIs/ABIs to break their link compatibility either (though that's not to say that they won't). But there are a few minor language changes which can cause unexpected behavior (awesome S/O post ). In my experience, it's easiest to just stick to the standard that the library currently supports. There are more qualified people than me to figure out if this is really an issue or not, but it just struck me as a potential pain point for people down the road.

mingwandroid commented 6 years ago

As @msarahan says, at present, Anaconda Defaults compiler activation scripts set CXXFLAGS etc to enable the latest possible C++ standard (and changing this is something I will resist in the strongest possible terms).

The reason for this choice is to enjoy the performance benefits of these standards for code that checks the _cplusplus define to enable those benefits.

Sure some software breaks in this case (the C++14 noexcept being a prime example) so when you run into problems you need to do something along the lines of:

# Filter out -std=.* from CXXFLAGS as it disrupts checks for C++ language levels.
re='(.*[[:space:]])\-std\=[^[:space:]]*(.*)'
if [[ "${CXXFLAGS}" =~ $re ]]; then
    export CXXFLAGS="${BASH_REMATCH[1]}${BASH_REMATCH[2]}"
fi

.. I am moving away from using bash's re though since it behaves differently on macOS and Linux. Now I'd just use sed instead (which also behaves differently; we need to use our own build of GNU sed instead).

grlee77 commented 6 years ago

That "fixing linking" phase was annoyingly slow on OS X, so thanks for tracking down that problem. This will definitely get us much closer to the CI time limit!

I am hopeful for success, but in the past I have observed the timeout occur even before that phase is reached: https://travis-ci.org/conda-forge/vtk-feedstock/builds/242985816?utm_source=github_status&utm_medium=notification

Korijn commented 6 years ago

Even on SSD i7 development machines it can take longer than the Travis CI timeout..

mwaskom commented 6 years ago

Linking to #10 (since that's where I end up when I try to track this down to see the status)

rlizzo commented 6 years ago

Hi all! Just wanted to see if there is a status update on this? We would really like to move our package to conda-forge but don't want to make the transition unless all three platforms can be supported (especially since most of our main devs all work on macs :smile:).

I noticed that manual uploads were performed for the qt conda-forge packages. If there's not an obvious way to speed up the build times on OSX, is there a possibility that mac builds can be handled in the same way for VTK?

jakirkham commented 6 years ago

We could certainly do a manual upload. Another option would be to build on CircleCI (as we have some macOS workers). Though not sure offhand what the status is for the last option.

cc @isuruf

jakirkham commented 6 years ago

If someone would like to take a look at migrating to the latest conda-smithy, we can look at other macOS options.

isuruf commented 6 years ago

Here's a branch that you can work on, https://github.com/isuruf/vtk-feedstock/tree/osx/recipe

jakirkham commented 6 years ago

@looooo's and @isuruf's work in PR ( https://github.com/conda-forge/vtk-feedstock/pull/54 ) enables macOS builds using CircleCI. Could some people take a look and make sure it is being built ok?

jakirkham commented 6 years ago

VTK macOS packages are currently being generated. It may take several hours for them to complete, but expect them to be done by end of day (assuming no issues). Please try them and share your findings. We can iterate as needed since the builds are automated.

dstansby commented 6 years ago

I can't see any macOS packages here: https://anaconda.org/conda-forge/vtk ; are there meant to be some?

looooo commented 6 years ago

In the meantime a new issue came up and no build for osx was created. I guess this is the current state: https://github.com/conda-forge/vtk-feedstock/pull/56 osx seems to fail because of a freetype problem.