Repository: Yelp/venv-update

Issue:
incompatibility with virtualenv 20
`virtualenv` version 20 waas released three days ago. Since then, using venv-update unmodified results in this error:

```
OSError: [Errno 39] Directory not empty: 
   '$HOME/.cache/venv-update/3.1.1/src.tmp' -> 
   '$HOME/.cache/venv-update/3.1.1/src'
```

This is due to a check for existance of virtualenv.py always failing, which is in turn due to virtualenv project refactoring from a module to a package with virtualenv/__main__.py. I tried to simply patch venv-update to handle the new structure, but the changes are too extensive.

As a short-term fix, this works for me (tm):

```!diff
--- sbin/venv-update.orig       2020-02-13 11:25:28.210285813 -0800
+++ sbin/venv-update    2020-02-13 11:33:31.696390868 -0800
@@ -200,7 +200,7 @@
         scratch_python = venv_python(scratch.venv)
         # TODO: do we allow user-defined override of which version of virtualenv to install?
         tmp = scratch.src + '.tmp'
-        run((scratch_python, '-m', 'pip.__main__', 'install', 'virtualenv', '--target', tmp))
+        run((scratch_python, '-m', 'pip.__main__', 'install', 'virtualenv<20', '--target', tmp))
 
         from os import rename
         rename(tmp, scratch.src)
```

Discussion:

Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 05940cc..ded0df3 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -2,16 +2,8 @@
 name: venv-update-ci
 on: push
 jobs:
-  tox:
+  lint:
     runs-on: ubuntu-18.04
-    strategy:
-      fail-fast: false
-      matrix:
-        toxenv:
-          - lint
-          - py36
-          - py27
-          - pypy
     steps:
       - uses: actions/checkout@v2
       - uses: actions/setup-python@v2
@@ -19,5 +11,34 @@ jobs:
           python-version: 3.6
       - run: pip install tox==3.21.2
       - run: pip install -r CI/requirements.txt
-      - run: sudo apt-get install -y pypy pypy-dev
-      - run: tox -e ${{ matrix.toxenv }}
+      - run: tox -e lint
+  py36:
+    runs-on: ubuntu-18.04
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v2
+        with:
+          python-version: 3.6
+      - run: pip install tox==3.21.2
+      - run: pip install -r CI/requirements.txt
+      - run: tox -e py36
+  py27:
+    runs-on: ubuntu-18.04
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v2
+        with:
+          python-version: 3.6
+      - run: pip install tox==3.21.2
+      - run: pip install -r CI/requirements.txt
+      - run: tox -e py27
+  pypy3:
+    runs-on: ubuntu-18.04
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v2
+        with:
+          python-version: pypy-3.6
+      - run: pip install tox==3.21.2
+      - run: pip install -r CI/requirements.txt
+      - run: tox -e pypy3
diff --git a/CI/requirements.txt b/CI/requirements.txt
index 48ad566..5d4c368 100644
--- a/CI/requirements.txt
+++ b/CI/requirements.txt
@@ -1,4 +1,4 @@
 # minimal set of packages for github actions to run our test suites
 codecov
 tox==3.21.2
-virtualenv<20
+virtualenv>=20.0.8
diff --git a/docs/source/index.rst b/docs/source/index.rst
index 880b391..8160e0e 100644
--- a/docs/source/index.rst
+++ b/docs/source/index.rst
@@ -115,7 +115,7 @@ Python library.
 
 .. parsed-literal::
 
- curl -o venv-update `<https://raw.githubusercontent.com/Yelp/venv-update/v3.2.4/venv_update.py>`_
+ curl -o venv-update `<https://raw.githubusercontent.com/Yelp/venv-update/v4.0.0/venv_update.py>`_
  chmod +x venv-update
 
 
