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

_distutils.dist.Distribution.parse_command_line() wrongly causes unexpected change to global state #4190

Open
vsajip opened this issue Jan 23, 2024 · 4 comments

Comments

@vsajip
Copy link
Contributor

vsajip commented Jan 23, 2024

This line in the eponymous method:

logging.getLogger().setLevel(logging.WARN - 10 * self.verbose)

causes a global change to the root logger. This is unexpected and unwanted when being used as a library. Changes to logging configuration must not in general be made by a library, only by application code. Parsing a command line can be done without running it, and this shouldn't change the root logger.

@abravalheri
Copy link
Contributor

Hi @vsajip, thank you very much for opening the issue.

/cc @jaraco should we transfer this issue to the pypa/distutils repository?

@jaraco
Copy link
Member

jaraco commented Jan 23, 2024

Perhaps, although setuptools.logging does the same thing:

logging.root.setLevel(level * 10)

So we should probably handle the issue here first.

More importantly, Setuptools and distutils are getting out of the business of acting as a library, except as a backend for builders.

@vsajip - what's your use-case for using Setuptools as a library? Under what conditions does the current implementation cause problems? This behavior goes back decades, so it'll likely be difficult to disentangle, especially with the dual-implementation nature of setuptools+distutils.

@vsajip
Copy link
Contributor Author

vsajip commented Jan 23, 2024

The problem surfaced when I ran the unit tests for an old project (logutils) which uses distutils (it dates from around Python 3.2) - the tests ran fine on Python 3.10, but failed on 3.11 because the legacy code in setup.py called parse_command_line(), and this caused the root logger to have a different initial state than it does under 3.10 and older. (This old project uses python setup.py test to run the tests.)

I would have thought the solution should be to move the logic from parse_command_line() to wherever output is about to be done. I'm not sure whether in this case logging is being used to do normal program output, but if so, then that's not good practice, either. Use of the root logger should only be for simple single-file scripts - in all other cases, named loggers (and not the root) should be used to identify where a logging event is coming from, and to allow application writers or sysadmins to configure logging verbosity for that part of the system.

@owocado
Copy link

owocado commented Mar 12, 2024

so this is the culprit, I was scratching my forehead since a month why in my small project the root logger was set to logging.WARN state at startup each time and after doing painstaking debugging every now and then, I just realised today it started occuring ever since I updated setuptools last month from v65.6.0 to latest. It took me a while to find this issue. You guys should revert this change. The amount of debugging I have done to find the cause is insane and this bug is not pleasant to say the least.

This change was introduced in #3674 in case someone find that useful.

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

No branches or pull requests

4 participants