Skip to content

CI: make sure CI stays on VS2019 unless changed explicitly #20524

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

Merged
merged 1 commit into from
Dec 6, 2021

Conversation

h-vetinari
Copy link
Contributor

To avoid the deprecation warnings on azure for the old windows images, #20234 moved to windows-latest. This is currently equivalent to windows-2019 (bringing visual studio 2019, and corresponding toolset), but will presumably change to windows-2022 at some point in the future.

Such a change of compiler version should be made consciously for the numpy CI (rather than imposed by what azure does), because the compiler version used for windows effectively defines the lower bound of support on windows (as otherwise, changes that break on older compilers will not be caught in CI).

CC @charris @rgommers

PS. If this is received favourably, I'm happy to do the same for scipy.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 good idea. Thanks @h-vetinari

@mattip mattip merged commit 0749f57 into numpy:main Dec 6, 2021
@mattip
Copy link
Member

mattip commented Dec 6, 2021

Thanks @h-vetinari

@h-vetinari h-vetinari deleted the vs2019 branch December 6, 2021 21:59
@h-vetinari
Copy link
Contributor Author

Cool! Should we backport this to 1.22?

@charris
Copy link
Member

charris commented Dec 7, 2021

Cool! Should we backport this to 1.22?

Yes. vs2019 should be good until 2024 (IIRC). If we want consistency, might want to change the wheels builds as well. Scipy-wheels build on appveyor using vs2017.

@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Dec 7, 2021
@h-vetinari
Copy link
Contributor Author

Scipy-wheels build on appveyor using vs2017.

Are we talking about https://github.com/MacPython/scipy-wheels? There are no windows jobs in that repo (anymore?). When I researched the scipy windows build infra at the time of scipy/scipy#13713, all signs pointed to the fact that these were built in the main repo, and this passed review at the time. I'm more than happy to correct that info if I was wrong of course - perfect chance with scipy/scipy#15167

@charris
Copy link
Member

charris commented Dec 7, 2021

See appveyor.yml at https://github.com/MacPython/scipy-wheels.

@charris
Copy link
Member

charris commented Dec 7, 2021

That said, it might be easier for someone used to the repository. @tylerjereddy @rgommers Thoughts?

@h-vetinari
Copy link
Contributor Author

Ah, sorry, I overlooked this. Thanks for the pointer!

@h-vetinari
Copy link
Contributor Author

@ Ralf & Tyler

Is there a reason not to use azure-pipelines for the windows builds as well? I'm worried that the gap between the CI on the main repo (VS2019) and on the wheels-repo (VS2017) will soon lead to divergences that are a big timesink. Alternatively, the wheels-repo would have to move to VS2019 as well, at which point the question arises why not azure-pipelines directly?

@mattip
Copy link
Member

mattip commented Dec 7, 2021

Perhaps scipy would be interested in adopting the work to use cibuildwheel in numpy, xref tracking issue #20262

@rgommers
Copy link
Member

rgommers commented Dec 7, 2021

Yes, we are not attached to Appveyor and would be happy to get rid of it. Either as a one-off migration to Azure, or as part of a larger cibuildwheel overhaul. The latter should happen over the next year, but not clear when exactly.

@h-vetinari
Copy link
Contributor Author

or as part of a larger cibuildwheel overhaul. The latter should happen over the next year, but not clear when exactly.

@rgommers
Is there an issue/thread/repo to track this? Searching for cibuildwheel here, I only found a more recent comment:

To be considered after the ongoing migration to cibuildwheel is complete.

... which also doesn't yield much more insight, though #20102 looks like some progress was made already, and that @lithomas1 has some PRs (both merged and open) in that area.

I see that some repos under https://github.com/MacPython already use cibuildwheel, but also that some are moving wheel-building directly to the main repo. Is that the goal for numpy/scipy as well?

Background: I wanted to get restart the efforts to update scipy wheel builder (vendored gfortran, vs2019, etc.), and wanted to gauge if I should wait/not wait for (or somehow contribute to) the cibuildwheel stuff.

@h-vetinari
Copy link
Contributor Author

Ping @rgommers

Should I perhaps open an issue to track the cibuildwheel migration/integration?

@rgommers
Copy link
Member

Should I perhaps open an issue to track the cibuildwheel migration/integration?

That may be useful as a SciPy issue. That said, the plan is "wait till NumPy is completely done with the migration, then just copy the NumPy setup to SciPy". And since NumPy is not yet done, I'm not sure there is anything to do right now.

@h-vetinari
Copy link
Contributor Author

The thing is I wanted to know primarily is the status and if makes sense to wait for it / contribute to it...

@rgommers
Copy link
Member

Waiting then I'd say. Copying things over should be quick, because the numpy/scipy setups are very similar.

@mattip
Copy link
Member

mattip commented Feb 13, 2022

The last two pieces are

@lithomas1
Copy link
Collaborator

I think I also need to add something to build the sdist(This is pretty easy and I'll try to put up a PR sometime this week).
We are also considering adding universal2 wheels.

@rgommers
Copy link
Member

Yes, building the sdist would be great! We've always done that in the wrong way.

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

Successfully merging this pull request may close these issues.

5 participants