diff --git a/setup.py b/setup.py
index f9c490d..a69d75e 100644
--- a/setup.py
+++ b/setup.py
@@ -56,10 +56,7 @@ def main():
         packages=find_packages(exclude=('tests*',)),
         install_requires=[
             'pip>=10.0.0,<=18.1',
-            # >0.25.0 because 0.25.0 causes get_tag AssertionError in python3
-            # <0.34.0 because 0.34.0 changes pypy tag generation in a way that
-            # our old pip version can't handle
-            'wheel>0.25.0,<0.34.0',
+            'wheel>0.25.0',  # 0.25.0 causes get_tag AssertionError in python3
             'setuptools>=0.8.0',  # 0.7 causes "'sys_platform' not defined" when installing wheel >0.25
         ],
         entry_points={
diff --git a/tox.ini b/tox.ini
index d07d8bf..f05a36d 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,6 +1,6 @@
 [tox]
 # These should match the travis env list
-envlist = py27,py36,pypy,lint
+envlist = py27,py36,pypy3,lint
 skipsdist=True
 
 [testenv]
diff --git a/venv_update.py b/venv_update.py
index 5bfde27..86c18fe 100755
--- a/venv_update.py
+++ b/venv_update.py
@@ -71,7 +71,7 @@ from subprocess import CalledProcessError
 # To fix this we just delete the environment variable.
 os.environ.pop('__PYVENV_LAUNCHER__', None)
 
-__version__ = '3.2.4'
+__version__ = '4.0.0'
 DEFAULT_VIRTUALENV_PATH = 'venv'
 DEFAULT_OPTION_VALUES = {
     'venv=': (DEFAULT_VIRTUALENV_PATH,),
@@ -208,13 +208,11 @@ def exec_scratch_virtualenv(args):
     if not exists(scratch.python):
         run(('virtualenv', scratch.venv))
 
-    if not exists(join(scratch.src, 'virtualenv.py')):
+    if not exists(join(scratch.src, 'virtualenv')):
         scratch_python = venv_python(scratch.venv)
         # TODO: do we allow user-defined override of which version of virtualenv to install?
-        # https://github.com/Yelp/venv-update/issues/231 virtualenv 20+ is not supported.
         tmp = scratch.src + '.tmp'
-        run((scratch_python, '-m', 'pip.__main__', 'install', 'virtualenv<20', '--target', tmp))
-
+        run((scratch_python, '-m', 'pip.__main__', 'install', 'virtualenv>=20.0.8', '--target', tmp))
         from os import rename
         rename(tmp, scratch.src)
 
@@ -235,91 +233,95 @@ def get_original_path(venv_path):  # TODO-TEST: a unit test
     return check_output(('sh', '-c', '. %s; printf "$VIRTUAL_ENV"' % venv_executable(venv_path, 'activate')))
 
 
-def has_system_site_packages(interpreter):
-    # TODO: unit-test
-    system_site_packages = check_output((
-        interpreter,
-        '-c',
-        # stolen directly from virtualenv's site.py
-        """\
-import site, os.path
-print(
-    0
-    if os.path.exists(
-        os.path.join(os.path.dirname(site.__file__), 'no-global-site-packages.txt')
-    ) else
-    1
-)"""
-    ))
-    system_site_packages = int(system_site_packages)
-    assert system_site_packages in (0, 1)
-    return bool(system_site_packages)
-
-
 def get_python_version(interpreter):
     if not exists(interpreter):
         return None
 
-    cmd = (interpreter, '-c', 'import sys; print(sys.version)')
-    return check_output(cmd)
+    cmd = (interpreter, '-c', 'import sys; print(".".join(str(p) for p in sys.version_info))')
+    return check_output(cmd).strip()
 
 
-def invalid_virtualenv_reason(venv_path, source_python, destination_python, options):
+def invalid_virtualenv_reason(venv_path, source_python, destination_python, virtualenv_system_site_packages):
     try:
         orig_path = get_original_path(venv_path)
     except CalledProcessError:
         return 'could not inspect metadata'
     if not samefile(orig_path, venv_path):
         return 'virtualenv moved {} -> {}'.format(timid_relpath(orig_path), timid_relpath(venv_path))
-    elif has_system_site_packages(destination_python) != options.system_site_packages:
-        return 'system-site-packages changed, to %s' % options.system_site_packages
 
+    pyvenv_cfg_path = join(venv_path, 'pyvenv.cfg')
+
+    if not exists(pyvenv_cfg_path):
+        return 'virtualenv created with virtualenv<20'
+
+    # Avoid using pathlib.Path which doesn't exist in python2 and
+    # hack around configparser's inability to handle sectionless config
+    # files: https://bugs.python.org/issue22253
+    from configparser import ConfigParser
+    pyvenv_cfg = ConfigParser()
+    with open(pyvenv_cfg_path, 'r') as f:
+        pyvenv_cfg.read_string('[root]\n' + f.read())
+    if pyvenv_cfg.getboolean('root', 'include-system-site-packages', fallback=False) != virtualenv_system_site_packages:
+        return 'system-site-packages changed, to %s' % virtualenv_system_site_packages
     if source_python is None:
         return
-    destination_version = get_python_version(destination_python)
+
+    destination_version = pyvenv_cfg.get('root', 'version_info', fallback=None)
     source_version = get_python_version(source_python)
     if source_version != destination_version:
         return 'python version changed {} -> {}'.format(destination_version, source_version)
 
+    base_executable = pyvenv_cfg.get('root', 'base-executable', fallback=None)
+    base_executable_version = get_python_version(base_executable)
+    if base_executable_version != destination_version:
+        return 'base executable python version changed {} -> {}'.format(destination_version, base_executable_version)
+
 
 def ensure_virtualenv(args, return_values):
     """Ensure we have a valid virtualenv."""
-    def adjust_options(options, args):
-        # TODO-TEST: proper error message with no arguments
-        venv_path = return_values.venv_path = args[0]
 
-        if venv_path == DEFAULT_VIRTUALENV_PATH or options.prompt == '<dirname>':
+    from sys import argv
+    argv[:] = ('virtualenv',) + args
+    info(colorize(argv))
+
+    import virtualenv
+
+    run_virtualenv = True
+    filtered_args = [a for a in args if not a.startswith('-')]
+    if filtered_args:
+        venv_path = return_values.venv_path = filtered_args[0] if filtered_args else None
+        if venv_path == DEFAULT_VIRTUALENV_PATH:
             from os.path import abspath, basename, dirname
-            options.prompt = '(%s)' % basename(dirname(abspath(venv_path)))
-        # end of option munging.
+            args = ('--prompt=({})'.format(basename(dirname(abspath(venv_path)))),) + args
 
+        # Validate existing virtualenv if there is one
         # there are two python interpreters involved here:
         # 1) the interpreter we're instructing virtualenv to copy
-        if options.python is None:
+        python_options_arg = [a for a in args if a.startswith('-p') or a.startswith('--python')]
+        if not python_options_arg:
             source_python = None
         else:
-            source_python = virtualenv.resolve_interpreter(options.python)
+            virtualenv_session = virtualenv.session_via_cli(args)
+            source_python = virtualenv_session._interpreter.executable
         # 2) the interpreter virtualenv will create
         destination_python = venv_python(venv_path)
 
         if exists(destination_python):
-            reason = invalid_virtualenv_reason(venv_path, source_python, destination_python, options)
+            # Check if --system-site-packages is a passed-in option
+            dummy_session = virtualenv.session_via_cli(args)
+            virtualenv_system_site_packages = dummy_session.creator.enable_system_site_package
+
+            reason = invalid_virtualenv_reason(venv_path, source_python, destination_python, virtualenv_system_site_packages)
             if reason:
                 info('Removing invalidated virtualenv. (%s)' % reason)
                 run(('rm', '-rf', venv_path))
             else:
                 info('Keeping valid virtualenv from previous run.')
-                raise SystemExit(0)  # looks good! we're done here.
+                run_virtualenv = False  # looks good! we're done here.
 
-    # this is actually a documented extension point:
-    #   http://virtualenv.readthedocs.org/en/latest/reference.html#adjust_options
-    import virtualenv
-    virtualenv.adjust_options = adjust_options
+    if run_virtualenv:
+        raise_on_failure(lambda: virtualenv.cli_run(args), ignore_return=True)
 
-    from sys import argv
-    argv[:] = ('virtualenv',) + args
-    info(colorize(argv))
-    raise_on_failure(virtualenv.main)
     # There might not be a venv_path if doing something like "venv= --version"
     # and not actually asking virtualenv to make a venv.
     if return_values.venv_path is not None:
@@ -436,11 +438,11 @@ def pip_faster(venv_path, pip_command, install, bootstrap_deps):
     run(pip_command + install)
 
 
-def raise_on_failure(mainfunc):
+def raise_on_failure(mainfunc, ignore_return=False):
     """raise if and only if mainfunc fails"""
     try:
         errors = mainfunc()
-        if errors:
+        if not ignore_return and errors:
             exit(errors)
     except CalledProcessError as error:
         exit(error.returncode)


Test patch:
diff --git a/requirements.d/import_tests.txt b/requirements.d/import_tests.txt
index e97d141..bbd3236 100644
--- a/requirements.d/import_tests.txt
+++ b/requirements.d/import_tests.txt
@@ -5,5 +5,5 @@
 pytest>=3.6.0
 ephemeral-port-reserve
 pip>=10.0.0,<=18.1
-wheel<0.34.0
+wheel
 six
diff --git a/requirements.d/test.txt b/requirements.d/test.txt
index 23a4f06..626e7d5 100644
--- a/requirements.d/test.txt
+++ b/requirements.d/test.txt
@@ -1,7 +1,7 @@
 # minimal set of packages to run tests (under tox, for example)
 -r import_tests.txt
 
-virtualenv<20
+virtualenv>=20.0.8
 pytest-xdist
 
 pypiserver
diff --git a/tests/functional/faster.py b/tests/functional/faster.py
index 99f2779..1c1c53b 100644
--- a/tests/functional/faster.py
+++ b/tests/functional/faster.py
@@ -4,6 +4,7 @@ from __future__ import print_function
 from __future__ import unicode_literals
 
 import contextlib
+import sys
 
 import pytest
 from py._path.local import LocalPath as Path
@@ -103,6 +104,7 @@ def test_noop_install_faster(tmpdir):
     assert time_savings(tmpdir, between=do_nothing) > 6
 
 
+@pytest.mark.skipif('__pypy__' in sys.builtin_module_names, reason='slower under pypy for some reason')
 @pytest.mark.usefixtures('pypi_server_with_fallback', 'pypi_packages')
 def test_cached_clean_install_faster(tmpdir):
     def clean():
diff --git a/tests/functional/simple_test.py b/tests/functional/simple_test.py
index b76b079..86839f3 100644
--- a/tests/functional/simple_test.py
+++ b/tests/functional/simple_test.py
@@ -64,11 +64,18 @@ def test_arguments_version(tmpdir):
     # should show virtualenv version, successfully
     out, err = venv_update('venv=', '--version')
     err = strip_pip_warnings(err)
-    assert err == ''
+    if sys.version_info < (3, 0):
+        import virtualenv
+        assert 'virtualenv {}'.format(virtualenv.__version__) in err
+    else:
+        assert err == ''
 
     out = uncolor(out)
     lines = out.splitlines()
-    assert lines[-2] == '> virtualenv --version', repr(lines)
+    if sys.version_info < (3, 0):
+        assert lines[-1] == '> virtualenv --version', repr(lines)
+    else:
+        assert lines[-2] == '> virtualenv --version', repr(lines)
 
 
 @pytest.mark.skipif('__pypy__' in sys.builtin_module_names, reason="site-packages doesn't show up under pypy for some reason")
@@ -79,11 +86,14 @@ def test_arguments_system_packages(tmpdir):
     requirements('')
 
     venv_update('venv=', '--system-site-packages', 'venv')
-
+    # virtualenv>20 doesn't set sys.real_prefix anymore. The accepted method
+    # for checking if we are in a virtual environment is to check for base_prefix
+    # See: https://github.com/pypa/virtualenv/issues/1622
     out, err = run('venv/bin/python', '-c', '''\
 import sys
+non_venv_prefix = sys.real_prefix if hasattr(sys, "real_prefix") else sys.base_prefix
 for p in sys.path:
-    if p.startswith(sys.real_prefix) and p.endswith("-packages"):
+    if p.startswith(non_venv_prefix) and p.endswith("-packages"):
         print(p)
         break
 ''')
@@ -217,6 +227,7 @@ def pipe_output(read, write):
     vupdate = Popen(
         ('venv-update', 'venv=', '--version'),
         env=environ,
+        stderr=write,
         stdout=write,
         close_fds=True,
     )
@@ -233,10 +244,9 @@ def pipe_output(read, write):
     assert uncolored.startswith('> ')
     # FIXME: Sometimes this is 'python -m', sometimes 'python2.7 -m'. Weird.
     import virtualenv
-    assert uncolored.endswith('''
-> virtualenv --version
-%s
-''' % virtualenv.__version__)
+    split_uncolored = uncolored.strip().split('\n')
+    version_cmd_index = split_uncolored.index('> virtualenv --version')
+    assert 'virtualenv {}'.format(virtualenv.__version__) in split_uncolored[version_cmd_index + 1]
 
     return result, uncolored
 
@@ -276,19 +286,13 @@ def test_args_backward(tmpdir):
 
     with pytest.raises(CalledProcessError) as excinfo:
         venv_update('venv=', 'requirements.txt')
-
-    assert excinfo.value.returncode == 3
+    assert excinfo.value.returncode == 2
     out, err = excinfo.value.result
     err = strip_coverage_warnings(err)
     err = strip_pip_warnings(err)
-    assert err == ''
     out = uncolor(out)
-    assert out.rsplit('\n', 4)[-4:] == [
-        '> virtualenv requirements.txt',
-        'ERROR: File already exists and is not a directory.',
-        'Please provide a different path or delete the file.',
-        '',
-    ]
+    assert '> virtualenv requirements.txt' in out
+    assert 'virtualenv: error: argument dest: the destination requirements.txt already exists and is a file' in err
 
     assert Path('requirements.txt').isfile()
     assert Path('requirements.txt').read() == ''
diff --git a/tests/functional/validation.py b/tests/functional/validation.py
index f38a951..4eb67c2 100644
--- a/tests/functional/validation.py
+++ b/tests/functional/validation.py
@@ -2,6 +2,8 @@ from __future__ import absolute_import
 from __future__ import print_function
 from __future__ import unicode_literals
 
+import fileinput
+
 import pytest
 
 from testing import enable_coverage
@@ -170,6 +172,44 @@ Removing invalidated virtualenv. (could not inspect metadata)
 ''')
 
 
+@pytest.mark.usefixtures('pypi_server')
+def test_update_invalidated_missing_pyvenv_cfg(tmpdir):
+    with tmpdir.as_cwd():
+        enable_coverage()
+        requirements('')
+
+        venv_update()
+        tmpdir.join('venv/pyvenv.cfg').remove()
+
+        out, _ = venv_update()
+        out = uncolor(out)
+        assert out.startswith('''\
+> virtualenv venv
+Removing invalidated virtualenv. (virtualenv created with virtualenv<20)
+''')
+
+
+@pytest.mark.usefixtures('pypi_server')
+def test_update_invalidated_changed_base_executable(tmpdir):
+    with tmpdir.as_cwd():
+        enable_coverage()
+        requirements('')
+
+        from sys import executable as python
+        venv_update('venv=', '--python=' + python, 'venv')
+
+        for line in fileinput.input(str(tmpdir.join('venv/pyvenv.cfg')), inplace=True):
+            if line.startswith('base-executable = '):
+                print('base-executable = /some/other/python')
+            else:
+                print(line, end='')
+        out, _ = venv_update('venv=', '--python=' + python, 'venv')
+        out = uncolor(out)
+        assert out.startswith('''\
+> virtualenv --python={} venv
+Removing invalidated virtualenv. (base executable python version changed'''.format(python))
+
+
 @pytest.mark.usefixtures('pypi_server')
 def it_gives_the_same_python_version_as_we_started_with(tmpdir):
     other_python = OtherPython()
diff --git a/tests/regression/pip_faster.py b/tests/regression/pip_faster.py
index e4d460e..94cff77 100644
--- a/tests/regression/pip_faster.py
+++ b/tests/regression/pip_faster.py
@@ -64,7 +64,12 @@ tracing: circular-dep-b==1.0 (from circular-dep-a)
 )
 @pytest.mark.parametrize('reqs', [
     # new setuptools and old pip
-    ['setuptools==18.2', 'pip==1.4.1'],
+    [
+        'setuptools==18.2',
+        # Non-SNI compatible clients (i.e. pip<2.7.9) cannot access public pypi anymore.
+        # pip==6.0 is the earliest supported version. See https://github.com/pypa/pypi-support/issues/978
+        'pip==6.0',
+    ],
 ])
 def test_old_pip_and_setuptools(tmpdir, reqs):
     """We should be able to use pip-faster's wheel building even if we have
@@ -85,7 +90,7 @@ def test_old_pip_and_setuptools(tmpdir, reqs):
     # We need to add public PyPI as an extra URL since we're installing
     # packages (setuptools and pip) which aren't available from our PyPI fixture.
     from os import environ
-    environ['PIP_EXTRA_INDEX_URL'] = 'https://pypi.python.org/simple/'
+    environ['PIP_EXTRA_INDEX_URL'] = 'https://pypi.org/simple/'
     try:
         pip = venv.join('bin/pip').strpath
         for req in reqs:
diff --git a/tests/testing/__init__.py b/tests/testing/__init__.py
index d67f015..93fb14e 100644
--- a/tests/testing/__init__.py
+++ b/tests/testing/__init__.py
@@ -77,6 +77,7 @@ pip_warnings_regex = Regex(
         r"^You should consider upgrading via the 'pip install --upgrade pip' command\.\n",
         r"^DEPRECATION: Python 2\.7 will reach the end of its life on January 1st, 2020\. Please upgrade your Python as Python 2\.7 won't be maintained after that date\. A future version of pip will drop support for Python 2\.7\.\n",  # noqa: E501
         r"^DEPRECATION: Python 2\.7 will reach the end of its life on January 1st, 2020\. Please upgrade your Python as Python 2\.7 won't be maintained after that date\. A future version of pip will drop support for Python 2\.7\. More details about Python 2 support in pip, can be found at https://pip\.pypa\.io/en/latest/development/release-process/#python-2-support\n",  # noqa: E501
+        r'DEPRECATION: Python 2\.7 reached the end of its life on January 1st, 2020\. Please upgrade your Python as Python 2\.7 is no longer maintained\. pip 21\.0 will drop support for Python 2\.7 in January 2021\. More details about Python 2 support in pip can be found at https://pip\.pypa\.io/en/latest/development/release-process/#python-2-support pip 21\.0 will remove support for this functionality\.',  # noqa: E501
         r"^DEPRECATION: Python 3\.4 support has been deprecated. pip 19\.1 will be the last one supporting it\. Please upgrade your Python as Python 3\.4 won't be maintained after March 2019 \(cf PEP 429\)\.\n",   # noqa: E501
         r'^DEPRECATION: A future version of pip will drop support for Python 2\.7\.\n',  # noqa: E501
         r'^DEPRECATION: A future version of pip will drop support for Python 2\.7\. More details about Python 2 support in pip, can be found at https://pip\.pypa\.io/en/latest/development/release-process/#python-2-support\n',  # noqa: E501
diff --git a/tests/testing/make_sdists.py b/tests/testing/make_sdists.py
index 7b0a493..79a5302 100755
--- a/tests/testing/make_sdists.py
+++ b/tests/testing/make_sdists.py
@@ -112,7 +112,7 @@ def download_sdist(source, destination):
 
 def do_build(sources, destination):
     build_all(sources, destination)
-    wheel('virtualenv<20', destination)
+    wheel('virtualenv>=20.0.8', destination)
     wheel('coverage-enable-subprocess', destination)
     download_sdist('coverage', destination)
     download_sdist('coverage-enable-subprocess', destination)
diff --git a/tests/unit/simple_test.py b/tests/unit/simple_test.py
index 84e57df..afd421e 100644
--- a/tests/unit/simple_test.py
+++ b/tests/unit/simple_test.py
@@ -170,10 +170,9 @@ def test_user_cache_dir():
 def test_get_python_version():
     import sys
 
-    expected = '.'.join(str(part) for part in sys.version_info[:3])
+    expected = '.'.join(str(part) for part in sys.version_info)
     actual = venv_update.get_python_version(sys.executable)
-    assert actual.startswith(expected)
-    assert actual[len(expected)] in ' +'
+    assert expected == actual
 
     assert venv_update.get_python_version('total garbage') is None