Finish pinning system rewrite (#8934)

* add oldest pyproject.toml file that works

* make single oldest_constraints.txt file

* remove unused merge_requirements.py

* remove unused import

* make conditional right

* simplify pip_install.py

* fix typo

* bump min dns-lexicon dependency

* fix zope import warning

* pin back wheel

* refactor pinning script

* Add oldest script.

* add pip comment

* add pipstrap extra

* simplify pinning scripts

* remove pipstrap extra

* update contributing

* Add design doc

* Update tools/pinning/DESIGN.md

Co-authored-by: ohemorange <erica@eff.org>

* Update tools/pinning/DESIGN.md

Co-authored-by: ohemorange <erica@eff.org>

* Update tools/pinning/DESIGN.md

Co-authored-by: ohemorange <erica@eff.org>

* Update tools/pinning/DESIGN.md

Co-authored-by: ohemorange <erica@eff.org>

* rename normal to current

* no dummies

* script improvements

* mention need to update setup.py

* try and clarify poetry behavior

* tweak section title

Co-authored-by: ohemorange <erica@eff.org>
This commit is contained in:
Brad Warren
2021-07-22 12:00:30 -07:00
committed by GitHub
co-authored by ohemorange
parent 10eecf9c97
commit 08839758bd
26 changed files with 583 additions and 452 deletions
+125
View File
@@ -0,0 +1,125 @@
# Certbot dependency pinning
As described in the developer guide, we try to pin Certbot's dependencies to
well tested versions in almost all cases. Pinning Python dependencies across
different environments like this is actually quite tricky though and the files
under this directory make a somewhat reasonable, best effort approach to solve
this problem.
## Python packaging background
### Sdists and wheels
Python projects are most commonly distributed on [PyPI](https://pypi.org/) as
either a [source
distribution](https://packaging.python.org/glossary/#term-Source-Distribution-or-sdist)
or a [wheel](https://packaging.python.org/glossary/#term-Wheel). Wheels don't
present a problem for us pinning dependencies because they offer a well defined
format where the dependencies of a package can be easily parsed.
Source distributions or "sdists" are a problem though because they can contain
arbitrary Python code that must be run to determine the dependencies of the
package. This code could theoretically do anything, but it most commonly
inspects the environment it is running in and changes the project's
dependencies based the environment. As of writing this, we even do this in some
of Certbot's packages as you can see
[here](https://github.com/certbot/certbot/blob/8b610239bfcf7aac06f6e36d09a5abba3ba87047/certbot-dns-cloudflare/setup.py#L15-L27).
This is a problem because it means the environment an sdist was run in affects
the dependencies it declares making it difficult for us to determine Certbot's
dependencies for an arbitrary environment. Luckily, this is becoming less and
less of an issue with the increasing use of wheels, however, as of writing
this, some of Certbot's dependencies are still only available as sdists on some
platforms.
### Environment markers and pyproject.toml
Two other things have helped reduce the problems caused by sdists and are
relevant here. The first is the usage of [environment
markers](https://www.python.org/dev/peps/pep-0496/) which allows a package to
consistently declare its conditional dependencies with a static string
specifying the conditions where a dependency is required instead of dynamically
generating the list of required dependencies at runtime. This static string
keeps the package's declaration of its dependencies consistent across
environments.
The other relatively recent change in Python packaging is the adoption of
[pyproject.toml files](https://www.python.org/dev/peps/pep-0518/) which allows
sdists to define their packages using a static file instead of a setup.py
file, which has historically been the norm.
Using a static file instead of arbitrary Python code makes it
much easier for package declarations to be reliably interpreted. The
introduction of pyproject.toml also allows for the use of build systems other
than setuptools which becomes relevant in the next section of this doc.
## Our pinning system
### Overview
The files inside `tools/pinning` are used to generate Certbot's pinning files.
The files under `oldest` are used to generate the constraints file used for our
"oldest" tests while `current` is used to generate the constraints used
everywhere else. `common` includes shared files that are used for both sets of
pinnings.
Under `current` and `oldest`, there are two files as of writing this. One is a
pyproject.toml file for use with [Poetry](https://python-poetry.org/) while
the other is a script that can be run to regenerate pinnings. The
pyproject.toml file defines a Python package that depends on everything we want
to pin. This file largely just depends on our own local packages, however,
extra dependencies can be declared to further constrain package versions or to
declare additional dependencies.
The reason we use Poetry is that it is somewhat unique among Python packaging
tools in that when locking dependencies, it makes a best effort approach to do
this for all environments rather than just the current environment. This
includes recursively resolving dependencies declared through environment
markers that are not relevant for the current platform. It also includes
checking all wheels and sdists of a package for dependencies when picking a
specific version of a package from PyPI. You can see this in action through
the inclusion of dependencies like pywin32 which we only have a dependency on
for Windows.
### Potential problems
As of writing this, I'm aware of two potential problems with this pinning
system. The first is largely described earlier in the doc which is the problem
of sdists that use code to dynamically declare its dependencies. It's simply
not feasible to ensure this arbitrary Python code declares its dependencies in
the same way across all environments. Luckily, this is a largely a theoretical
problem and I'm aware of no issues with our current dependencies.
The second problem with this approach is that build dependencies are not
tracked and pinned. Unfortunately, [this seems to be a largely unsolved problem
in Python right
now](https://discuss.python.org/t/pinning-build-dependencies/8363). Our tooling
ensures that when installing build dependencies when using our pinning files
that versions from the pinning files are used (see
https://github.com/certbot/certbot/pull/8443 for more info about that), but I'm
not aware of any tool that automates the process of tracking and pinning down
build dependencies. For now, if we find any unpinned build dependencies, we can
declare a dependency on them in pyproject.toml. If a build dependency isn't
included in the constraints file, pip will use the latest version available on
PyPI.
## Theoretical future work
I think the system described above should work pretty well and I think it's
much better than the system we had before where how to update things like our
"oldest" pinnings was an open question. If we wanted to improve on this in the
future though, I think things to consider would be:
1. We could require that wheels are used for all of our dependencies. If a
wheel is not available for one of our dependencies, we could try to work
with upstream to change that or build it and host it locally for ourselves.
(If we do the latter, how to pin build dependencies when building the wheel
remains an open question.)
2. We could only really try to pin our dependencies for certain environments.
This would be done by doing something like installing our packages in each
environment we care about and saving the output of a command like `pip
freeze`. With our use of snaps and Docker, this may be somewhat reasonable
because we could base them all on a common system like Ubuntu LTS, however,
it's not entirely trivial because we still have problems such as supporting
multiple CPU architectures and pinning dependencies for Windows. Alternative
development and test environments also wouldn't be fully supported.
3. We could help build better tooling that solves some of the problems with
this approach or adopts it when it becomes available.
+44
View File
@@ -0,0 +1,44 @@
#!/bin/bash
# This script accepts a directory containing a pyproject.toml file configured
# for use with poetry and generates and prints the pinned dependencies of that
# file. Any dependencies on acme or those referencing certbot will be removed
# from the output. The exported requirements are printed to stdout.
#
# For example, if a directory containing a pyproject.toml file for poetry is at
# ../current, you could activate Certbot's developer environment and then run a
# command like the following to generate requirements.txt for that environment:
# ./export-pinned-dependencies.sh ../current > requirements.txt
set -euo pipefail
# If this script wasn't given a command line argument, print usage and exit.
if [ -z ${1+x} ]; then
echo "Usage:" >&2
echo "$0 PYPROJECT_TOML_DIRECTORY" >&2
exit 1
fi
REPO_ROOT="$(git rev-parse --show-toplevel)"
WORK_DIR="$1"
if ! command -v poetry >/dev/null || [ $(poetry --version | grep -oE '[0-9]+\.[0-9]+' | sed 's/\.//') -lt 12 ]; then
echo "Please install poetry 1.2+." >&2
echo "You may need to recreate Certbot's virtual environment and activate it." >&2
exit 1
fi
# Old eggs can cause outdated dependency information to be used by poetry so we
# delete them before generating the lock file. See
# https://github.com/python-poetry/poetry/issues/4103 for more info.
rm -rf ${REPO_ROOT}/*.egg-info
cd "${WORK_DIR}"
if [ -f poetry.lock ]; then
rm poetry.lock
fi
poetry lock >&2
trap 'rm poetry.lock' EXIT
# We need to remove local packages from the output.
poetry export --without-hashes | sed '/^acme @/d; /certbot/d;'
@@ -12,28 +12,28 @@ python = "^3.6"
# 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"}
certbot-compatibility-test = {path = "../../certbot-compatibility-test"}
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"}
certbot-apache = {path = "../../certbot-apache", extras = ["dev"]}
certbot = {path = "../../certbot", extras = ["all"]}
acme = {path = "../../acme", extras = ["docs", "test"]}
letstest = {path = "../../letstest"}
windows-installer = {path = "../../windows-installer"}
certbot-ci = {path = "../../../certbot-ci"}
certbot-compatibility-test = {path = "../../../certbot-compatibility-test"}
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"}
certbot-apache = {path = "../../../certbot-apache", extras = ["dev"]}
certbot = {path = "../../../certbot", extras = ["all"]}
acme = {path = "../../../acme", extras = ["docs", "test"]}
letstest = {path = "../../../letstest"}
windows-installer = {path = "../../../windows-installer"}
# Extra dependencies
# awscli is just listed here as a performance optimization. As of writing this,
+24
View File
@@ -0,0 +1,24 @@
#!/bin/bash
# This script accepts no arguments and automates the process of updating
# Certbot's dependencies including automatically updating the correct file.
# 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 )"
COMMON_DIR="$(dirname "${WORK_DIR}")/common"
REPO_ROOT="$(git rev-parse --show-toplevel)"
RELATIVE_SCRIPT_PATH="$(realpath --relative-to "$REPO_ROOT" "$WORK_DIR")/$(basename "${BASH_SOURCE[0]}")"
REQUIREMENTS_FILE="$REPO_ROOT/tools/requirements.txt"
PINNINGS=$("${COMMON_DIR}/export-pinned-dependencies.sh" "${WORK_DIR}")
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
echo "${PINNINGS}" >> "${REQUIREMENTS_FILE}"
+145
View File
@@ -0,0 +1,145 @@
[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]
# The Python version here should be kept in sync with the one used in our
# oldest tests in tox.ini.
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"}
certbot-dns-cloudflare = {path = "../../../certbot-dns-cloudflare"}
certbot-dns-cloudxns = {path = "../../../certbot-dns-cloudxns"}
certbot-dns-digitalocean = {path = "../../../certbot-dns-digitalocean"}
certbot-dns-dnsimple = {path = "../../../certbot-dns-dnsimple"}
certbot-dns-dnsmadeeasy = {path = "../../../certbot-dns-dnsmadeeasy"}
certbot-dns-gehirn = {path = "../../../certbot-dns-gehirn"}
certbot-dns-google = {path = "../../../certbot-dns-google"}
certbot-dns-linode = {path = "../../../certbot-dns-linode"}
certbot-dns-luadns = {path = "../../../certbot-dns-luadns"}
certbot-dns-nsone = {path = "../../../certbot-dns-nsone"}
certbot-dns-ovh = {path = "../../../certbot-dns-ovh"}
certbot-dns-rfc2136 = {path = "../../../certbot-dns-rfc2136"}
certbot-dns-route53 = {path = "../../../certbot-dns-route53"}
certbot-dns-sakuracloud = {path = "../../../certbot-dns-sakuracloud"}
certbot-nginx = {path = "../../../certbot-nginx"}
certbot-apache = {path = "../../../certbot-apache", extras = ["dev"]}
certbot = {path = "../../../certbot", extras = ["test"]}
acme = {path = "../../../acme", extras = ["test"]}
# Oldest dependencies
# We specify the oldest versions our dependencies that we're trying to keep
# support for below. Usually these version numbers are taken from the packages
# of our dependencies available in popular LTS Linux distros. Keeping
# compatibility with those versions makes it much easier for OS maintainers to
# update their Certbot packages.
#
# When updating these dependencies, we should ideally try to only update them
# to the oldest version of the dependency that is found in a non-EOL'd version
# of CentOS, Debian, or Ubuntu that has Certbot packages in their OS
# repositories using a version of Python we support. If the distro is EOL'd or
# using a version of Python we don't support, it can be ignored. If the
# dependency being updated is a direct dependency of one of our own packages,
# the minimum required version of that dependency should be updated in our
# setup.py files as well to communicate this information to our users.
# CentOS/RHEL 7 EPEL dependencies
# Some of these dependencies may be stricter than necessary because they
# initially referred to the Python 2 packages in CentOS/RHEL 7 with EPEL.
cffi = "1.9.1"
chardet = "2.2.1"
ipaddress = "1.0.16"
mock = "1.0.1"
ndg-httpsclient = "0.3.2"
ply = "3.4"
pyOpenSSL = "17.3.0"
pyasn1 = "0.1.9"
pycparser = "2.14"
pyRFC3339 = "1.0"
python-augeas = "0.5.0"
oauth2client = "4.0.0"
requests = "2.14.2"
urllib3 = "1.10.2"
# Package names containing "." need to be quoted.
"zope.component" = "4.1.0"
"zope.event" = "4.0.3"
"zope.interface" = "4.0.5"
# Debian Jessie Backports dependencies
# Debian Jessie has reached end of life so these dependencies can probably be
# updated as needed or desired.
pbr = "1.8.0"
pytz = "2012rc0"
# Debian Buster dependencies
google-api-python-client = "1.5.5"
pyparsing = "2.2.0"
# Our setup.py dependencies
apacheconfig = "0.3.2"
cloudflare = "1.5.1"
python-digitalocean = "1.11"
# Ubuntu Xenial dependencies
# Ubuntu Xenial only has versions of Python which we do not support available
# so these dependencies can probably be updated as needed or desired.
ConfigArgParse = "0.10.0"
funcsigs = "0.4"
# Package names containing "." need to be quoted.
"zope.hookable" = "4.0.4"
# Ubuntu Bionic dependencies.
cryptography = "2.1.4"
distro = "1.0.1"
httplib2 = "0.9.2"
idna = "2.6"
setuptools = "39.0.1"
six = "1.11.0"
# Ubuntu Focal dependencies
asn1crypto = "0.24.0"
configobj = "5.0.6"
parsedatetime = "2.4"
# Plugin dependencies
# These aren't necessarily the oldest versions we need to support
# Tracking at https://github.com/certbot/certbot/issues/6473
boto3 = "1.4.7"
botocore = "1.7.41"
dns-lexicon = "3.2.1"
# Build dependencies
# Since there doesn't appear to
# doesn't appear to be a good way to automatically track down and pin build
# dependencies in Python (see
# https://discuss.python.org/t/how-to-pin-build-dependencies/8238), we list any
# build dependencies here to ensure they're pinned for extra stability.
# cython is a build dependency of pyyaml
cython = "*"
# Other dependencies
# We add any dependencies that must be specified in this file for any another
# reason below.
# pip's new dependency resolver fails on local packages that depend on each
# other when those packages are requested with extras such as 'certbot[dev]' so
# let's pin it back for now. See https://github.com/pypa/pip/issues/9204.
pip = "20.2.4"
# wheel 0.34.1+ does not support the version of setuptools pinned above (and
# wheel 0.34.0 is buggy).
wheel = "<0.34.0"
[tool.poetry.dev-dependencies]
[build-system]
requires = ["poetry-core>=1.0.0"]
build-backend = "poetry.core.masonry.api"
+19
View File
@@ -0,0 +1,19 @@
#!/bin/bash
# This script accepts no arguments and automates the process of updating
# Certbot's dependencies including automatically updating the correct file.
# 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 )"
COMMON_DIR="$(dirname "${WORK_DIR}")/common"
REPO_ROOT="$(git rev-parse --show-toplevel)"
RELATIVE_SCRIPT_PATH="$(realpath --relative-to "$REPO_ROOT" "$WORK_DIR")/$(basename "${BASH_SOURCE[0]}")"
CONSTRAINTS_FILE="$REPO_ROOT/tools/oldest_constraints.txt"
PINNINGS=$("${COMMON_DIR}/export-pinned-dependencies.sh" "${WORK_DIR}")
cat << EOF > "$CONSTRAINTS_FILE"
# This file was generated by $RELATIVE_SCRIPT_PATH and can be updated using
# that script.
EOF
echo "${PINNINGS}" >> "${CONSTRAINTS_FILE}"
-48
View File
@@ -1,48 +0,0 @@
#!/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}")")"
RELATIVE_SCRIPT_PATH="$(realpath --relative-to "$REPO_ROOT" "$WORK_DIR")/$(basename "${BASH_SOURCE[0]}")"
REQUIREMENTS_FILE="$REPO_ROOT/tools/requirements.txt"
if ! command -v poetry >/dev/null || [ $(poetry --version | grep -oE '[0-9]+\.[0-9]+' | sed 's/\.//') -lt 12 ]; then
echo "Please install poetry 1.2+."
echo "You may need to recreate Certbot's virtual environment and activate it."
exit 1
fi
# Old eggs can cause outdated dependency information to be used by poetry so we
# delete them before generating the lock file. See
# https://github.com/python-poetry/poetry/issues/4103 for more info.
cd "${REPO_ROOT}"
rm -rf */*.egg-info
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}"
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}"