search

conda-forge / vtk-feedstock

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

Cross-platform VTK 7 build #1

Closed Korijn closed 8 years ago

Korijn commented 8 years ago
conda-forge-linter commented 8 years ago

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.

Korijn commented 8 years ago

Looks like OS X is now building as well. Build timeouts will still be an issue though, so I tried to pre-emptively skip less popular configurations.

ccordoba12 commented 8 years ago

Why AppVeyor is not getting triggered?

ivoflipse commented 8 years ago

@ccordoba12 because it was taking too long, running into the limits of AppVeyor

jakirkham commented 8 years ago

So, @Korijn, when one changes the skip setting, one needs to re-render with conda-smithy. Please install conda-smithy in the root environment from conda-forge and run conda smithy rerender in the top level directory of this repo. Commit and push all changes. That should enable AppVeyor support.

grlee77 commented 8 years ago

looks like Travis timed out at 80% on the build. Maybe adding -j2 to the make line would run the build in parallel and allow it to complete. I don't know if it is guaranteed that we can use 2 cores on Travis though?

grlee77 commented 8 years ago

To answer my own question above: unfortunately, the specs for Os X builds on Travis only list 1 core: https://docs.travis-ci.com/user/ci-environment/

ccordoba12 commented 8 years ago

@ccordoba12 because it was taking too long

Yes, but you skipped win32 so win64 should work. So you need to follow @jakirkham advice to update the skip rules :-)

@jakirkham, this is the perfect example to ask for more time to Travis and AppVeyor :-)

Korijn commented 8 years ago

I was wondering about that @jakirkham! I will get to it later. It's getting late on this side of the globe. :) Thanks for your input! I'll try enabling multicore builds for linux and os x using $CPU_COUNT as well.

Also, what do you think about raising some funds to upgrade the CI packages? I think it shouldn't be too much of a stretch...

jakirkham commented 8 years ago

No worries, @Korijn. If I have time some day, I'll try to write some docs. 😄

Yeah, Travis CI says it has only 1 core. So, not sure what we can do about it.

Unfortunately, the number of cores is often incorrectly reported by CI and may be reporting the number of cores on a shared node. In short, I wouldn't recommend using $CPU_COUNT.

@jakirkham, this is the perfect example to ask for more time to Travis and AppVeyor :-)
Also, what do you think about raising some funds to upgrade the CI packages? I think it shouldn't be too much of a stretch...

I think this is a great idea, @Korijn @ccordoba12.

There have been people that have periodically mentioned interest in donating build time. This certainly is a possibility, but really hasn't gotten the attention it probably deserves. We would probably want to figure out some terms for acceptable use and code necessary to make this function.

There has also been some discussion about becoming a NumFOCUS sponsored group. It seems NumFOCUS would be interested in having us and I haven't heard any dissent on our side, but it hasn't been discussed in detail. This would also be another path for acquiring such funds. It also might lower the administrative overhead.

Writing letters are probably the easiest. If people want to start on a letter, please go for it. We did something similar with GitHub in the past. My suggestion is someone start a Google Doc and open editing to everyone. Then just drop this in a new issue on the web repo issue tracker. I can't promise that I will get to it myself any time soon. However, I'm more than happy to participate and guide the process along.

Korijn commented 8 years ago

I rerendered for the new skip rules, and added what is supposed to configure cl on Windows to use as many cores as are available on AppVeyor (which is 2).

Some surprising output on the Windows build:

CMake Warning:
  Manually-specified variables were not used by the project:
    INSTALL_BIN_DIR
    INSTALL_LIB_DIR
    INSTALL_MAN_DIR

After reading a bit more, these variables are used when VTK also compiles and installs 3rd party, libs such as zlib. Currently the build is not configured to do so (I think), but I propose leaving them in anyway.

Korijn commented 8 years ago

Everything timed out! :') Most reached into 90% though. Maybe there's some optional components we can skip...

patricksnape commented 8 years ago

You can definitely build VTK on Appveyor but you need to ask for 1.5 hour build times.

Korijn commented 8 years ago

@patricksnape I emailed AppVeyor about it. Can we do the same thing for CircleCI and TravisCI?

demarle commented 8 years ago

Thanks for working on this! If build times are still a limiting factor, is it possible to switch to ninja? (cmake -G Ninja, then ninja instead of make or nmake). In VTK/PV developer land we've all pretty much gravitated to it since the build times are so much better.

Korijn commented 8 years ago

I've been considering that! Looks like conda-forge already has a ninja package so it should be relatively painless.

jakirkham commented 8 years ago

Seemed to help AppVeyor. Travis CI and CircleCI appear to still have issues. Maybe we can try cotire ( https://github.com/conda-forge/staged-recipes/pull/897 ). Might require a patch to get working.

Korijn commented 8 years ago

@jakirkham I'm not terribly sure how to go about adding cotire to this PR. Would you mind doing it if I give you permissions on the fork?

jakirkham commented 8 years ago

I can try to add a PR against your branch if that would work. Let's get cotire in and then we can see what we can do from there.

jakirkham commented 8 years ago

@jschueller, here is a case where it might be nice to have cotire as a dependency. Though it is true it can be added to VTK. Having it as a dependency makes it much simpler to play with.

Korijn commented 8 years ago

@jakirkham

I'm also a little bit baffled that AppVeyor is completing in about 23 minutes now. That's amazing.

Korijn commented 8 years ago

@jakirkham I have an odd suggestion: iterating over subsets of the configurations through two or three PRs. Is that acceptable, or do we really want to get below the build timeout?

Korijn commented 8 years ago

By skipping py3.4 on unix only I was able to get AppVeyor and CircleCI to pass. Travis still times out though. I will skip OS X once more so we can reach all-green. Then we can open another PR to push out the remaining configurations. Agree @jakirkham ?

Korijn commented 8 years ago

Well, once Travis finally responds to let us know that it skipped the build... everything should be green. Compared to the feedstock, this PR upgrades the build scripts to function cross-platform. Due to CI build time limits, it unfortunately only adds 6 Windows configurations (x86 & x64 for py2.7, 3.4 & 3.5) to the already available linux builds (x64 for py2.7 & 3.5).

I'd like to merge and open another PR where we can dig into optimizing the build even further, using things like cotire as suggested by @jakirkham.

Korijn commented 8 years ago

@ocefpaf This TravisCI build could also use a restart... :)

Korijn commented 8 years ago

There we go! Ready for merge.

We can then open another PR where we can squeeze out OS X and Linux 3.4 support.

ocefpaf commented 8 years ago

@ocefpaf This TravisCI build could also use a restart... :)

@Korijn you should have rights to restart Travis-CI and merge things here. Let me know if that is not working.

Korijn commented 8 years ago

I do have merge rights on github, but no rights for CI.

ocefpaf commented 8 years ago

I do have merge rights on github, but no rights for CI.

Maybe you need to manually sync your Travis-CI. That did bite me in the past.

jakirkham commented 8 years ago

With Travis CI, you have to resync your account.

ccordoba12 commented 8 years ago

Merged! @Korijn, great work making the Windows builds to work!

I also noticed that VTK builds its own copies of freetype, libpng and other libraries we have. Maybe there are CMake options to deactivate that?

jakirkham commented 8 years ago

Oops, sorry, @ocefpaf, that showed up after the comment posted.

jakirkham commented 8 years ago

Sorry, I have not been keeping up-to-date here. Your incremental approach to support sounds totally reasonable to me @Korijn.

jakirkham commented 8 years ago

@jakirkham I have an odd suggestion: iterating over subsets of the configurations through two or three PRs. Is that acceptable, or do we really want to get below the build timeout?

Could you please explain what you have in mind for this, @Korijn?

jakirkham commented 8 years ago

It looks like CircleCI is parallellizing (there are 4 sub-builds) but they're all running the same steps. Shouldn't it split up over the different configurations? It gets py2.7 and py3.4 done, and runs out of time working on py3.5 currently.

Yeah, CircleCI doesn't have a build matrix like Travis CI does. 😞 Though it may be possible to use the parallelism feature that you were looking at to imitate this. See this discussion and this code. I'll put this in a conda-smithy issue so we can figure out if/how we can use this.

TravisCI jobs stop just before the test phase. :/ It might succeed with only 5 minutes more time.

😢 We have sometimes skipped some tests when this happens. Though I'd prefer to try cotire or perhaps try generating an XCode Project with CMake instead. I've never tried the latter, but maybe that affects the performance.

jakirkham commented 8 years ago

I also noticed that VTK builds its own copies of freetype, libpng and other libraries we have. Maybe there are CMake options to deactivate that?

Good catch, @ccordoba12. Yeah, this must save some build time if we skip building these and use our own. What do you think, @Korijn?

demarle commented 8 years ago

Yes, most of our TPLs have a VTK_USE_SYSTEM cmake option, please do try them. Of course, if there is a significant amount of drift between our version and whatever we find on the system they will not work.

jakirkham commented 8 years ago

Is there somewhere in the source or docs we should look to see what versions are used by VTK, @demarle?

Korijn commented 8 years ago

Something for the next PR then, I guess. :)

We're currently experiencing some troubles with offscreen rendering on headless servers using this package, and the docs seem to indicate that it requires a custom build, so I'll be exploring the options there as well in another PR. Having some trouble finding docs that are up to date though. E.g.:

Korijn commented 8 years ago

CircleCI responded and recommended us to look into parallellism, so that resonates with your suggestions @jakirkham

jakirkham commented 8 years ago

FYI I have disabled CircleCI parallelism on this feedstock as we don't have a way to leverage that currently. Hope that is ok.