Correctly assign configuration to origin files with `pip config`
Fixes #12099
This pull requests extends the pip configuration dictionary to be nested, allowing the configuration keys to be connected with the origin files. It will look like this:
{'/etc/xdg/pip/pip.conf': {'install.require-virtualenv': 'true'}, '/etc/pip.conf': {'global.timeout': '60'}, '/home/user/.pip/pip.conf': {'install.require-virtualenv': 'true'}, '/home/user/.config/pip/pip.conf': {'global.timeout': '60', 'test.blah': '1', 'test.blaha': '1'}}
pip config debug correctly separates the configuration:
$ python -m pip config debug
env_var:
env:
global:
/etc/xdg/xdg-ubuntu/pip/pip.conf, exists: False
/etc/xdg/pip/pip.conf, exists: True
install.require-virtualenv: true
/etc/pip.conf, exists: True
global.timeout: 60
site:
/home/user/.virtualenvs/pip-dev/pip.conf, exists: False
user:
/home/user/.pip/pip.conf, exists: True
install.require-virtualenv: true
/home/user/.config/pip/pip.conf, exists: True
global.timeout: 60
test.blah: 1
test.blaha: 1
I'm not satisfied with the implementation, though... Happy about review feedback!
Thank you, updated!
Thanks for filing this PR! ^>^
Could you update the output to indent the configuration values by one level more compared to the output line about the origin file?
i.e.
$ python -m pip config debug
env_var:
env:
global:
/etc/xdg/xdg-ubuntu/pip/pip.conf, exists: False
/etc/xdg/pip/pip.conf, exists: True
install.require-virtualenv: true
/etc/pip.conf, exists: True
global.timeout: 60
site:
/home/user/.virtualenvs/pip-dev/pip.conf, exists: False
user:
/home/user/.pip/pip.conf, exists: True
install.require-virtualenv: true
/home/user/.config/pip/pip.conf, exists: True
global.timeout: 60
test.blah: 1
test.blaha: 1
Thanks for filing this PR! ^>^
You're welcome!
Could you update the output to indent the configuration values by one level more compared to the output line about the origin file?
I'm sorry, that's actually the default behaviour - my sed call messed that up :/
Updated the example. I sure agree it looks better!
Thank you for the code refinery suggestions! I've applied them.
Looks good. Can we add some tests for this?
@uranusjr I have added a test. Do you have a suggestion how to touch the configuration file platform-independent?
Looks like there’s some problem with test setup, the config is not where you expect it to be.
Don't know why, but on Windows the the new_config_file is in a subdirectory of the one used for the preceding test_user_values test:
new_config_file = 'C:\\Temp\\pytest-of-runneradmin\\pytest-1\\popen-gw1\\test_user_values0\\home\\AppData\\Roaming\\pip\\pip.ini'
Can I run a simple replace on that?
Can I run a simple replace on that?
IMO, only if you add a comment to the test explaining why you needed to do so for future maintainers. And if you can write such a comment, you'd probably be better just fixing the test to look in the correct place 🙂
I think that's actually state leaking between tests, which is what is causing the failure here. Doing a replace seems like a bodge and, ideally, we would instead change test_user_values to not leak state into other tests...
Maybe, that should delete new_config_file before exiting. @chrysle Could you try adding an os.remove(new_config_file) to the end of test_user_values?
@chrysle Please run git pull locally on this branch, before making any more changes. :)
Sure, done; I wanted to rebase after checking if your solution would work.
Do you have another suggestion how to handle the issue? I can't test on Windows, unfortunately.
Could you try adding an
os.remove(new_config_file)to the end oftest_user_values?
Have you tried this yet?
@chrysle are you interested in fixing the failing test on Windows CI and fixing the merge conflicts? This PR looks pretty close to be ready to be merged. It's okay if you aren't!
@chrysle are you interested in fixing the failing test on Windows CI
I am, unfortunately I don't know how to do that. I don't use Windows and thus can not investigate this properly. Any help would be greatly appreciated.