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

CLI parses unrelated config files and then crashes #5182

Closed
wimglenn opened this issue Apr 6, 2018 · 19 comments
Closed

CLI parses unrelated config files and then crashes #5182

wimglenn opened this issue Apr 6, 2018 · 19 comments
Labels
C: configuration Configuration management and loading C: error messages Improving error messages kind: crash For situations where pip crashes

Comments

@wimglenn
Copy link
Contributor

wimglenn commented Apr 6, 2018

  • Pip version: 9.0.3
  • Python version: 3.6.4
  • Operating system: Linux

Local project keeps configuration in a [tool:pytest] section of setup.cfg. Can't use pip from within that directory, because it attempts to parse the file incorrectly (using a ConfigParser with interpolation) and then crashes out.

minimal steps to reproduce:

  1. create a clean virtualenv, empty directory
  2. create this setup.cfg file in cwd:
[tool:pytest]
log_format = %(name)-18s %(levelname)-8s %(message)s
  1. run unrelated pip command, e.g. pip install --upgrade pip.
Exception:
Traceback (most recent call last):
  File "/tmp/blah/venv/lib64/python3.6/site-packages/pip/basecommand.py", line 215, in main
    status = self.run(options, args)
  File "/tmp/blah/venv/lib64/python3.6/site-packages/pip/commands/install.py", line 350, in run
    isolated=options.isolated_mode,
  File "/tmp/blah/venv/lib64/python3.6/site-packages/pip/commands/install.py", line 436, in get_lib_location_guesses
    scheme = distutils_scheme('', *args, **kwargs)
  File "/tmp/blah/venv/lib64/python3.6/site-packages/pip/locations.py", line 141, in distutils_scheme
    d.parse_config_files()
  File "/usr/lib64/python3.6/distutils/dist.py", line 402, in parse_config_files
    val = parser.get(section,opt)
  File "/usr/lib64/python3.6/configparser.py", line 800, in get
    d)
  File "/usr/lib64/python3.6/configparser.py", line 394, in before_get
    self._interpolate_some(parser, option, L, value, section, defaults, 1)
  File "/usr/lib64/python3.6/configparser.py", line 427, in _interpolate_some
    "bad interpolation variable reference %r" % rest)
configparser.InterpolationSyntaxError: bad interpolation variable reference '%(name)-18s %(levelname)-8s %(message)s'

That's a valid pytest config section but ConfigParser is getting confused thinking parts of the logging config are interpolation references. I don't know how best to fix this issue because it's actually in distutils.dist.Distribution where they create a ConfigParser and there is not public API to enable/disable interpolation. However, perhaps pip's locations.py shouldn't be attempting to eagerly parse this file in the first place?

Existing workaround: Changing out of the directory that has setup.cfg before using pip.

@benoit-pierre
Copy link
Member

Setuptools behavior is the same (see discussion). But pytest uses py.iniconfig hence the incompatible behavior...

@benoit-pierre
Copy link
Member

And py.iniconfig is a completly custom implementation...

@pradyunsg pradyunsg added C: configuration Configuration management and loading kind: crash For situations where pip crashes labels Apr 7, 2018
@pradyunsg pradyunsg added this to the Print Better Error Messages milestone Apr 7, 2018
@pradyunsg
Copy link
Member

I'm pretty sure we can try printing a better error message for this case.

This is precisely why I think #3809 (or something similar) is a good idea -- ini is not a standardized format.

@wimglenn
Copy link
Contributor Author

wimglenn commented Apr 7, 2018

Could you clear up a curiousity: why does pip want to read ./setup.cfg in the first place - i.e., what contents in setup.cfg could/should alter the outcome of this command:

pip install --upgrade pip  

Maybe I'm missing something obvious here, but it seems like a setup.cfg file present in current directory should only influence a command such as:

pip install .

@RonnyPfannschmidt
Copy link
Contributor

this shoudl be better documented in pytest - if the canonical configfile isnt used, its up to the user not to break the configfiles of other tools

@wimglenn
Copy link
Contributor Author

wimglenn commented Apr 7, 2018

Which is canonical? pytest.ini presumably?

@RonnyPfannschmidt
Copy link
Contributor

@wimglenn correct, BTW im one of the pytest maintainers

@wimglenn
Copy link
Contributor Author

wimglenn commented Apr 7, 2018

I like having pytest config as a section in setup.cfg because it avoids yet another random config file lying around in the project root (I already need it for setuptools). I see your point but it is also up to other tools not to break for silly reasons .. :) distutils should not try to interpolate, imo, but since it's stdlib it's probably impossible to change now - pip probably shouldn't need to eagerly parse everybody else's sections!

@RonnyPfannschmidt
Copy link
Contributor

@wimglenn until there is a common configformat with a common api, is most sensible to keep things in different files since different things can easily breka each other

hopefully pyproject.toml will sort that out

@gaborbernat
Copy link

I've run into this myself. This makes essentially not possible to put Unicode characters inside setup.cfg, for example, the name of the authors if you use configuration based setuptools... 😢 setuptools changed that setup.cfg is always UTF-8... distutils/pip does not make this assumption yet.

@jnoortheen
Copy link

I stumbled into the same even in versions 20.*

@uranusjr
Copy link
Member

@gaborbernat How did Setuptools communicate the encoding change when it happened? IMO it is reasonable to change to parser encoding to UTF-8, but I don’t want to break whatever is relying on platform encoding.

@gaborbernat
Copy link

As far as I remember later setuptools switched to utf 8 encoding so this should not be a problem now with latest pip and setuptools 🤔

@uranusjr
Copy link
Member

uranusjr commented Mar 24, 2020

Hmm, taking another look, this config file parsing logic is buried deep inside distutils. Does Setuptools use its own implementation? Not sure what’s the best course for pip to deal with this…

https://github.com/python/cpython/blob/6000087fe979705dc588a46a35da884cc0198c67/Lib/distutils/dist.py#L402-L419

@gaborbernat
Copy link

setuptools has it's own copy of dist; https://github.com/pypa/setuptools/blob/master/setuptools/dist.py#L553; was this release https://setuptools.readthedocs.io/en/latest/history.html#v41-0-0 that made it UTF-8

@uranusjr
Copy link
Member

Thanks for the clarification! pip is really short of options here (it can’t even just ignore the offending file, since the loop itself is buried in distutils). I guess it’s not very realistic to have another copy of dist in pip, so we can either push a new encoding argument to stdlib distutils (is this even possible?), or monkey-patch the open function in the module. Neither sounds particularly good 😞

landonb pushed a commit to landonb/nark that referenced this issue Apr 8, 2020
- Running flake8 via tox, e.g., `tox -e flake8` crashes.
  Setuptools uses configparser to read the config, which
  throws configparser.InterpolationMissingOptionError because
  of the percent (%) symbols in the format string.
  - Ref:
    pytest-dev/pytest#3062
    pypa/pip#5182
landonb pushed a commit to landonb/dob that referenced this issue Apr 8, 2020
- Running flake8 via tox, e.g., `tox -e flake8` crashes.
  Setuptools uses configparser to read the config, which
  throws configparser.InterpolationMissingOptionError because
  of the percent (%) symbols in the format string.
  - Ref:
    pytest-dev/pytest#3062
    pypa/pip#5182
landonb pushed a commit to landonb/dob-viewer that referenced this issue Apr 8, 2020
- Running flake8 via tox, e.g., `tox -e flake8` crashes.
  Setuptools uses configparser to read the config, which
  throws configparser.InterpolationMissingOptionError because
  of the percent (%) symbols in the format string.
  - Ref:
    pytest-dev/pytest#3062
    pypa/pip#5182
