From 944d0e05c854e51c0de6bdd44bc00af39e8db672 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 5 Apr 2019 15:01:09 -0700 Subject: [PATCH] Use venv over virtualenv in venv3 (#6922) Fixes #6861. _venv_common.py is no longer executable. The reason for this is the venv creation logic is now different between Python 2 and Python 3. We could add code that branches on the Python version running the script, but I personally think that's unnecessary. --setuptools and --no-site-packages is no longer passed to virtualenv either. These flags were made noops in virtualenv 1.10 and 1.7 respectively, but all of CentOS 6, 7, Debian 8+, and Ubuntu 14.04+ have new enough versions of virtualenv where these flags are no longer necessary. They are not even accepted as flags to Python 3's venv module. Use of VENV_ARGS from test_sdists.sh was also removed because that environment variable hasn't done anything in a while. I ran test farm tests on test_apache2.sh and test_sdists.sh with these changes and they passed. * Fixes #6861. * _venv_common is no longer executable. --- tests/letstest/scripts/test_apache2.sh | 2 +- tests/letstest/scripts/test_sdists.sh | 4 +- tools/_venv_common.py | 72 ++++++++++++++++++++------ tools/venv.py | 42 ++++++--------- tools/venv3.py | 44 ++++++---------- 5 files changed, 90 insertions(+), 74 deletions(-) mode change 100755 => 100644 tools/_venv_common.py diff --git a/tests/letstest/scripts/test_apache2.sh b/tests/letstest/scripts/test_apache2.sh index d24de2458..c52578003 100755 --- a/tests/letstest/scripts/test_apache2.sh +++ b/tests/letstest/scripts/test_apache2.sh @@ -45,7 +45,7 @@ if [ $? -ne 0 ] ; then exit 1 fi -python tools/_venv_common.py -e acme[dev] -e .[dev,docs] -e certbot-apache +python tools/venv.py -e acme[dev] -e .[dev,docs] -e certbot-apache sudo venv/bin/certbot -v --debug --text --agree-dev-preview --agree-tos \ --renew-by-default --redirect --register-unsafely-without-email \ --domain $PUBLIC_HOSTNAME --server $BOULDER_URL diff --git a/tests/letstest/scripts/test_sdists.sh b/tests/letstest/scripts/test_sdists.sh index 260a0acfb..a9177f690 100755 --- a/tests/letstest/scripts/test_sdists.sh +++ b/tests/letstest/scripts/test_sdists.sh @@ -4,13 +4,11 @@ cd letsencrypt ./certbot-auto --os-packages-only -n --debug PLUGINS="certbot-apache certbot-nginx" -PYTHON=$(command -v python2.7 || command -v python27 || command -v python2 || command -v python) TEMP_DIR=$(mktemp -d) VERSION=$(letsencrypt-auto-source/version.py) -export VENV_ARGS="-p $PYTHON" # setup venv -tools/_venv_common.py --requirement letsencrypt-auto-source/pieces/dependency-requirements.txt +tools/venv.py --requirement letsencrypt-auto-source/pieces/dependency-requirements.txt . ./venv/bin/activate # pytest is needed to run tests on some of our packages so we install a pinned version here. tools/pip_install.py pytest diff --git a/tools/_venv_common.py b/tools/_venv_common.py old mode 100755 new mode 100644 index 09383b4c0..cb19d583c --- a/tools/_venv_common.py +++ b/tools/_venv_common.py @@ -19,7 +19,30 @@ import time import subprocess import sys import re -import shlex + +REQUIREMENTS = [ + '-e acme[dev]', + '-e .[dev,docs]', + '-e certbot-apache', + '-e certbot-dns-cloudflare', + '-e certbot-dns-cloudxns', + '-e certbot-dns-digitalocean', + '-e certbot-dns-dnsimple', + '-e certbot-dns-dnsmadeeasy', + '-e certbot-dns-gehirn', + '-e certbot-dns-google', + '-e certbot-dns-linode', + '-e certbot-dns-luadns', + '-e certbot-dns-nsone', + '-e certbot-dns-ovh', + '-e certbot-dns-rfc2136', + '-e certbot-dns-route53', + '-e certbot-dns-sakuracloud', + '-e certbot-nginx', + '-e certbot-postfix', + '-e letshelp-certbot', + '-e certbot-compatibility-test', +] VERSION_PATTERN = re.compile(r'^(\d+)\.(\d+).*$') @@ -120,16 +143,22 @@ def get_venv_python_path(venv_path): .format(venv_path))) -def main(venv_name, venv_args, args): - """Creates a virtual environment and installs packages. +def prepare_venv_path(venv_name): + """Determines the venv path and prepares it for use. + + This function cleans up any Python eggs in the current working directory + and ensures the venv path is available for use. The path used is the + VENV_NAME environment variable if it is set and venv_name otherwise. If + there is already a directory at the desired path, the existing directory is + renamed by appending a timestamp to the directory name. :param str venv_name: The name or path at where the virtual - environment should be created. - :param str venv_args: Command line arguments for virtualenv - :param str args: Command line arguments that should be given to pip - to install packages - """ + environment should be created if VENV_NAME isn't set. + :returns: path where the virtual environment should be created + :rtype: str + + """ for path in glob.glob('*.egg-info'): if os.path.isdir(path): shutil.rmtree(path) @@ -145,15 +174,30 @@ def main(venv_name, venv_args, args): if os.path.isdir(venv_name): os.rename(venv_name, '{0}.{1}.bak'.format(venv_name, int(time.time()))) - command = [sys.executable, '-m', 'virtualenv', '--no-site-packages', '--setuptools', venv_name] - command.extend(shlex.split(venv_args)) - subprocess_with_print(command) + return venv_name + + +def install_packages(venv_name, pip_args=None): + """Installs packages in the given venv. + + If pip_args is given, they are the arguments given to pip, + otherwise, REQUIREMENTS is used. + + :param str venv_name: The name or path at where the virtual + environment should be created. + :param pip_args: Command line arguments that should be given to + pip to install packages + :type pip_args: `list` of `str` + + """ + if not pip_args: + pip_args = REQUIREMENTS # Using the python executable from venv, we ensure to execute following commands in this venv. py_venv = get_venv_python_path(venv_name) subprocess_with_print([py_venv, os.path.abspath('letsencrypt-auto-source/pieces/pipstrap.py')]) command = [py_venv, os.path.abspath('tools/pip_install.py')] - command.extend(args) + command.extend(pip_args) subprocess_with_print(command) if os.path.isdir(os.path.join(venv_name, 'bin')): @@ -171,7 +215,3 @@ def main(venv_name, venv_args, args): print('---------------------------------------------------------------------------') else: raise ValueError('Error, directory {0} is not a valid venv.'.format(venv_name)) - - -if __name__ == '__main__': - main('venv', '', sys.argv[1:]) diff --git a/tools/venv.py b/tools/venv.py index 93b012e76..3fe8e05fb 100755 --- a/tools/venv.py +++ b/tools/venv.py @@ -1,41 +1,29 @@ #!/usr/bin/env python # Developer virtualenv setup for Certbot client import os +import sys import _venv_common -REQUIREMENTS = [ - '-e acme[dev]', - '-e .[dev,docs]', - '-e certbot-apache', - '-e certbot-dns-cloudflare', - '-e certbot-dns-cloudxns', - '-e certbot-dns-digitalocean', - '-e certbot-dns-dnsimple', - '-e certbot-dns-dnsmadeeasy', - '-e certbot-dns-gehirn', - '-e certbot-dns-google', - '-e certbot-dns-linode', - '-e certbot-dns-luadns', - '-e certbot-dns-nsone', - '-e certbot-dns-ovh', - '-e certbot-dns-rfc2136', - '-e certbot-dns-route53', - '-e certbot-dns-sakuracloud', - '-e certbot-nginx', - '-e certbot-postfix', - '-e letshelp-certbot', - '-e certbot-compatibility-test', -] +def create_venv(venv_path): + """Create a Python 2 virtual environment at venv_path. + + :param str venv_path: path where the venv should be created + + """ + python2 = _venv_common.find_python_executable(2) + command = [sys.executable, '-m', 'virtualenv', '--python', python2, venv_path] + _venv_common.subprocess_with_print(command) -def main(): +def main(pip_args=None): if os.name == 'nt': raise ValueError('Certbot for Windows is not supported on Python 2.x.') - venv_args = '--python "{0}"'.format(_venv_common.find_python_executable(2)) - _venv_common.main('venv', venv_args, REQUIREMENTS) + venv_path = _venv_common.prepare_venv_path('venv') + create_venv(venv_path) + _venv_common.install_packages(venv_path, pip_args) if __name__ == '__main__': - main() + main(sys.argv[1:]) diff --git a/tools/venv3.py b/tools/venv3.py index b837baf70..dc56a322e 100755 --- a/tools/venv3.py +++ b/tools/venv3.py @@ -1,36 +1,26 @@ #!/usr/bin/env python3 # Developer virtualenv setup for Certbot client +import sys + import _venv_common -REQUIREMENTS = [ - '-e acme[dev]', - '-e .[dev,docs]', - '-e certbot-apache', - '-e certbot-dns-cloudflare', - '-e certbot-dns-cloudxns', - '-e certbot-dns-digitalocean', - '-e certbot-dns-dnsimple', - '-e certbot-dns-dnsmadeeasy', - '-e certbot-dns-gehirn', - '-e certbot-dns-google', - '-e certbot-dns-linode', - '-e certbot-dns-luadns', - '-e certbot-dns-nsone', - '-e certbot-dns-ovh', - '-e certbot-dns-rfc2136', - '-e certbot-dns-route53', - '-e certbot-dns-sakuracloud', - '-e certbot-nginx', - '-e certbot-postfix', - '-e letshelp-certbot', - '-e certbot-compatibility-test', -] + +def create_venv(venv_path): + """Create a Python 3 virtual environment at venv_path. + + :param str venv_path: path where the venv should be created + + """ + python3 = _venv_common.find_python_executable(3) + command = [python3, '-m', 'venv', venv_path] + _venv_common.subprocess_with_print(command) -def main(): - venv_args = '--python "{0}"'.format(_venv_common.find_python_executable(3)) - _venv_common.main('venv3', venv_args, REQUIREMENTS) +def main(pip_args=None): + venv_path = _venv_common.prepare_venv_path('venv3') + create_venv(venv_path) + _venv_common.install_packages(venv_path, pip_args) if __name__ == '__main__': - main() + main(sys.argv[1:])