Skip to content

Conversation

@Avasam
Copy link
Collaborator

@Avasam Avasam commented Dec 20, 2025

Noticed in mhammond/pywin32#2697
Upstream equivalent: pypa/distutils#390

As a reminder, here's the type restrictions for spawn:

  • subprocess.check_call's first argument is typed as _CMD which is an alias for StrOrBytesPath | Sequence[StrOrBytesPath]
  • if search_path=True:
    • the first element is passed to shutil.which, so cannot be a BytesPath
    • Must be mutable (re-assigning the first element)
  • In some circumstances, cmd: StrOrBytesPath could work, but is not part of the intended api, and will result in incorrect error messages on failures (where the first element is what's included in the message if not in debug mode)

I'm aware that MutableSequence is invariant, but I believe that most usages of spawn will either be a string/tuple literal (which at least pyright will allow a partial union match on invariant generics, idk about mypy though). And that otherwise explicitly marking one's variable as such is easy enough: cmd: list[bytes | StrPath] = ["my", "cmd"]. This is already an issue in existing stub anyway and pypa/distutils#390 will try to address that upstream.

@Avasam Avasam force-pushed the setuptools-spawn-functions-should-match branch from 177b1d7 to b0b1402 Compare December 20, 2025 05:35
@Avasam Avasam changed the title setuptools: spawn functions should match each other + add overload based on search_path param setuptools._distutils: spawn functions should match each other + add overload based on search_path param Dec 20, 2025
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

LGTM, although I would consider leaving out the unused arguments as a hint to the user that the argument probably doesn't do what they think it does. Whichever you prefer.

@Avasam
Copy link
Collaborator Author

Avasam commented Dec 20, 2025

LGTM, although I would consider leaving out the unused arguments as a hint to the user that the argument probably doesn't do what they think it does. Whichever you prefer.

I've considered doing that as well upstream. If you're also mentioning it, I think it's worth omitting the unused keyword-only arguments

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

pywin32 (https://github.com/mhammond/pywin32)
+ setup.py:918: error: Unused "type: ignore" comment  [unused-ignore]

@srittau srittau merged commit efcacfb into python:main Dec 20, 2025
48 checks passed
@Avasam Avasam deleted the setuptools-spawn-functions-should-match branch December 20, 2025 21:50
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