GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-154200: Use sysconfig.get_platform() in test_importlib.test_windows by SynaptSea · Pull Request #154210 · python/cpython

Conversation

@SynaptSea

@SynaptSea SynaptSea commented Jul 20, 2026 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Fixes #154200

test_tagged_suffix computes its expected extension suffix with a vendored
"Port of distutils.util.get_platform()" that sniffs sys.version.lower() for 'amd64'.

Python/getversion.c truncates the compiler segment of sys.version at 80 characters
("%.80s"). On clang-cl builds the banner is
"[Clang " __clang_version__ "] 64 bit (AMD64) with MSC v.<n> CRT]", and official LLVM
Windows binaries have an 86-character __clang_version__, so the (AMD64) marker is always
cut off. The helper falls through to return sys.platform -> 'win32', and the test expects
.cp316t-win32.pyd while the interpreter correctly reports .cp316t-win_amd64.pyd:

AssertionError: '.cp316t-win32.pyd' not found in ['.cp316t-win_amd64.pyd', '.pyd']

sysconfig.get_platform() no longer has this problem: since gh-145410 it does not parse
sys.version on Windows but delegates to the _sysconfig C accelerator, which derives the
platform from compile-time architecture macros and is compiler-agnostic. So this uses it and
deletes the vendored copy, which that call site was the only consumer of.

This is not circular: SYSCONFIG_PLATFORM in Modules/_sysconfig.c and PYD_PLATFORM_TAG in
PC/pyconfig.h are independent definitions in separate files, so the assertion still
cross-checks two separate derivations.

It also drops the VSCMD_ARG_TGT_ARCH branch, which trusted the ambient Visual Studio
developer-prompt target architecture over the interpreter actually executing the test.
That was backwards — EXTENSION_SUFFIXES is compiled into the running binary — and it is
why the failure only reproduced outside a VS developer prompt.

Testing

On a local clang-cl free-threaded build, python -m unittest test.test_importlib.test_windows:

  • before: Ran 11 tests ... FAILED (failures=2) (the Frozen_ and Source_ variants)
  • after: Ran 11 tests ... OK

Also OK with VSCMD_ARG_TGT_ARCH=x64, and with VSCMD_ARG_TGT_ARCH=x86 deliberately
mismatched against this amd64 interpreter — the old code would have computed 'win32' and
failed there. Full python -m test test_importlib: run=1229 skipped=23, SUCCESS.

Note the re.sub() is still required: sysconfig.get_platform() returns win-amd64 with a
hyphen while PYD_PLATFORM_TAG uses win_amd64. os and re both remain in use elsewhere
in the module, so no imports become unused.

NEWS

This is a test-only change, which the devguide lists as not requiring a Misc/NEWS.d entry,
so I have not added a blurb — could a maintainer apply skip news? Happy to add one instead
if you would prefer, on the grounds that it unblocks a whole build configuration.

  • Issue: test_importlib.test_windows: vendored get_platform() misdetects clang-cl builds as win32, failing test_tagged_suffix #154200

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@SynaptSea
pythongh-154200: Use sysconfig.get_platform() in test_importlib.test_…
d63344e
…windows

test_tagged_suffix computed its expected extension suffix with a vendored port of
distutils.util.get_platform() that sniffs sys.version for 'amd64'. Python/getversion.c
truncates the compiler segment of sys.version at 80 characters, so on clang-cl builds --
whose banner carries a long __clang_version__ -- the marker is cut off, the helper falls
through to sys.platform ('win32'), and the test expects '.cp316t-win32.pyd' while the
interpreter correctly reports '.cp316t-win_amd64.pyd'.

sysconfig.get_platform() no longer parses sys.version on Windows (pythongh-145410); it uses the
_sysconfig C accelerator, which derives the platform from compile-time architecture
macros and is compiler-agnostic. Use it and drop the vendored copy, its only caller.

This also removes the VSCMD_ARG_TGT_ARCH branch, which trusted the ambient Visual Studio
target architecture over the interpreter actually running the test.
Copilot AI review requested due to automatic review settings July 20, 2026 01:17
@SynaptSea
SynaptSea requested review from FFY00, brettcannon, ericsnowcurrently, ncoghlan and warsaw as code owners July 20, 2026 01:17
Copilot started reviewing on behalf of SynaptSea July 20, 2026 01:18
View session
Copilot AI reviewed Jul 20, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a Windows-only test failure in test_importlib.test_windows by replacing a local, sys.version-parsing platform detector with sysconfig.get_platform(), which is compiler-agnostic on Windows and avoids truncation-related mis-detection on clang-cl builds.

Changes:

  • Remove the vendored get_platform() helper that relied on VSCMD_ARG_TGT_ARCH and sys.version parsing.
  • Compute the expected .pyd platform tag using sysconfig.get_platform() and keep normalization via re.sub(..., '_', ...) for win-amd64 → win_amd64.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@chris-eibl
chris-eibl self-requested a review July 20, 2026 10:04
@chris-eibl chris-eibl added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 20, 2026
@bedevere-bot

bedevere-bot commented Jul 20, 2026

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @chris-eibl for commit d63344e 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F154210%2Fmerge

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 20, 2026
@brettcannon brettcannon added the skip news label Jul 20, 2026
@chris-eibl

chris-eibl commented Jul 21, 2026

Copy link
Copy Markdown
Member

