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

Default preference not given to venv DLL's #78192

Closed
jonathan-lp mannequin opened this issue Jun 30, 2018 · 9 comments
Closed

Default preference not given to venv DLL's #78192

jonathan-lp mannequin opened this issue Jun 30, 2018 · 9 comments
Assignees
Labels
3.8 only security fixes OS-windows stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@jonathan-lp
Copy link
Mannequin

jonathan-lp mannequin commented Jun 30, 2018

BPO 34011
Nosy @pfmoore, @vsajip, @tjguk, @zware, @eryksun, @zooba, @pablogsal
PRs
  • bpo-34011: Update code copying DLLs and init.tcl into venvs. #8253
  • bpo-34011: Fixes missing venv files and other tests #9458
  • bpo-34011: Fix the construction of subprocess.CalledProcessError in test_venv #10400
  • [3.6] bpo-34011: Fix the construction of subprocess.CalledProcessError in test_venv (GH-10400) #10402
  • [3.7] bpo-34011: Fix the construction of subprocess.CalledProcessError in test_venv (GH-10400) #10401
  • 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/zooba'
    closed_at = <Date 2019-07-26.16:40:38.030>
    created_at = <Date 2018-06-30.10:07:43.121>
    labels = ['3.8', 'type-bug', 'library', 'OS-windows']
    title = "Default preference not given to venv DLL's"
    updated_at = <Date 2019-07-26.16:40:38.029>
    user = 'https://bugs.python.org/jonathan-lp'

    bugs.python.org fields:

    activity = <Date 2019-07-26.16:40:38.029>
    actor = 'steve.dower'
    assignee = 'steve.dower'
    closed = True
    closed_date = <Date 2019-07-26.16:40:38.030>
    closer = 'steve.dower'
    components = ['Library (Lib)', 'Windows']
    creation = <Date 2018-06-30.10:07:43.121>
    creator = 'jonathan-lp'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 34011
    keywords = ['patch']
    message_count = 9.0
    messages = ['320765', '320932', '320934', '320935', '322024', '325899', '325930', '325933', '329428']
    nosy_count = 8.0
    nosy_names = ['paul.moore', 'vinay.sajip', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'pablogsal', 'jonathan-lp']
    pr_nums = ['8253', '9458', '10400', '10402', '10401']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue34011'
    versions = ['Python 3.8']

    @jonathan-lp
    Copy link
    Mannequin Author

    jonathan-lp mannequin commented Jun 30, 2018

    I don't know if this is a bug or an odd design decision or just something that hasn't been considered or maybe I'm missing something.

    On Windows and I create a venv with Python 3.6.3:

    python -m venv venv

    This creates a subdirectory called /venv. Inside this, there's:
    ./venv/Scripts/sqlite3.dll

    This is the sqlite library - except it's not, because Python isn't using this file. If I upgrade this library by replacing it with a newer sqlite3.dll version, Python keep using the original version of the library. Turns out, Python is by default reading the DLL in the root Python install:

    c:\Python36\DLLs\sqlite3.dll

    Now, I can change that file and sure enough my VENV (all VENVs) then get the newer version of SQLite, or I can delete that file and the VENV's will all use their local versions, or I can possibly play with some sys.path to try and get the VENV version loaded first.

    But this raises a few questions:

    1. This seems like a rather odd default - copying a file that is never used by default.
    2. Surely either the correct option is to use the VENV version by default?
    3. Otherwise, what's the point in copying across this DLL file into the VENV by default?

    @eryksun
    Copy link
    Contributor

    eryksun commented Jul 3, 2018

    Offhand I don't know why it copies PYD and DLL files from the DLLs directory. It's part of the standard library. The virtual environment should only need to copy or symlink the binaries in the application directory, such as python.exe, pythonw.exe, vcruntime<###>.dll, python<##>.dll, and python<#>.dll. Currently a virtual environment doesn't use the binaries that get copied or symlinked from the DLLs directory, since the system DLLs directory is in sys.path.

    Also, why does it copy over init.tcl? _tkinter uses Py_GetPrefix() to to get the real prefix directory in order to find the TCL library at "tcl\tclX.y". For a Python 3.7 virtual environment, I verified using Sysinternals procmon that TCL loads the init file from "C:\Program Files\Python37\tcl\tcl8.6\init.tcl", not the virtual environment.

    @eryksun eryksun added stdlib Python modules in the Lib dir OS-windows type-bug An unexpected behavior, bug, or error 3.7 (EOL) end of life 3.8 only security fixes labels Jul 3, 2018
    @vsajip
    Copy link
    Member

    vsajip commented Jul 3, 2018

    The code that does the copy is the original PEP-405 implementation code (from 26 May 2012). I'm fairly sure that these files were copied over only because (at least prior to the Python 3.3 release in September 2012) they were needed to be in the venv for the venv to work correctly. Python internals may have changed subsequently, making these copies unnecessary.

    I can't easily recreate a Python 3.3.0 on Windows to confirm this, but I'm pretty sure I only coded it to copy over what was actually needed at the time.

    @vsajip
    Copy link
    Member

    vsajip commented Jul 3, 2018

    Hmmm. I managed to find a machine to install Python3.3.0 on, and it appears that even with that copy suite removed, the tests still work. So possibly it's something that changed between 3.3.0 alpha and 3.3.0 final.

    I can try removing the copy code for 3.8 but I'm not sure if any code out there is relying on the copied files being in the venv, so it may not be a good idea to backport.

    @vsajip
    Copy link
    Member

    vsajip commented Jul 20, 2018

    New changeset 94487d4 by Vinay Sajip in branch 'master':
    bpo-34011: Update code copying DLLs and init.tcl into venvs. (GH-8253)
    94487d4

    @zooba
    Copy link
    Member

    zooba commented Sep 20, 2018

    This change breaks a number of tests when run on a proper installation, since we still need to copy pythonXY.dll or else python.exe refuses to start.

    Since I'm fixing these tests today, I'll also fix this issue.

    @zooba zooba self-assigned this Sep 20, 2018
    @zooba
    Copy link
    Member

    zooba commented Sep 20, 2018

    New changeset f14c28f by Steve Dower in branch 'master':
    bpo-34011: Fixes missing venv files and other tests (GH-9458)
    f14c28f

    @zooba
    Copy link
    Member

    zooba commented Sep 20, 2018

    I agree with this only applying to 3.8 - removing the copied files on earlier versions seems like an unnecessary risk to compatibility.

    @zooba zooba removed the 3.7 (EOL) end of life label Sep 20, 2018
    @zooba zooba closed this as completed Sep 20, 2018
    @pablogsal
    Copy link
    Member

    This buildbot is failing when raising when creating subprocess.CalledProcessError:

    https://buildbot.python.org/all/#/builders/40/builds/1135/steps/3/logs/stdio

    test_defaults (test.test_venv.BasicTest) ... ok
    test_executable (test.test_venv.BasicTest) ... ok
    test_executable_symlinks (test.test_venv.BasicTest) ... skipped 'Needs symlinks'
    test_isolation (test.test_venv.BasicTest) ... ok
    test_overwrite_existing (test.test_venv.BasicTest) ... ok
    test_prefixes (test.test_venv.BasicTest) ... ok
    test_prompt (test.test_venv.BasicTest) ... ok
    test_symlinking (test.test_venv.BasicTest) ... skipped 'Needs symlinks'
    test_unicode_in_batch_file (test.test_venv.BasicTest) ... ERROR
    test_unoverwritable_fails (test.test_venv.BasicTest) ... ok
    test_upgrade (test.test_venv.BasicTest) ... ok
    test_devnull (test.test_venv.EnsurePipTest) ... ok
    test_explicit_no_pip (test.test_venv.EnsurePipTest) ... ok
    test_no_pip_by_default (test.test_venv.EnsurePipTest) ... ok
    test_with_pip (test.test_venv.EnsurePipTest) ... ok
    ======================================================================
    ERROR: test_unicode_in_batch_file (test.test_venv.BasicTest)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_venv.py", line 302, in test_unicode_in_batch_file
        out, err = check_output(
      File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_venv.py", line 37, in check_output
        raise subprocess.CalledProcessError(
    TypeError: __init__() takes from 3 to 5 positional arguments but 6 were given

    I will prepare a PR fixing this

    @pablogsal pablogsal reopened this Nov 7, 2018
    @zooba zooba closed this as completed Jul 26, 2019
    @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
    3.8 only security fixes OS-windows 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