Redo the majority of Certbot's pinning system (certbot#8741)

* add initial pyproject.toml

* add extra dependencies

* add simple bash script

* polish

* reuse pipstrap

* add requirements.txt

* temporarily remove hashin dep

* Switch to requirements.txt

* remove hashin check

* update requirements.txt again

* remove unnecessary merge

* pin back augeas

* unpin cryptography

* simplify pywin32 pinning

* update comment

* pin back pytest and pylint

* pin back pytest-forked

* pin back coverage

* update script comments

* fix pyopenssl case

* add minimum poetry version

* run pin.sh

.gitignore

@@ -11,6 +11,7 @@ dist*/
 letsencrypt.log
 certbot.log
 letsencrypt-auto-source/letsencrypt-auto.sig.lzma.base64
+poetry.lock
 
 # coverage
 .coverage

certbot/setup.py

@@ -59,7 +59,7 @@ def read_file(filename, encoding='utf8'):
 # However environment markers are supported only with setuptools >= 36.2.
 # So this dependency is not added for old Linux distributions with old setuptools,
 # in order to allow these systems to build certbot from sources.
-pywin32_req = 'pywin32>=300'  # do not forget to edit pywin32 dependency accordingly in windows-installer/construct.py
+pywin32_req = 'pywin32>=300'
 setuptools_known_environment_markers = (LooseVersion(setuptools_version) >= LooseVersion('36.2'))
 if setuptools_known_environment_markers:
     install_requires.append(pywin32_req + " ; sys_platform == 'win32'")
@@ -79,6 +79,9 @@ def read_file(filename, encoding='utf8'):
     'ipdb',
     'mypy',
     'PyGithub',
+    # 1.1.0+ is required for poetry to use the poetry-core library for the
+    # build system declared in tools/pinning/pyproject.toml.
+    'poetry>=1.1.0',
     'pylint',
     'pytest',
     'pytest-cov',

snap/snapcraft.yaml

@@ -93,9 +93,7 @@ parts:
       snapcraftctl build
     override-pull: |
       snapcraftctl pull
-      python3 "${SNAPCRAFT_PART_SRC}/tools/strip_hashes.py" "${SNAPCRAFT_PART_SRC}/tools/certbot_constraints.txt" | grep -v python-augeas >> "${SNAPCRAFT_PART_SRC}/snap-constraints.txt"
-      python3 "${SNAPCRAFT_PART_SRC}/tools/strip_hashes.py" "${SNAPCRAFT_PART_SRC}/tools/pipstrap_constraints.txt" >> "${SNAPCRAFT_PART_SRC}/snap-constraints.txt"
-      echo "$(python3 "${SNAPCRAFT_PART_SRC}/tools/merge_requirements.py" "${SNAPCRAFT_PART_SRC}/snap-constraints.txt")" > "${SNAPCRAFT_PART_SRC}/snap-constraints.txt"
+      grep -v python-augeas "${SNAPCRAFT_PART_SRC}/tools/requirements.txt" >> "${SNAPCRAFT_PART_SRC}/snap-constraints.txt"
       snapcraftctl set-version `grep -oP "__version__ = '\K.*(?=')" "${SNAPCRAFT_PART_SRC}/certbot/certbot/__init__.py"`
   shared-metadata:
     plugin: dump

tests/letstest/scripts/test_sdists.sh

@@ -18,18 +18,15 @@ tools/pip_install.py pytest
 # setup constraints
 TEMP_DIR=$(mktemp -d)
 CONSTRAINTS="$TEMP_DIR/constraints.txt"
-# We strip the hashes because we don't have hashes of our local packages and
-# the mix of hashed and unhashed packages makes pip error out.
-python3 tools/strip_hashes.py tools/certbot_constraints.txt > "$CONSTRAINTS"
-python3 tools/strip_hashes.py tools/pipstrap_constraints.txt >> "$CONSTRAINTS"
+cp tools/requirements.txt "$CONSTRAINTS"
 
-# We pin cryptography to 3.1.1 and pyOpenSSL to 19.1.0 specifically for CentOS 7 / RHEL 7
+# We pin cryptography to 3.1.1 and pyopenssl to 19.1.0 specifically for CentOS 7 / RHEL 7
 # because these systems ship only with OpenSSL 1.0.2, and this OpenSSL version support has been
-# dropped on cryptography>=3.2 and pyOpenSSL>=20.0.0.
-# Using this old version of OpenSSL would break the cryptography and pyOpenSSL wheels builds.
+# dropped on cryptography>=3.2 and pyopenssl>=20.0.0.
+# Using this old version of OpenSSL would break the cryptography and pyopenssl wheels builds.
 if [ -f /etc/redhat-release ] && [ "$(. /etc/os-release 2> /dev/null && echo "$VERSION_ID" | cut -d '.' -f1)" -eq 7 ]; then
   sed -i 's|cryptography==.*|cryptography==3.1.1|g' "$CONSTRAINTS"
-  sed -i 's|pyOpenSSL==.*|pyOpenSSL==19.1.0|g' "$CONSTRAINTS"
+  sed -i 's|pyopenssl==.*|pyopenssl==19.1.0|g' "$CONSTRAINTS"
 fi
 
 

tools/dev_constraints.txt

@@ -1,7 +1,7 @@
 # Specifies Python package versions for development and building Docker images.
 # It includes in particular packages not specified in tools/certbot_constraints.txt.
 # Some dev package versions specified here may be overridden by higher level constraints
-# files during tests (eg. tools/certbot_constraints.txt).
+# files during tests (eg. tools/oldest_constraints.txt).
 alabaster==0.7.10
 apacheconfig==0.3.2
 apipkg==1.4

tools/pinning/pin.sh

@@ -0,0 +1,51 @@
+#!/bin/bash
+# This script accepts no arguments and automates the process of updating
+# Certbot's dependencies. Dependencies can be pinned to older versions by
+# modifying pyproject.toml in the same directory as this file.
+set -euo pipefail
+
+WORK_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )"
+REPO_ROOT="$(dirname "$(dirname "${WORK_DIR}")")"
+PIPSTRAP_CONSTRAINTS="${REPO_ROOT}/tools/pipstrap_constraints.txt"
+RELATIVE_SCRIPT_PATH="$(realpath --relative-to "$REPO_ROOT" "$WORK_DIR")/$(basename "${BASH_SOURCE[0]}")"
+REQUIREMENTS_FILE="$REPO_ROOT/tools/requirements.txt"
+STRIP_HASHES="${REPO_ROOT}/tools/strip_hashes.py"
+
+if ! command -v poetry >/dev/null; then
+    echo "Please install poetry."
+    echo "You may need to recreate Certbot's virtual environment and activate it."
+    exit 1
+fi
+
+cd "${WORK_DIR}"
+
+if [ -f poetry.lock ]; then
+    rm poetry.lock
+fi
+
+poetry lock
+
+TEMP_REQUIREMENTS=$(mktemp)
+trap 'rm poetry.lock; rm $TEMP_REQUIREMENTS' EXIT
+
+poetry export -o "${TEMP_REQUIREMENTS}" --without-hashes
+# We need to remove local packages from the requirements file.
+sed -i '/^acme @/d; /certbot/d;' "${TEMP_REQUIREMENTS}"
+# Poetry currently will not include pip, setuptools, or wheel in lockfiles or
+# requirements files. This was resolved by
+# https://github.com/python-poetry/poetry/pull/2826, but as of writing this it
+# hasn't been included in a release yet. For now, we continue to keep
+# pipstrap's pinning separate which has the added benefit of having it continue
+# to check hashes when pipstrap is run directly.
+"${STRIP_HASHES}" "${PIPSTRAP_CONSTRAINTS}" >>  "${TEMP_REQUIREMENTS}"
+
+cat << EOF > "$REQUIREMENTS_FILE"
+# This file was generated by $RELATIVE_SCRIPT_PATH and can be updated using
+# that script.
+#
+# It is normally used as constraints to pip, however, it has the name
+# requirements.txt so that is scanned by GitHub. See
+# https://docs.github.com/en/github/visualizing-repository-data-with-graphs/about-the-dependency-graph#supported-package-ecosystems
+# for more info.
+EOF
+cat "${TEMP_REQUIREMENTS}" >> "${REQUIREMENTS_FILE}"

tools/pinning/pyproject.toml

@@ -0,0 +1,56 @@
+[tool.poetry]
+name = "certbot-pinner"
+version = "0.1.0"
+description = "A simple project for pinning Certbot's dependencies using Poetry."
+authors = ["Certbot Project"]
+license = "Apache License 2.0"
+
+[tool.poetry.dependencies]
+python = "^3.6"
+
+# Local dependencies
+# Any local packages that have dependencies on other local packages must be
+# listed below before the package it depends on. For instance, certbot depends
+# on acme so certbot must be listed before acme.
+certbot-ci = {path = "../../certbot-ci", extras = ["docs"]}
+certbot-compatibility-test = {path = "../../certbot-compatibility-test", extras = ["docs"]}
+certbot-dns-cloudflare = {path = "../../certbot-dns-cloudflare", extras = ["docs"]}
+certbot-dns-cloudxns = {path = "../../certbot-dns-cloudxns", extras = ["docs"]}
+certbot-dns-digitalocean = {path = "../../certbot-dns-digitalocean", extras = ["docs"]}
+certbot-dns-dnsimple = {path = "../../certbot-dns-dnsimple", extras = ["docs"]}
+certbot-dns-dnsmadeeasy = {path = "../../certbot-dns-dnsmadeeasy", extras = ["docs"]}
+certbot-dns-gehirn = {path = "../../certbot-dns-gehirn", extras = ["docs"]}
+certbot-dns-google = {path = "../../certbot-dns-google", extras = ["docs"]}
+certbot-dns-linode = {path = "../../certbot-dns-linode", extras = ["docs"]}
+certbot-dns-luadns = {path = "../../certbot-dns-luadns", extras = ["docs"]}
+certbot-dns-nsone = {path = "../../certbot-dns-nsone", extras = ["docs"]}
+certbot-dns-ovh = {path = "../../certbot-dns-ovh", extras = ["docs"]}
+certbot-dns-rfc2136 = {path = "../../certbot-dns-rfc2136", extras = ["docs"]}
+certbot-dns-route53 = {path = "../../certbot-dns-route53", extras = ["docs"]}
+certbot-dns-sakuracloud = {path = "../../certbot-dns-sakuracloud", extras = ["docs"]}
+certbot-nginx = {path = "../../certbot-nginx", extras = ["docs"]}
+certbot-apache = {path = "../../certbot-apache", extras = ["dev"]}
+certbot = {path = "../../certbot", extras = ["dev", "docs"]}
+acme = {path = "../../acme", extras = ["dev", "docs"]}
+
+# Extra dependencies
+# See https://github.com/certbot/certbot/issues/8425.
+mypy = "0.710"
+# Upgrading coverage, pylint, pytest, and some of pytest's plugins causes many
+# test failures so let's pin these packages back for now.
+coverage = "4.5.4"
+pylint = "2.4.3"
+pytest = "3.2.5"
+pytest-forked = "0.2"
+# We were originally pinning back python-augeas for certbot-auto because we
+# found the way older versions of the library linked to Augeas were more
+# reliable. That's no longer a concern, however, we continue to pin back the
+# library for now because it causes Certbot tests on Windows to fail. See
+# https://github.com/certbot/certbot/issues/8732.
+python-augeas = "0.5.0"
+
+[tool.poetry.dev-dependencies]
+
+[build-system]
+requires = ["poetry-core>=1.0.0"]
+build-backend = "poetry.core.masonry.api"

tools/pip_install.py

@@ -39,51 +39,34 @@ def find_tools_path():
     return os.path.dirname(readlink.main(__file__))
 
 
-def certbot_oldest_processing(tools_path, args, test_constraints):
+def certbot_oldest_processing(tools_path, args, constraints_path):
     if args[0] != '-e' or len(args) != 2:
         raise ValueError('When CERTBOT_OLDEST is set, this script must be run '
                          'with a single -e <path> argument.')
     # remove any extras such as [dev]
     pkg_dir = re.sub(r'\[\w+\]', '', args[1])
+    # The order of the files in this list matters as files specified later can
+    # override the pinnings found in earlier files.
+    pinning_files = [os.path.join(tools_path, 'dev_constraints.txt'),
+                     os.path.join(tools_path, 'oldest_constraints.txt')]
     requirements = os.path.join(pkg_dir, 'local-oldest-requirements.txt')
-    shutil.copy(os.path.join(tools_path, 'oldest_constraints.txt'), test_constraints)
     # packages like acme don't have any local oldest requirements
-    if not os.path.isfile(requirements):
-        return None
-
+    if os.path.isfile(requirements):
+        # We add requirements to the end of the list so it can override
+        # anything that it needs to.
+        pinning_files.append(requirements)
+    else:
+        requirements = None
+    with open(constraints_path, 'w') as fd:
+        fd.write(merge_module.main(*pinning_files))
     return requirements
 
 
-def certbot_normal_processing(tools_path, test_constraints):
+def certbot_normal_processing(tools_path, constraints_path):
     repo_path = os.path.dirname(tools_path)
-    certbot_requirements = os.path.normpath(os.path.join(
-        repo_path, 'tools/certbot_constraints.txt'))
-    with open(certbot_requirements, 'r') as fd:
-        certbot_reqs = fd.readlines()
-    with open(os.path.join(tools_path, 'pipstrap_constraints.txt'), 'r') as fd:
-        pipstrap_reqs = fd.readlines()
-    with open(test_constraints, 'w') as fd:
-        data_certbot = "\n".join(strip_hashes.process_entries(certbot_reqs))
-        data_pipstrap = "\n".join(strip_hashes.process_entries(pipstrap_reqs))
-        data = "\n".join([data_certbot, data_pipstrap])
-        fd.write(data)
-
-
-def merge_requirements(tools_path, requirements, test_constraints, all_constraints):
-    # Order of the files in the merge function matters.
-    # Indeed version retained for a given package will be the last version
-    # found when following all requirements in the given order.
-    # Here is the order by increasing priority:
-    # 1) The general development constraints (tools/dev_constraints.txt)
-    # 2) The general tests constraints (oldest_requirements.txt or
-    #    certbot_constraints.txt + pipstrap's constraints for the normal processing)
-    # 3) The local requirement file, typically local-oldest-requirement in oldest tests
-    files = [os.path.join(tools_path, 'dev_constraints.txt'), test_constraints]
-    if requirements:
-        files.append(requirements)
-    merged_requirements = merge_module.main(*files)
-    with open(all_constraints, 'w') as fd:
-        fd.write(merged_requirements)
+    requirements = os.path.normpath(os.path.join(
+        repo_path, 'tools/requirements.txt'))
+    shutil.copy(requirements, constraints_path)
 
 
 def call_with_print(command, env=None):
@@ -104,24 +87,21 @@ def main(args):
     tools_path = find_tools_path()
 
     with temporary_directory() as working_dir:
-        test_constraints = os.path.join(working_dir, 'test_constraints.txt')
-        all_constraints = os.path.join(working_dir, 'all_constraints.txt')
-
         if os.environ.get('CERTBOT_NO_PIN') == '1':
             # With unpinned dependencies, there is no constraint
             pip_install_with_print(' '.join(args))
         else:
             # Otherwise, we merge requirements to build the constraints and pin dependencies
+            constraints_path = os.path.join(working_dir, 'constraints.txt')
             requirements = None
             if os.environ.get('CERTBOT_OLDEST') == '1':
-                requirements = certbot_oldest_processing(tools_path, args, test_constraints)
+                requirements = certbot_oldest_processing(tools_path, args, constraints_path)
             else:
-                certbot_normal_processing(tools_path, test_constraints)
+                certbot_normal_processing(tools_path, constraints_path)
 
             env = os.environ.copy()
-            env["PIP_CONSTRAINT"] = all_constraints
+            env["PIP_CONSTRAINT"] = constraints_path
 
-            merge_requirements(tools_path, requirements, test_constraints, all_constraints)
             if requirements:  # This branch is executed during the oldest tests
                 # First step, install the transitive dependencies of oldest requirements
                 # in respect with oldest constraints.