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

Backfill distutils.log.Log for compatibility #192

Merged
merged 2 commits into from
Nov 22, 2022

Conversation

abravalheri
Copy link
Contributor

@abravalheri abravalheri commented Nov 22, 2022

in pypa/setuptools#3693 we learned that some projects rely on distutils.log.Log and users are asking for distutils.log.Log to be added back to ensure compatibility is not broken.

I am not sure what is the best way forward, but I am providing this PR if pypa/distutils' maintainers are interested in the "backfill" approach.

  • I found just a few usages in https://grep.app/search?q=from%20distutils.log%20import%20Log,
    • However it seams that the main problem is users importing numpy.disutils without pinning setuptools.
  • I am not sure about adding tests. I noticed that all the tests about deprecated distutils.log behaviour were removed.

@abravalheri
Copy link
Contributor Author

The error in ci_setuptools is probably solved by pypa/setuptools#3701

@rgommers
Copy link

Thanks @abravalheri, this PR is much appreciated.

However it seams that the main problem is users importing numpy.disutils without pinning setuptools.

This is also for usage at runtime, and we've got issues in e.g. Debian which simply ships only one setuptools version, so there is nothing to pin. Released numpy versions are out there using Log, can't yank those. So this seems like a good way to keep things from falling apart. Can be done without tests imho, it's just a band-aid.

@abravalheri
Copy link
Contributor Author

Thank you very much @rgommers.

The way I manually tested this was:

git clone https://github.com/abravalheri/distutils --branch setuptools-issue-3693
git clone https://github.com/pypa/setuptools
rm -rf setuptools/setuptools/_distutils
cp -r distutils/distutils setuptools/setuptools/_distutils
virtualenv .venv
.venv/bin/python -m pip install -U pip ./setuptools
.venv/bin/python -m pip install -m numpy
.venv/bin/python -c '
import distutils

assert "setuptools/_distutils" in next(iter(distutils.__path__))

from numpy.distutils import log

log.warn("hello world")

import logging
assert isinstance(log.Log(), logging.Logger), f"{log.Log.__mro__=}"
'
After that I can see a coloured `hello world` message, so I am assuming the patch works well.

image

@jaraco
Copy link
Member

jaraco commented Nov 22, 2022

Brilliant. Thank you.

@jaraco jaraco closed this Nov 22, 2022
@jaraco jaraco reopened this Nov 22, 2022
@jaraco jaraco merged commit 3e9d47e into pypa:main Nov 22, 2022
@abravalheri abravalheri deleted the setuptools-issue-3693 branch November 23, 2022 00:02
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