Open hugovk opened 8 months ago
aclark4life
commented
8 months ago While I still support providing 32 bit wheels as described in #7443 until folks no longer notice if we stop, and while I support having one less vendor involved in this process (no offense to AppVeyor), it looks like the official word is no 32-bit on GHA and so we'd need a reliable work around.
radarhere
commented
8 months ago To start testing 32-bit Windows on GHA, the simplest way would just be to run the relevant part of the existing wheels job - the 32-bit Windows Wheels job tests all Python versions in 19 minutes, and testing just one Python version would be 10 minutes.
AppVeyor takes 24 minutes, so it's faster than that. It would be taking up parts of our GHA queue, but I don't think the final result would be definitively slower than our current form.
hugovk
commented
8 months ago Another option is to only test 32-bit on AppVeyor, so it would be ~10 minutes.
But it would be nice having just one CI vendor, less types of config to maintain.
aclark4life
commented
8 months ago But it would be nice having just one CI vendor, less types of config to maintain.
That's what I'm interested in as well. I'm leaning towards:
We probably should ask @cgohlke or someone else with lots of Windows experience to weigh in, but my feeling is we support our matrix running on Windows "latest" and not much else.
nulano
commented
8 months ago I thought GHA also had 32-bit available, but I can't find it right now.
x86 is tested using an x86_64 CPU with a 32-bit Python. Both AppVeyor and GHA provide this, it's only a matter of updating the config (i.e. reverting part of #7228).
Actually, #7228 removed testing 32-bit also on AppVeyor, I then suggested restoring that in https://github.com/python-pillow/Pillow/pull/7234#issuecomment-1609239161 as it was simpler to revert the changes there than on GHA; moving the 32-bit job to GHA would slightly complicate the test-windows.yml matrix.
Does OS version matter?
We currently claim to support Visual Studio 2017, which is only available on AppVeyor using the Windows Server 2016 image; the GHA windows-2019 image only has Visual Studio 2019. I'm also hesitant about using older windows images on GHA after the (at least initially) short deprecation window of the windows-2016 image (#4188).
3.8 on on 64-bit Windows Server 2017 is "too old to care about"
We claim to "support all current versions of Linux, macOS, and Windows". If that includes security support, we should keep testing Windows Server 2016 until 12 Jan 2027.
My main concern with removing AppVeyor would be that the build scripts could get too "locked in" with GHA, although that should not be too much of a concern after #4495. At the very least, I think we should keep testing with at least two different Visual Studio versions to make sure the scripts are not "locked in" with a particular version of Visual Studio, which could make it difficult for people to compile Pillow locally.
hugovk
commented
8 months ago Thanks @nulano!
It's not been a big problem having AppVeyor in addition to GHA. It would indeed be nice to have just provider, but broad and reliable test coverage is more important. It's good to review things once in a while.
What do you think about using a single job on AppVeyor, combining 32-bit (either 32-bit CPU or 64-bit CPU with 32-bit Python) with Windows Server 2016?
As mentioned, the AppVeyor duration isn't too slow compared to GHA, so I'm also fine to keep 2 jobs, it doesn't add much complexity.
Correcting my OP:
AppVeyor tests:
- 3.8 on x86-64 (Windows Server 2017)
- 3.12 on x86 (Windows Server 2022)
That should have read Windows Server 2016 not 2017; I mixed up that the image has Visual Studio 2017.
nulano
commented
8 months ago What do you think about using a single job on AppVeyor, combining 32-bit (either 32-bit CPU or 64-bit CPU with 32-bit Python) with Windows Server 2016?
We are currently testing:
Given that the Windows Server 2016 image only has Python 3.8 and older, does it make sense wait until Python 3.8 EOL on 31 Oct 2024 to decide this? We will need to then figure out how to install a newer Python version on the Windows Server 2016 image (assuming it is even possible) or change to a newer image anyway. We could then have either:
hugovk
commented
8 months ago Yes, it can wait.
We'll be dropping Python 3.8 in Pillow 11.0 released in October, so I've added this to the milestone as a reminder.
We can already drop 3.8 from the codebase and CI after the preceding 10.4 is released in ~3 months :)
radarhere
commented
4 months ago For the moment at least, https://github.com/python-pillow/Pillow/pull/8183 aims to move from Python 3.8 on Windows Server 2016 to Python 3.9 on 2019.
hugovk
commented
2 months ago Well, any thoughts on this? Is AppVeyor giving us any value over GitHub Actions?
aclark4life
commented
2 months ago I don't remember all the details of all the issues described above, off hand, but for example, does Actions support Windows Server 2016, as @nulano says we're self-committed to supporting until 2027? I remain equally committed to:
So if AppVeyor is extra now then maybe it can go. If not maybe Jan 2025?
radarhere
commented
2 months ago We could add a 32-bit GHA Windows build and drop AppVeyor without losing any value from what we currently have. It might just lock us into the mindset of giving up on Windows Server 2016 on AppVeyor.
radarhere
commented
1 month ago I've created #8437 as a way that Windows Server 2016 could be restored on AppVeyor, if the goal is to keep testing it.
hugovk
commented
3 weeks ago Please see https://github.com/python-pillow/Pillow/pull/8525 to test oldest Python (3.9) on 32-bit Windows 2019 and remove Appveyor.
Are we still getting value from AppVeyor compared to the Windows tests on GitHub Actions?
AppVeyor tests:
Which of these are important? I expect 32-bit, whilst we're supporting it. Does OS version matter?
GitHub Actions tests:
GHA also has Windows Server 2019 available:
I thought GHA also had 32-bit available, but I can't find it right now.