Skip to content

Conversation

@Avasam
Copy link
Contributor

@Avasam Avasam commented Mar 15, 2025

This fixes an issue that makes mypy.stubtest fail when trying to compare stubs with this runtime.
This is also flagged by pyright's reportUnsupportedDunderAll

Aesthetic change: I sorted the content of __all__ to match the imports.

@jaraco jaraco merged commit 1d120d9 into pypa:main Mar 19, 2025
22 checks passed
@Avasam Avasam deleted the Explicit-re-exports-of-submodules-in-`distutils.command.__all__` branch March 19, 2025 23:12
@abravalheri
Copy link
Contributor

abravalheri commented Mar 20, 2025

@jaraco, in pypa/setuptools#4902 I hypothesized that eagerly importing the command modules in command/__init__.py affected the end result of setuptools.monkey.patch_all:

  • Previous to this PR the modules were imported late, so by the time they were executed distutils.core.Command was already patched to setuptools.Command.
  • Now, the modules are executed before the patch.

The following experiment seems to corroborate this conclusion:

> docker run --rm -it python:3.10-bookworm /bin/bash
python -m pip install 'setuptools==77.0.1'
python
>>> import setuptools
>>> from distutils.command.sdist import sdist
>>> sdist.__mro__
(<class 'distutils.command.sdist.sdist'>, <class 'setuptools.Command'>, <class 'distutils.cmd.Command'>, <class 'object'>)


python -m pip install 'setuptools @ https://github.com/pypa/setuptools/archive/refs/tags/v77.0.2.zip'   
# tag not published to PyPI.
python
>>> import setuptools
>>> from distutils.command.sdist import sdist
>>> sdist.__mro__
(<class 'distutils.command.sdist.sdist'>, <class 'distutils.cmd.Command'>, <class 'object'>)

I don't know if there are the deeper unforeseen consequences of this change (other than problems similar to pypa/setuptools#4902).

@Avasam
Copy link
Contributor Author

Avasam commented Mar 20, 2025

@abravalheri Instead of a revert, could you simply put the imports behind a if TYPE_CHECKING block for static tooling? (not 100% sure if that's enough for stubtest or not, but at least no more early import at runtime)

@jaraco
Copy link
Member

jaraco commented Mar 23, 2025

I accepted this PR without thinking about it too hard, thinking it was just some syntactic cleanup, but I see now it's meaningfully changed the behavior.

The problem is that mypy and pyright expect a very regimented, uninteresting use of __all__. Probably if written today, __all__ would better be replaced by a distutils-specific property (all_ maybe).

@abravalheri Instead of a revert, could you simply put the imports behind a if TYPE_CHECKING block for static tooling? (not 100% sure if that's enough for stubtest or not, but at least no more early import at runtime)

Let's confirm that it's enough for stubtest. I don't want to employ a fix that has no effect. My instinct, let's just revert this change for now and work out the appropriate solution in light of the troubles. I still need to review the proposed changes and the bug report, but I'll get something out soon.

jaraco added a commit that referenced this pull request Mar 23, 2025
…bmodules-in-`distutils.command.__all__`"

This reverts commit 1d120d9, reversing
changes made to 89a42e3.

Fix #351
Fix pypa/setuptools#4902
Fix pypa/setuptools#4906
Fix pypa/setuptools#4908
clrpackages pushed a commit to clearlinux-pkgs/pypi-setuptools that referenced this pull request Mar 25, 2025
…version 78.0.1

Anderson Bravalheri (16):
      Disalow deprecated dash-separated and uppercase options in setup.cfg
      Add news fragment
      Avoid duplication in setuptools.dist
      Update mentions to PEP in newsfragment
      Simplify negative conditions by applying DeMorgan's theorem
      Fix error in `setuptools/dist.py`
      Document version of setuptools introducing support for PEP 639
      Fix mispell
      Fixes for version and doc warning
      Fix external sphinx ref
      Add news fragment
      Apply suggestions from code review
      Fix reference.
      Temporarily remove 'requests' from integration tests
      Add news fragment
      Bump version: 78.0.0 → 78.0.1

Jason R. Coombs (4):
      Remove news fragments; merged downstream.
      Revert "Merge pull request pypa/distutils#345 from Avasam/Explicit-re-exports-of-submodules-in-`distutils.command.__all__`"
      Add news fragment.
      Bump version: 77.0.3 → 78.0.0
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.

3 participants