Skip to content

Conversation

@nulano
Copy link
Contributor

@nulano nulano commented Jun 16, 2020

Add cache for Windows builds on GHA to cut build times almost in half. Using key based on test-windows.yml, build_prepare.py, and the current Python location (unique per Python minor version). The only other variable affecting builds should be the VS install location, which is unlikely to change.

I disabled libimagequant on GHA to simplify this, it is still enabled on AppVeyor. To make sure caching is available for PRs (see note below), libimagequant is now always built (takes 5s), but it is disabled in the Build Pillow and Build Wheel steps if building wheels.

There is a step to delete all intermediate files (~180MB on my system), the remaining size is ~30MB on my system, which seems to be getting compressed down to ~8MB on GHA per build configuration.

Note that for security reasons, PRs only have access to caches from upstream/master.
See run on my repo or re-run the Windows job to see the effect here.

@radarhere
Copy link
Member

So the downside of this is the fact that libimagequant isn't tested anymore. Perhaps that's a minor downside, and perhaps AppVeyor is enough, but the Windows GHA jobs are slightly faster than the macOS and Linux GHA jobs, so Windows isn't a bottleneck when running the jobs either.

Others may disagree with me, it just seems like an odd move in the history of our jobs transitioning from AppVeyor to GHA.

@hugovk
Copy link
Member

hugovk commented Jun 21, 2020

I agree, it would be nice to have libimagequant on GHA if possible.

I disabled libimagequant on GHA to simplify this, it is still enabled on AppVeyor.

How complicated would it be to keep it?

@nulano
Copy link
Contributor Author

nulano commented Jun 21, 2020

I figured out how to keep it enabled - compile it always (takes 5s), but then disable it in the Build Pillow step. See new commit.

@hugovk
Copy link
Member

hugovk commented Jun 27, 2020

So imagequant is now available for PRs, but not for pushes (eg. normal builds on mater)?

What's the reasoning for disabling for pushes?

@nulano
Copy link
Contributor Author

nulano commented Jun 27, 2020

This is the same as the current behaviour. It is disabled for pushes because that is when wheels are selected to build, and imagequant is GPL. (This is not a problem for Raqm because it is LGPL and linked at runtime.)

The idea is to make it easier to test builds for contributors (especially before #4495 was done). Another option would be to build wheels only on tag creation (e.g. release)

@hugovk hugovk merged commit 57c4be1 into python-pillow:master Aug 14, 2020
@hugovk
Copy link
Member

hugovk commented Aug 14, 2020

thanks!

@nulano nulano deleted the winbuild-cache branch October 14, 2020 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants