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
venv and ensurepip are affected by default pip config file #64252
Comments
In resolving bpo-19734, I realised venv and ensurepip are actually in a similar situation with respect to the default pip configuration file as they were with respect to environment variables: those settings are unlikely to be appropriate for ensurepip, but pip will still pay attention to them during the bootstrapping process. However, it's a bit trickier to test, since PIP_CONFIG_FILE will be ignored (due to the resolution of bpo-19734). The approach I will run with (if nobody has any better suggestions):
That should cause the test_venv tests to fail, just as setting PIP_NO_INSTALL in the environment caused them to fail as a test for the bpo-19734 resolution. In terms of forcing pip to *ignore* the global config file, the best option I have found is setting PIP_CONFIG_FILE=/dev/null (I believe the Windows equivalent would be PIP_CONFIG_FILE=NUL). The fact the file exists means pip uses it without falling back to the default file location, while the fact it is always read as empty means it has no effect on pip's operation. I'm open to better suggestions on how to do that, but it seems like the best available option without an "isolated mode" equivalent in pip. |
pip side counterpart, suggesting an explicit "isolated mode" option: pypa/pip#1397 |
I don't know anything about pip configuration or the tests, but perhaps you want to use something like: 'PIP_CONFIG_FILE={}'.format(os.devnull) |
Oh, I forgot about os.devnull. ensurepip.bootstrap mutates the environment of the current process (hence the recommendation to use the CLI instead), so yes, doing "os.environ['PIP_CONFIG_FILE'] = os.devnull" before importing pip should do the trick. And then generate a "no-install=true" config with HOME in test_venv to make sure it is properly ignored. |
New changeset 1b8ba1346e67 by Nick Coghlan in branch 'default': |
*sigh*, Windows is not cooperating, suggesting it is still reading the default config file: ====================================================================== Traceback (most recent call last):
File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_venv.py", line 342, in test_with_pip
self.assertEqual(err, "")
AssertionError: 'C:\\Users\\Buildbot\\AppData\\Local\\Temp[57 chars]\r\n' != ''
- C:\Users\Buildbot\AppData\Local\Temp\tmpfiz5hrop\Scripts\python_d.exe: No module named pip |
New changeset ddc82c4d1a44 by Nick Coghlan in branch 'default': |
D'oh, still failing even though that new assumption check passes on Windows: http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/4002/steps/test/logs/stdio Paul, any ideas? |
Nothing obvious or Windows-specific from what I can see. It does seem to me that EnvironmentVarGuard doesn't monkeypatch os.environ even though it looks like it intends to (exit sets it back, but __enter__ doesn't monkeypatch it). So maybe that's doing something weird? I'll try to have a better look later (not much spare time right now). |
EnvironmentVarGuard doesn't work through monkeypatching - you make your changes *through* the guard, and it undoes them in __exit__, as well as restoring the original binding (in case *other* code monkeypatched it). This allows it to still be used when testing code that is *supposed* to make real environment changes (e.g. so they're visible in a subprocess). Buildbot is currently down, so I can't check the logs again :( |
New changeset 17bea44a9fa7 by Nick Coghlan in branch 'default': |
Sigh. Yes, I looked at that and wondered if os.devnul would show as existing, but I similarly misread the test and assumed it was testing what it was meant to test :-( os.path.exists(os.devnull) is false on Windows (just checked) so this assumption *is* what's failing. |
As Paul noted, I discovered there was a bug in my assumption testing test - when I tried it directly on Windows, I discovered the culprit here is bpo-20541. So adding that as a dependency, and I'll disable that part of the test on Windows (and mark the assumption as an expected failure). |
New changeset f3f92d55f942 by Nick Coghlan in branch 'default': |
As noted in http://bugs.python.org/issue1311 (referenced from http://bugs.python.org/issue20541) it's not actually correct to assume that os.path.exists(os.devnull) returns true, as on Windows the "null device" is not a proper filesystem object. So I'd suggest that the code here needs to avoid relying on this assumption. |
The problem is that the assumption of assumption of an exists <-> open equivalence is in pip's configuration file loading rather than in ensurepip or venv - the idea was to pass in an existing but empty file to avoid needing a change in pip to fix this. However, it looks like we're just going to have to live with the misbehaviour on Windows until it can be fixed on the pip side for Python 3.4.1. |
(Note that the fix on the pip side may be something more direct like just ignoring all config files and environment variables when ENSUREPIP_OPTIONS is set, rather than changing the way it reads the config files) |
I'm not sure I grasp what the problem is |
In ensurepip, _disable_pip_configuration_settings has the line:
On Windows, os.devnull does not behave like a real file in that os.path.exists(os.devnull) is False even though opening it works fine. So that line of code is not portable to Windows. The problem in pip us in baseparser.py in ConfigOptionParser: def get_config_files(self):
config_file = os.environ.get('PIP_CONFIG_FILE', False)
if config_file and os.path.exists(config_file):
files = [config_file]
else:
files = [default_config_file] There's no way to force pip to *not* use a config file. A quick hack would be to respect a new environment variable (PIP_NO_CONFIG_FILE) which if set says to set files to [] here. A bigger fix is to implement a proper "isolated mode" in pip. I could probably knock up the "quick hack" as a PR for pip reasonably easily. This should really be discussed on the pip tracker, though. Nick - did you raise an issue on pip for this? Assuming you did, can you post the reference here? Either way, ensurepip needs to be changed to use whatever mechanism pip implements. I am against special casing os.devnull in pip just to make this work (although it would address the issue). |
The proper fix is an isolated mode, but we could special case devnull in pip for 1.5.3 and make a proper isolated solution in 1.6. |
Maybe. I don't know what *else* might fail because devnull is special. The filename gets passed straight to configparser, for a start. But if you want to do that I'm OK with that - I just won't make that change myself. Would the special-casing be permanent or would it be removed when isolated mode is implemented? |
I'd remove it in 1.6 with a proper isolated mode. I'm purely thinking of minimal changes to make it easier to to get it into 3.4. |
Sounds reasonable. Having thought about it a bit more, I don't think it's a big deal. I'll put a PR together for special-casing devnull. |
Fix now merged into pip 1.5.X branch |
I created bpo-20570 to propose updating to pip 1.5.3 for 3.4rc2. That would include the pip side workaround for this issue, also allowing this issue to be closed. |
pip 1.5.3 is released and I've requested larry cherry-pick it into 3.4.0 with bpo-20713 |
pip 1.5.3 has been bundled and cherry picked to the release clone. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: