-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Adds macOS and Windows builds to Azure Pipelines #12982
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
Conversation
So it's probably obvious, but as a CI-related change, this is going to take me a few goes. Do you normally squash merge at the end? Or would you like me to do it before declaring it ready to merge? |
We squash-merge at our discretion, whichever is easier for. |
The windows failure is missing libpng (it looks like it is going to try to (correctly) handle the freetype dependency on it's own. Not sure what is going on with the linux failures... |
Was just taking a look at that. Is there an environment variable or something that points to where the png files are? Something I can set to point at wherever I download files to? |
Lines 1061 to 1064 in 0fec7f1
is where we check if we can find it the png headers. Chasing that back it looks like setting |
The linux failure definitely looks like a bug in our afm handling code. May I push commits to this branch to debug that (mostly to sort out what font we are not parsing right)? Otherwise, mark that test as a known-fail and create an issue for it. |
Give me 45 minutes to keep trying to get the Windows build at least building, and then go for it. Otherwise the pull/push is going to get real annoying real quick. |
@zooba I'll do it this evening (~8hrs from now). |
It looks like Freetype 2.9 supports building with VS 2015+ now (through the same solution file as for VS 2010). Am I right in thinking that MPL only supports 3.5 or later these days? That will simplify FreeType.do_custom_build significantly (it should really simplify down to finding MSBuild, and let it do the rest of discovery), though I'm not sure what your back-compat needs look like here. |
We're stuck on an old freetype version because freetype's rasterization changes across versions, so changing the freetype version would mean regenerating the entire list of test baseline images and we've been postponing this until we can figure out a way that does not involve adding another dozen megabytes to the repo for each freetype release... |
Good to know, though unfortunately that means it won't build on Pipelines because the image doesn't have such an old compiler available. Maybe there's somewhere else we can push sets of test images and then download from there based on the available version? Rather than checking them in. |
maybe we need to go the sub-module route with a branch / tag(?) for the outer-product of freetype and Matplotlib versions? We do occasionally update the test images, the biggest sticking point on moving to hosting them else where was not having a good story for updating them when needed. @QuLogic has a patch that gets applied to the fedora installs that bumps the tolerance on the image tests to allow them to pass despite the text moving around a bit (we used to have a higher default tolerance, but then we had issues with completely wrong text symbols passing the tests). That may be the quickest path forward here. I assume we do not want to ask why old versions of freetype do not build on new versions of VS? |
It's probably just considered a new feature, and so it only gets added to a new version. Or it just wasn't relevant until they'd already moved onto the next version. There are likely some special cases in code that have to change -- VC10->VC14 became more standards-compliant, which means some names lost leading underscores and others gained them, but now there are some weird incompatibilities with not-quite-standard compilers such as all of them :) |
What's exactly the problem with sticking to FreeType2.6? Right now FreeTyppe.do_custom_build is also basically just downloading the sources and invoking MSBuild on it, but perhaps I'm missing something... |
It requires VS 2010, which is not supported by Microsoft and is not installed (or installable) on Azure Pipelines. By contrast, the reason for sticking with it is to avoid regenerating test data. So one of these interests should win. I'm not going to presume to dictate which. |
How to migrate the test images to a new freetype is a problem we (I) have been assiduously kicking down the road. This is a good reason to finally address that. I stand by the right short-term solution is to use whatever version of freetype will build and bump the dependancies. Even though the images are not pixel identical we can put off turning off appveyor (maybe just pull back how many versions we test on it) and we will still catch many bugs (for example the 3.7.1 import issue or any windows-only logic errors). |
As long as we don't commit another 30Mb of baseline images to the repo, anything else works for me. |
There was an extra layer of indentation.
Expect this to fail tests, but trying to get it to build.
What follows is more-or-less stream of concious notes I took while working on this sorry @zooba I thought this would be a simple configuration thing and it has turned out to be a morass.... Tried blindly changing to vs14 and hoping the comment @jankatins left is wrong (no, it did not work as expected). Other points of refernce cf has a recipe that builds freetype https://github.com/conda-forge/freetype-feedstock/tree/master/recipe . It looks like they have to apply a bunch of patches and use cmake. @dopplershift has 2.6.1 conda builds (but I can not find them). There is a git repository of built version https://github.com/ubawurinna/freetype-windows-binaries (but does not include 2.6.1, but does include 2.6.3). The FT 2.9.1 also does not include as vs14 project. It his visualc and visualce, not sure if those are helpful? It also seems that we were building FT on appveyor anyway (despite it being installed by conda) pushed a commit fix that. We now get FT 2.6.3 which does infact fail tests (@dopplershift where is your 2.6.1 freetype conda package?) As for the font, it seems to be |
I don't have a patch; I regenerate all test images with new FreeType and review them to make sure they aren't completely broken. Only on 32-bit systems do I increase tolerance, because way too many images are not the same there. You can find the changed images at https://github.com/QuLogic/mpl-images (note: there's no commit for 3.0.2 because no images changed since 3.0.1, so I didn't bother.) |
@tacaswell I don't have a cf build of free type 2.6.1, only matplotlib packages on CF that are using the internal build of 2.6.1 rather than the CF freetype. |
(As a tangent, the issue in #12982 (comment) would have been better resolved IMO by merging #11983 and then just using OS-standard evnrionement variables ($CFLAGS/$CPATH on unices, %CL% on Windows) to set the include path.) |
From Christof Gohlke
@dopplershift curses 😞 @QuLogic ok, that makes sense. |
For the record, I get freetype 2.6.1 to build with a recent VS (from this fall) with the following do_custom_build():
Stuff crashes later with numpy failing to import but it's probably unrelated... Unfortunately it's not clear whether there's really a good solution to avoid hardcoding the version of the latest installed SDK per https://developercommunity.visualstudio.com/content/problem/198082/upgrading-a-solution-from-the-command-line-with-vi.html, though some stackoverflow answers suggest that we could lookup that version from the registry. If anyone wants to take over from here... (The trick of using cc.spawn was found while writing the mplcairo build scripts while trying to look for a "public" API to the use the paths that distutils.msvc9compiler looks up and carefully stashes in private attributes...) |
As requested by @tacaswell :)