bpo-40280: Change subprocess imports for cleaner error on wasm32 (GH-30620)

[tiran]

https://bugs.python.org/issue40280

[markshannon]

Why was this merged with a failing check?

[tiran]

Why was this merged with a failing check?

The failing test it unrelated to my PR. Victor reverted the commit that caused the test regression in GH-30632.

[markshannon]

The failing test it unrelated to my PR. Victor reverted the commit that caused the test regression in GH-30632.

I'd recommend always saying why you are merging if a check is failing, it helps narrow down the failure when looking at the merge history.

[zware]

I'll note that the last change here (GH-9053) specifically moved away from doing imports based on sys.platform == 'win32', so it's a bit disappointing to see that come back :)

[gpshead]

Yeah, based on Zach's older PR and issue that resolved this should probably be reworked to a different approach.

[tiran]

Yeah, based on Zach's older PR and issue that resolved this should probably be reworked to a different approach.

The old approach gave a confusing exception when neither msvcrt nor _posixsubprocess is available. Should I revert my change until we have a better solution?

[zware]

I won't push for a revert yet, but I also wouldn't like to see this version in a final release.

I'm afraid there's not a perfect solution here: the behavior should be based on what can be imported, which means at least one platform is going to have to raise and handle a ModuleNotFoundError. We could go a little crazy and guess at which imports should be tried first based on sys.platform (or 'msvcrt' in sys.builtin_module_names?), but I'm not sure how much effort it's worth to avoid the exception.

What was the exception when both are missing, and how was it confusing?

[tiran]

The exception was confusing because it was a nested exception that referred to msvcrt as primary cause of an import error.

How do you like #30930 ?