Skip to content
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

Do we still need AppVeyor? #7910

Open
hugovk opened this issue Mar 27, 2024 · 8 comments
Open

Do we still need AppVeyor? #7910

hugovk opened this issue Mar 27, 2024 · 8 comments

Comments

@hugovk
Copy link
Member

hugovk commented Mar 27, 2024

Are we still getting value from AppVeyor compared to the Windows tests on GitHub Actions?

AppVeyor tests:

  • 3.8 on x86-64 (Windows Server 2017)
  • 3.12 on x86 (Windows Server 2022)

Which of these are important? I expect 32-bit, whilst we're supporting it. Does OS version matter?

GitHub Actions tests:

  • 3.8-3.13 and PyPy3.9-3.10 on x86-64 (Windows Server 2022)

GHA also has Windows Server 2019 available:

I thought GHA also had 32-bit available, but I can't find it right now.

@aclark4life
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

hugovk commented Mar 27, 2024

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
Copy link
Member

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:

  • 3.8 on on 64-bit Windows Server 2017 is "too old to care about"
  • 3.12 on 32-bit Windows 2022 should remain in some form but not on AppVeyor.

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
Copy link
Contributor

nulano commented Mar 27, 2024

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 #7234 (comment) 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
Copy link
Member Author

hugovk commented Mar 28, 2024

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
Copy link
Contributor

nulano commented Mar 28, 2024

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:

  • the newest available 32-bit Python on Windows Server 2022,
  • the oldest supported 64-bit Python on Windows Server 2016.

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:

  • the newest available 32-bit Python on Windows Server 2016, or if no Python version supports it
  • the newest available 32-bit Python on Windows Server 2019 (either on GHA or AV).

@hugovk hugovk added this to the 11.0.0 milestone Mar 28, 2024
@hugovk
Copy link
Member Author

hugovk commented Mar 28, 2024

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants