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

gh-115582 and gh-115545: Windows release build mixes up free-threaded files #98

Merged
merged 6 commits into from
Feb 26, 2024

Conversation

zooba
Copy link
Member

@zooba zooba commented Feb 19, 2024

No description provided.

@zooba zooba changed the title gh-115545: Avoid building python_uwp.exe for free-threaded gh-115582 and gh-115545: Windows release build mixes up free-threaded files Feb 20, 2024
@zooba
Copy link
Member Author

zooba commented Feb 20, 2024

This straightens out the builds, and also simplifies a few things now that we run so many configurations.

I've tested that the correct packages are all created for unsigned builds. Need to run a signed build to make sure nothing else goes wrong, but otherwise this should fix everything except that nogil extensions won't have the right Py_GIL_DISABLED setting (unless they're using the Nuget packages). Because of the explicit #undef in pyconfig.h, they can't override it either, but that's a change to the main repo and one we should discuss in the context of all platforms (over at python/cpython#115582).

@zooba
Copy link
Member Author

zooba commented Feb 20, 2024

Also running test builds against 3.12 and 3.10 to make sure nothing has broken there. But that's enough for tonight.

@zooba
Copy link
Member Author

zooba commented Feb 22, 2024

@ambv I don't know if you're interested in reviewing this, but it should be ready to go. It's the way I should've done the freethreaded builds in the first place - as separate configurations. Luckily, thanks to the parallelism, the overall build doesn't take much longer.

@ambv ambv self-requested a review February 22, 2024 15:12
@ambv
Copy link
Contributor

ambv commented Feb 23, 2024

I read through this but ultimately it's a lot of YAML changes that look good to me but I'm not confident about judging either way. So to observe the changes I started build #151696 from this PR branch on the current cpython main without the "Publish" checkbox. There is a small risk doing that since you changed the "PublishPipelineArtifact@0" task into the built-in publish so we'll see if the checkbox is honored 😅

@ambv
Copy link
Contributor

ambv commented Feb 23, 2024

Hm, signing binaries failed across the board here even though I did approve it.

@zooba
Copy link
Member Author

zooba commented Feb 23, 2024

I'll check my Azure settings. I updated that key vault to the newer permissions model, so maybe it didn't stick properly?

@zooba
Copy link
Member Author

zooba commented Feb 23, 2024

Looks like it didn't stick :) I restarted the signing jobs

@zooba
Copy link
Member Author

zooba commented Feb 23, 2024

There is a small risk doing that since you changed the "PublishPipelineArtifact@0" task into the built-in publish so we'll see if the checkbox is honored

Those tasks are just internal publishes. The actual checkbox impacts the entire Publish stage, so you'll see as soon as it starts running whether there's a chance it'll go public (this one won't).

@zooba
Copy link
Member Author

zooba commented Feb 23, 2024

Well done finding the one combination of build options I missed 😆

@ambv
Copy link
Contributor

ambv commented Feb 23, 2024

That's why they pay me the big bucks haha

@ambv
Copy link
Contributor

ambv commented Feb 23, 2024

I started another one to see it go to completion.

@zooba
Copy link
Member Author

zooba commented Feb 23, 2024

You'll be able to grab the built files from the Artifacts page to try them out if you want. msi will have the installer in it.

@zooba
Copy link
Member Author

zooba commented Feb 26, 2024

@ambv Are you happy enough with the changes?

@ambv ambv merged commit 58a7c91 into master Feb 26, 2024
3 checks passed
@ambv
Copy link
Contributor

ambv commented Feb 26, 2024

Yeah, thanks! I went to Czechia and back over the weekend, so I didn't have access to a Windows box to test the installers. I assume they work :)

@zooba zooba deleted the gh-115545 branch February 26, 2024 20:23
@zooba
Copy link
Member Author

zooba commented Feb 26, 2024

They work at least as well as in the last alpha 😆

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

Successfully merging this pull request may close these issues.

2 participants