Since version 22, clang-cl returns a much longer __clang_version__. I think it would make sense to switch to

#if defined(__clang__)
#define COMPILER "[Clang " __clang_major__ "." __clang_minor__ "." __clang_patchlevel__ "]"

like suggested in #145410 (comment).

This would also fix #154199.

Nevertheless, this change here lgtm, too.

Fwiw, during EP2026 sprints @zware setup a clang-cl buildbot.

Cc @encukou .

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@chris-eibl

chris-eibl commented Jul 21, 2026

Copy link
Copy Markdown
Member

Fwiw, during EP2026 sprints @zware setup a clang-cl buildbot.

See python/buildmaster-config#764

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@chris-eibl
Merge branch 'main' into fix-test-windows-platform
0cb4a36
@python-cla-bot

python-cla-bot Bot commented Jul 21, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot mentioned this pull request Jul 21, 2026
test_importlib.test_windows: vendored get_platform() misdetects clang-cl builds as win32, failing test_tagged_suffix #154200
Closed
@chris-eibl

chris-eibl commented Jul 21, 2026

Copy link
Copy Markdown
Member

I do not know why the CLA step got stuck. I've clicked "update branch" in the hope it works now ...

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@chris-eibl

chris-eibl commented Jul 21, 2026

Copy link
Copy Markdown
Member

Yupp, that did it :)

@SynaptSea can you please sign the CLA?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou

encukou commented Jul 22, 2026

Copy link
Copy Markdown
Member

Cc @encukou

I trust your judgement here. My main concern is that supported platforms don't break :)
Steve Dower has better opinions on how Windows builds/tests should work.

Fwiw, during EP2026 sprints @zware setup a clang-cl buildbot.

Great news!

Please keep using OS-unsupported tag on issues/PRs though: that'll make them (eventually) get listed in the project. If/when clang-cl gets a PEP-11 tier, we can add a clang-cl tag and put it on the older issues.

chris-eibl reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@chris-eibl

chris-eibl commented Jul 22, 2026

Copy link
Copy Markdown
Member

I trust your judgement here. My main concern is that supported platforms don't break :)

m2, hence I've run the build bots in addition to CI to play safe: all 18 failures are unrelated (mostly test.test_curses.TestAscii.test_complexchar). So once CLA is signed I'd be inclined to approve.
Less code and re-using sysconfig.get_platform() seems a clear win to me.

Steve Dower has better opinions on how Windows builds/tests should work.

cc @zooba

Please keep using OS-unsupported tag on issues/PRs though

👍
I was involved in a lot of clang-cl issues/PRs where back then this tag wasn't applied, but will keep an eye on it from now on.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@chris-eibl chris-eibl added tests Tests in the Lib/test dir OS-windows OS-unsupported labels Jul 22, 2026
@github-project-automation github-project-automation Bot added this to Unsupported platforms Jul 22, 2026
@zooba

zooba commented Jul 22, 2026

Copy link
Copy Markdown
Member

The change in this PR is perfect, happy to see it.

No real opinion on what to do about sys.version for Clang-cl, other than make sure it doesn't break tests like this one.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@chris-eibl

chris-eibl commented Aug 29, 2026

Copy link
Copy Markdown
Member

@SynaptSea can you sign the CLA so that we can proceed here?

Otherwise can you / we close this PR?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@SynaptSea

SynaptSea commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

@SynaptSea can you sign the CLA so that we can proceed here?

Otherwise can you / we close this PR?

Sorry for the delay. Signed

chris-eibl reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

chris-eibl
chris-eibl approved these changes Aug 29, 2026
View reviewed changes
@bedevere-app bedevere-app Bot added the awaiting merge label Aug 29, 2026
@chris-eibl

chris-eibl commented Aug 29, 2026

Copy link
Copy Markdown
Member

The change in this PR is perfect, happy to see it.

Me2, CLA is signed, so merging.

@SynaptSea Thanks for your contribution ❤️

SynaptSea reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@chris-eibl
chris-eibl merged commit 5cb7c39 into python:main Aug 29, 2026
55 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@github-project-automation github-project-automation Bot moved this to Done in Unsupported platforms Aug 29, 2026
@bedevere-app bedevere-app Bot removed the awaiting merge label Aug 29, 2026
clin1234 pushed a commit to clin1234/cpython that referenced this pull request Sep 12, 2026
@SynaptSea @clin1234
pythongh-154200: Use sysconfig.get_platform() in test_importlib.test_…
aa9cbd2
…windows (python#154210)

instead of a port of distutils.util.get_platform()
@chris-eibl chris-eibl mentioned this pull request Sep 12, 2026
Fix clang-cl version in sys.version on Windows #157368
Closed
@chris-eibl chris-eibl mentioned this pull request Oct 8, 2026
gh-158948: Generate Modules/getbuildinfo.h #158951
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@chris-eibl chris-eibl chris-eibl approved these changes
@brettcannon brettcannon Awaiting requested review from brettcannon brettcannon is a code owner
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner
@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner
@FFY00 FFY00 Awaiting requested review from FFY00 FFY00 is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

OS-unsupported OS-windows skip news tests Tests in the Lib/test dir

Projects

Unsupported platforms
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

test_importlib.test_windows: vendored get_platform() misdetects clang-cl builds as win32, failing test_tagged_suffix

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@SynaptSea @bedevere-bot @chris-eibl @encukou @zooba @brettcannon
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.