Skip to content

bpo-40432: Use python 3.8 or higher to compile cpython on windows#25389

Merged
zooba merged 5 commits into
python:masterfrom
Fidget-Spinner:find-python-3.8
Apr 26, 2021
Merged

bpo-40432: Use python 3.8 or higher to compile cpython on windows#25389
zooba merged 5 commits into
python:masterfrom
Fidget-Spinner:find-python-3.8

Conversation

@Fidget-Spinner
Copy link
Copy Markdown
Member

@Fidget-Spinner Fidget-Spinner commented Apr 13, 2021 •
edited
Loading

Some regen grammar scripts use the walrus := operator and so they need Python 3.8 or higher to compile. Hopefully this fixes the AMD64 Windows10 Pro build.

Co-Authored-By: Jeremy Kloth 20042737+jkloth@users.noreply.github.com

https://bugs.python.org/issue40432

@Fidget-Spinner Fidget-Spinner mentioned this pull request Apr 13, 2021
Merged
@Fidget-Spinner Fidget-Spinner changed the title Use python 3.8 or higher to compile cpython on windows bpo-40432: Use python 3.8 or higher to compile cpython on windows Apr 13, 2021
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 13, 2021
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Apr 13, 2021

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 781fc83 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 13, 2021
@jkloth
Copy link
Copy Markdown
Contributor

jkloth commented Apr 13, 2021

The PR does indeed solve the issue. The worker compiles and completes the tests (with warnings, but that is unrelated to this change)

@Fidget-Spinner Fidget-Spinner marked this pull request as ready for review April 13, 2021 16:04
@Fidget-Spinner Fidget-Spinner requested a review from a team as a code owner April 13, 2021 16:04
jkloth
jkloth approved these changes Apr 13, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@jkloth jkloth left a comment

Choose a reason for hiding this comment

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

LGTM and passes the PR check!

Comment thread PCbuild/find_python.bat
@Fidget-Spinner
Copy link
Copy Markdown
Member Author

Fidget-Spinner commented Apr 13, 2021

The PR does indeed solve the issue. The worker compiles and completes the tests (with warnings, but that is unrelated to this change)

Thanks for taking the time to set up a buildbot worker! Without it this may have gone unnoticed longer.

@pablogsal pablogsal requested a review from zooba April 13, 2021 23:36
@pablogsal
Copy link
Copy Markdown
Member

pablogsal commented Apr 14, 2021

@Fidget-Spinner please, ping me if @zooba didn't have time to review this in the following days so I can review it.

@Fidget-Spinner @jkloth
Update find_python.bat
83689f0 
Apply suggestions by Jeremy

Co-Authored-By: Jeremy Kloth <20042737+jkloth@users.noreply.github.com>
@Fidget-Spinner Fidget-Spinner added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 14, 2021
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Apr 14, 2021

🤖 New build scheduled with the buildbot fleet by @Fidget-Spinner for commit 83689f0 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 14, 2021
@jkloth
Copy link
Copy Markdown
Contributor

jkloth commented Apr 17, 2021

While this is still a valid issue with the build scripts, I have now installed 3.8 on my buildbot so as to finally see it useful.

The reason my buildbot has this problem is that I have installed Visual Studio 2019 with the Python component selected. This component also installs Py3.7. David's buildbot uses the 2017 Build Tools which means the logic in find_python falls through to fetching the latest build from nuget.

zooba
zooba reviewed Apr 23, 2021
View reviewed changes
Comment thread PCbuild/find_python.bat Outdated
@zooba zooba merged commit 425434d into python:master Apr 26, 2021
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Apr 26, 2021

@zooba: Please replace # with GH- in the commit message next time. Thanks!

@zooba
Copy link
Copy Markdown
Member

zooba commented Apr 26, 2021

That test failure was asyncio, unrelated to this change.

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Apr 26, 2021

Thanks @Fidget-Spinner for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Apr 26, 2021

GH-25637 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 26, 2021
@Fidget-Spinner @miss-islington
bpo-40432: Use python 3.8 or higher to compile CPython on Windows (py…
ed18b5d 
…thonGH-25389)

(cherry picked from commit 425434d)

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
ambv pushed a commit that referenced this pull request Apr 26, 2021
@miss-islington @Fidget-Spinner
bpo-40432: Use python 3.8 or higher to compile CPython on Windows (GH…
f65f3f0 
…-25389) (#25637)

(cherry picked from commit 425434d)

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
@Fidget-Spinner
Copy link
Copy Markdown
Member Author

Fidget-Spinner commented Apr 26, 2021

@zooba , thanks for reviewing this PR and merging it!

@Fidget-Spinner Fidget-Spinner deleted the find-python-3.8 branch April 28, 2021 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zooba zooba zooba left review comments

@pablogsal pablogsal Awaiting requested review from pablogsal

+1 more reviewer

@jkloth jkloth jkloth approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Fidget-Spinner @bedevere-bot @jkloth @pablogsal @zooba @miss-islington @the-knights-who-say-ni