Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

minstall: help mypy out with our chown overriding #14220

Merged
merged 1 commit into from
Feb 4, 2025

Conversation

dcbaker
Copy link
Member

@dcbaker dcbaker commented Feb 3, 2025

This is an annoying issue to look at, because shutil.chown has (for our purposes) three signatures:

chown(path: int | AnyPathLike, uid: int | str, group: None = None) -> None: ...
chown(path: int | AnyPathLike, uid: None, group: int | str) -> None: ...
chown(path: int | AnyPathLike, uid: int | str, group: int | str) -> None: ...

This is a really difficult thing to guarantee from our code. We more or less depend on being able to pass two parameters of None | int | str, and it working. In our only caller we do ensure that at least one of the variables is not None, but convincing mypy of this is more work than it's worth.

This issue only shows up in CI with python >= 3.13. It also shows up with pyright (used in VSCode) for earlier versions because of an updated typeshed.

@dcbaker dcbaker requested a review from jpakkane as a code owner February 3, 2025 19:47
@dcbaker dcbaker force-pushed the submit/fix-mypy-chown branch 3 times, most recently from 418dcc5 to 8321754 Compare February 3, 2025 20:02
This is an annoying issue to look at, because shutil.chown has (for our
purposes) three signatures:
```python
chown(path: int | AnyPathLike, uid: int | str, group: None = None) -> None: ...
chown(path: int | AnyPathLike, uid: None, group: int | str) -> None: ...
chown(path: int | AnyPathLike, uid: int | str, group: int | str) -> None: ...
```

This is a really difficult thing to guarantee from our code. We more or
less depend on being able to pass two parameters of `None | int | str`,
and it working. In our only caller we do ensure that at least one of the
variables is not None, but convincing mypy of this is more work than
it's worth.

This will show up in our CI only for python >= 3.13. An updated typshed
will make this show up for earlier versions, however. Pyright (which is
used by the VSCode Python extension) will spot this for earlier
versions. I have changed the code in such a way to make our CI turn
green.
@dcbaker dcbaker force-pushed the submit/fix-mypy-chown branch from 8321754 to 43f64c0 Compare February 3, 2025 21:01
@dcbaker dcbaker merged commit 0e6e687 into mesonbuild:master Feb 4, 2025
31 checks passed
@dcbaker dcbaker deleted the submit/fix-mypy-chown branch February 4, 2025 05: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