landonb pushed a commit to landonb/dob-prompt that referenced this issue Apr 8, 2020
- Running flake8 via tox, e.g., `tox -e flake8` crashes.
  Setuptools uses configparser to read the config, which
  throws configparser.InterpolationMissingOptionError because
  of the percent (%) symbols in the format string.
  - Ref:
    pytest-dev/pytest#3062
    pypa/pip#5182
landonb added a commit to landonb/dob-bright that referenced this issue Apr 8, 2020
- Running flake8 via tox, e.g., `tox -e flake8` crashes.
  Setuptools uses configparser to read the config, which
  throws configparser.InterpolationMissingOptionError because
  of the percent (%) symbols in the format string.
  - Ref:
    pytest-dev/pytest#3062
    pypa/pip#5182
landonb pushed a commit to landonb/config-decorator that referenced this issue Apr 10, 2020
- Running flake8 via tox, e.g., `tox -e flake8` crashes.
  Setuptools uses configparser to read the config, which
  throws configparser.InterpolationMissingOptionError because
  of the percent (%) symbols in the format string.
  - Ref:
    pytest-dev/pytest#3062
    pypa/pip#5182
landonb pushed a commit to landonb/human-friendly_pedantic-timedelta that referenced this issue Apr 16, 2020
- Running flake8 via tox, e.g., `tox -e flake8` crashes.
  Setuptools uses configparser to read the config, which
  throws configparser.InterpolationMissingOptionError because
  of the percent (%) symbols in the format string.
  - Ref:
    pytest-dev/pytest#3062
    pypa/pip#5182
landonb added a commit to landonb/pep440-version-compare-cli that referenced this issue Apr 26, 2020
- Running flake8 via tox, e.g., `tox -e flake8` crashes.
  Setuptools uses configparser to read the config, which
  throws configparser.InterpolationMissingOptionError because
  of the percent (%) symbols in the format string.
  - Ref:
    pytest-dev/pytest#3062
    pypa/pip#5182
landonb added a commit to landonb/easy-as-pypi that referenced this issue Apr 26, 2020
- Running flake8 via tox, e.g., `tox -e flake8` crashes.
  Setuptools uses configparser to read the config, which
  throws configparser.InterpolationMissingOptionError because
  of the percent (%) symbols in the format string.
  - Ref:
    pytest-dev/pytest#3062
    pypa/pip#5182
@nlhkabu nlhkabu added C: error messages Improving error messages UX User experience related labels Jul 28, 2020
@nlhkabu nlhkabu removed this from the Print Better Error Messages milestone Jul 29, 2020
@aucampia
Copy link

rather annoying :/

@pradyunsg pradyunsg removed type: bug A confirmed bug or unintended behavior project: setuptools Related to setuptools UX User experience related labels Jan 15, 2022
aucampia added a commit to aucampia/rdflib that referenced this issue Apr 18, 2022
pytest was using config from `tox.ini` preferentially and ignoring config
from `setup.cfg`, as a side-effect doctests were not running on
code/docstrings in `rdflib/`.

The reason why some pytest config was in `tox.ini` instead of `setup.cfg`
was because of these issues:
- pypa/pip#5182
- pytest-dev/pytest#3062

As a compromise to fix this I have opted for moving all pytest config to
`pyproject.toml`:
- https://docs.pytest.org/en/stable/reference/customize.html#pyproject-toml

This seems sensible as `pyproject.toml` is standarized by PEPs and
eventually most things from `setup.cfg` will end up in there anyway.

Also:
- remove the pytest ignore on `test/translate_algebra` as tests in there
  have been running and passing for some time.
- fixed path to test data in `rdflib/plugins/parsers/nquads.py`.
@pradyunsg
Copy link
Member

This is no longer an issue in 3.10+. Closing this out to reflect that.

@wimglenn
Copy link
Contributor Author

Related: #11298

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: configuration Configuration management and loading C: error messages Improving error messages kind: crash For situations where pip crashes
Projects
None yet
Development

No branches or pull requests

9 participants