Navigation Menu

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

venv and ensurepip are affected by default pip config file #64252

Closed
ncoghlan opened this issue Dec 23, 2013 · 28 comments
Closed

venv and ensurepip are affected by default pip config file #64252

ncoghlan opened this issue Dec 23, 2013 · 28 comments
Assignees
Labels
deferred-blocker stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ncoghlan
Copy link
Contributor

BPO 20053
Nosy @pfmoore, @ncoghlan, @larryhastings, @bitdancer, @dstufft
Dependencies
  • bpo-20541: os.path.exists() gives wrong answer for Windows special files
  • bpo-20570: Bundle pip 1.5.3 in Python 3.4rc2
  • 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:

    assignee = 'https://github.com/ncoghlan'
    closed_at = <Date 2014-03-08.12:40:42.595>
    created_at = <Date 2013-12-23.06:41:08.275>
    labels = ['deferred-blocker', 'type-bug', 'library']
    title = 'venv and ensurepip are affected by default pip config file'
    updated_at = <Date 2014-03-08.12:40:42.593>
    user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2014-03-08.12:40:42.593>
    actor = 'ncoghlan'
    assignee = 'ncoghlan'
    closed = True
    closed_date = <Date 2014-03-08.12:40:42.595>
    closer = 'ncoghlan'
    components = ['Library (Lib)']
    creation = <Date 2013-12-23.06:41:08.275>
    creator = 'ncoghlan'
    dependencies = ['20541', '20570']
    files = []
    hgrepos = []
    issue_num = 20053
    keywords = ['buildbot']
    message_count = 28.0
    messages = ['206843', '206846', '206864', '206867', '210223', '210311', '210313', '210319', '210325', '210376', '210458', '210462', '210463', '210470', '210473', '210474', '210476', '210504', '210515', '210516', '210519', '210520', '210551', '210553', '210576', '210718', '211789', '212927']
    nosy_count = 6.0
    nosy_names = ['paul.moore', 'ncoghlan', 'larry', 'r.david.murray', 'python-dev', 'dstufft']
    pr_nums = []
    priority = 'deferred blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue20053'
    versions = ['Python 3.4']

    @ncoghlan
    Copy link
    Contributor Author

    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):

    • create a temporary directory

    • set os.environ["HOME"] to point to that directory

    • create a platform appropriate file (pip\pip.ini on Windows, .pip/pip.conf elsewhere) containing the settings:

      [global]
      no-install=1

    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.

    @ncoghlan ncoghlan self-assigned this Dec 23, 2013
    @ncoghlan
    Copy link
    Contributor Author

    pip side counterpart, suggesting an explicit "isolated mode" option: pypa/pip#1397

    @bitdancer
    Copy link
    Member

    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)

    @ncoghlan
    Copy link
    Contributor Author

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 4, 2014

    New changeset 1b8ba1346e67 by Nick Coghlan in branch 'default':
    Close bpo-20053: ignore default pip config settings
    http://hg.python.org/cpython/rev/1b8ba1346e67

    @python-dev python-dev mannequin closed this as completed Feb 4, 2014
    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Feb 5, 2014

    *sigh*, Windows is not cooperating, suggesting it is still reading the default config file:

    http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/4000/steps/test/logs/stdio

    ======================================================================
    FAIL: test_with_pip (test.test_venv.EnsurePipTest)
    ----------------------------------------------------------------------

    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

    @ncoghlan ncoghlan added the stdlib Python modules in the Lib dir label Feb 5, 2014
    @ncoghlan ncoghlan reopened this Feb 5, 2014
    @ncoghlan ncoghlan added the type-bug An unexpected behavior, bug, or error label Feb 5, 2014
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 5, 2014

    New changeset ddc82c4d1a44 by Nick Coghlan in branch 'default':
    Issue bpo-20053: new test to check an assumption
    http://hg.python.org/cpython/rev/ddc82c4d1a44

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Feb 5, 2014

    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?

    @pfmoore
    Copy link
    Member

    pfmoore commented Feb 5, 2014

    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).

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Feb 6, 2014

    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 :(

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 7, 2014

    New changeset 17bea44a9fa7 by Nick Coghlan in branch 'default':
    Issue bpo-20053: Actually test relevant assumption
    http://hg.python.org/cpython/rev/17bea44a9fa7

    @pfmoore
    Copy link
    Member

    pfmoore commented Feb 7, 2014

    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.

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Feb 7, 2014

    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).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 7, 2014

    New changeset f3f92d55f942 by Nick Coghlan in branch 'default':
    Issue bpo-20053: Mark as an expected failure for 3.4
    http://hg.python.org/cpython/rev/f3f92d55f942

    @pfmoore
    Copy link
    Member

    pfmoore commented Feb 7, 2014

    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.

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Feb 7, 2014

    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.

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Feb 7, 2014

    (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)

    @dstufft
    Copy link
    Member

    dstufft commented Feb 7, 2014

    I'm not sure I grasp what the problem is

    @pfmoore
    Copy link
    Member

    pfmoore commented Feb 7, 2014

    In ensurepip, _disable_pip_configuration_settings has the line:

    os.environ['PIP_CONFIG_FILE'] = os.devnull
    

    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).

    @dstufft
    Copy link
    Member

    dstufft commented Feb 7, 2014

    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.

    @pfmoore
    Copy link
    Member

    pfmoore commented Feb 7, 2014

    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?

    @dstufft
    Copy link
    Member

    dstufft commented Feb 7, 2014

    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.

    @pfmoore
    Copy link
    Member

    pfmoore commented Feb 7, 2014

    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.

    @pfmoore
    Copy link
    Member

    pfmoore commented Feb 7, 2014

    pypa/pip#1543

    @pfmoore
    Copy link
    Member

    pfmoore commented Feb 7, 2014

    Fix now merged into pip 1.5.X branch

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Feb 9, 2014

    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.

    @dstufft
    Copy link
    Member

    dstufft commented Feb 21, 2014

    pip 1.5.3 is released and I've requested larry cherry-pick it into 3.4.0 with bpo-20713

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Mar 8, 2014

    pip 1.5.3 has been bundled and cherry picked to the release clone.

    @ncoghlan ncoghlan closed this as completed Mar 8, 2014
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    deferred-blocker stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants