Skip to content

Conversation

@savannahostrowski
Copy link
Member

@savannahostrowski savannahostrowski commented Feb 6, 2026

Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

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

Thanks for this!

@hugovk
Copy link
Member

hugovk commented Feb 6, 2026

Is this clang-20 also the LLVM number? (in three places)

-CC=clang-20 ...
+CC=clang-${{ matrix.llvm }} ...

@hugovk
Copy link
Member

hugovk commented Feb 6, 2026

Is this clang-20 also the LLVM number? (in three places)

-CC=clang-20 ...
+CC=clang-${{ matrix.llvm }} ...

Included in savannahostrowski#15, which also combines the Linux steps.

@hugovk
Copy link
Member

hugovk commented Feb 6, 2026

savannahostrowski#16 combines two Windows steps.

That will leave us with these Windows jobs:

#          - target: i686-pc-windows-msvc/msvc
#            architecture: Win32
#            runner: windows-2022
          - target: x86_64-pc-windows-msvc/msvc
            architecture: x64
            runner: windows-2025-vs2026
            build_flags: ""
            run_tests: true
          - target: x86_64-pc-windows-msvc/msvc-free-threading
            architecture: x64
            runner: windows-2025-vs2026
            build_flags: --disable-gil
            run_tests: false
#          - target: aarch64-pc-windows-msvc/msvc
#            architecture: ARM64
#            runner: windows-2022

Note Win32 and ARM64 are commented out, and have been since initial commit exactly one year ago! 🎂 cb640b6

Only Windows x64 architecture is run. I suggest we delete the commented code, we can re-add it when we're ready.

And after combining two of the three Windows steps into one, we have two remaining:

      - name: Native Windows MSVC (release)
        if: runner.os == 'Windows' && matrix.architecture != 'ARM64'
        shell: pwsh
        run: |
          $env:PlatformToolset = "v145"
          ./PCbuild/build.bat --tail-call-interp ${{ matrix.build_flags }} -c Release -p ${{ matrix.architecture }}
          if ("${{ matrix.run_tests }}" -eq "true") {
            ./PCbuild/rt.bat -p ${{ matrix.architecture }} -q --multiprocess 0 --timeout 4500 --verbose2 --verbose3
          }

      # No tests (yet):
      - name: Emulated Windows Clang (release)
        if: runner.os == 'Windows' && matrix.architecture == 'ARM64'
        shell: pwsh
        run: |
          choco install llvm --allow-downgrade --no-progress --version ${{ matrix.llvm }}.1.0
          $env:PlatformToolset = "clangcl"
          $env:LLVMToolsVersion = "${{ matrix.llvm }}.1.0"
          $env:LLVMInstallDir = "C:\Program Files\LLVM"
          ./PCbuild/build.bat --tail-call-interp -p ${{ matrix.architecture }}

I don't think that second one has ever been run? I suggest we delete that too. And when we're ready, we can re-add as needed. And that might be by combining into the first Windows step instead of a standalone one.

The && matrix.architecture != 'ARM64' guard in this first step is also redundant and can be deleted.

@savannahostrowski
Copy link
Member Author

Thanks for the pulls @hugovk! I was initially trying to be somewhat conservative but I'm inclined to remove the unused job and commented out platforms as well. I think it'll be obvious enough if/when we need to add them back in.

@hugovk
Copy link
Member

hugovk commented Feb 7, 2026

savannahostrowski#17 removes the duplicate target list, and then I have one more suggestion that builds on top of that.

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Thanks!

@github-project-automation github-project-automation bot moved this from 🧐 @webknjaz's review queue 📋 to ⏰ Merge reminder needed 🔀 in 📅 Procrastinating in public 😵‍💫 Feb 9, 2026
@savannahostrowski savannahostrowski merged commit 30cfe6e into python:main Feb 9, 2026
34 checks passed
@github-project-automation github-project-automation bot moved this from ⏰ Merge reminder needed 🔀 to 🌈 Done 🦄 in 📅 Procrastinating in public 😵‍💫 Feb 9, 2026
@hugovk
Copy link
Member

hugovk commented Feb 10, 2026

And let's also backport.

@hugovk hugovk added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Feb 10, 2026
@miss-islington-app
Copy link

Thanks @savannahostrowski for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks @savannahostrowski for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @savannahostrowski, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 30cfe6ee23a8974625c9860bc401f60ed75b2dea 3.13

@miss-islington-app
Copy link

Sorry, @savannahostrowski, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 30cfe6ee23a8974625c9860bc401f60ed75b2dea 3.14

@hugovk
Copy link
Member

hugovk commented Feb 10, 2026

Before merging this PR, here's the diff between 3.14 and main:

image image

Anything on the RHS that should not go back to 3.14? Can we just copy the current main version back?

(And the file doesn't exist on 3.13, no need to backport there.)

@hugovk hugovk removed the needs backport to 3.13 bugs and security fixes label Feb 10, 2026
savannahostrowski added a commit to savannahostrowski/cpython that referenced this pull request Feb 10, 2026
(cherry picked from commit 30cfe6e)

Co-authored-by: Savannah Ostrowski <savannah@python.org>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented Feb 10, 2026

GH-144683 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Feb 10, 2026
@chris-eibl
Copy link
Member

Anything on the RHS that should not go back to 3.14? Can we just copy the current main version back?

3.14 cannot use VS2026 (PlatformToolSet v145) for a tailcalling build. The necessary changes are only in 3.15 (#139922)

So on 3.14 we can only use clang (or just omit it to save CI).

cc @Fidget-Spinner

hugovk added a commit that referenced this pull request Feb 10, 2026
Co-authored-by: Savannah Ostrowski <savannah@python.org>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
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