Skip to content

Build certbot snap without the isolation build#8255

Merged
bmw merged 1 commit intocertbot:masterfrom
adferrand:no-build-isolation-snap
Sep 2, 2020
Merged

Build certbot snap without the isolation build#8255
bmw merged 1 commit intocertbot:masterfrom
adferrand:no-build-isolation-snap

Conversation

@adferrand
Copy link
Copy Markdown
Collaborator

@adferrand adferrand commented Sep 2, 2020

Fixes #8252

With @bmw we digged quite a lot on why the failure happens on ARM snap, and here we what we understood:

  • the failure occurs since the version 50 of setuptools is available
  • normally, we should not be impacted because the setuptools version used in the snap build should be the one installed by the core20 base snap, because the build occurs in a venv created with --system-site-packages
  • BUT associated with the build isolation provided by recent versions of pip (to implement PEP 517), a bad interaction happens: following the definition of the build system provided by cryptography, pip installs the most recent version of setuptools on a separate path for the build (because cryptography just asks for a minimal version of setuptools), then features of this version conflict with the old version of setuptools initially present
  • the exact interaction is described here: virtualenv with --system-site-packages breaks pip's build isolation pypa/pip#6264 (comment). Basically the new version of setuptools triggers some hacks, that are then applied at runtime on the old version of setuptools that is also still available in sys.path at this point, and breaks the build.

To fix that, one can disable the isolation build on cryptography, by passing PIP_NO_ISOLATION_BUILD=no to pip. It is the purpose of this PR.

This will have the consequence to not be PEP 517 compliant: if needed the cryptography library will be built using the setuptools available in the system. In general I think it makes sense for the snap build purpose, since we control precisely the build environment, and makes consistent build that will not be broken by a new version of a build system if library maintainers did not provide a strict version of it in their build requirements. However we need now to take care about having a compatible build system for all libraries that may have specific requirements in their build system using the PEP 517 definition in pyproject.toml.

I think as of now that it is a safe move if we keep using the most recent version of setuptools available in Ubuntu 20.04, and it is the case here for snap builds. It may however be problematic if some libraries require another build system than setuptools and do not provide a fallback to a setuptools build. For the record, dns-lexicon, that I maintain, uses poetry and so a PEP 517 compliant definition of a build system, but provides also this fallback (https://github.com/AnalogJ/lexicon/blob/master/setup.py).

@adferrand
Copy link
Copy Markdown
Collaborator Author

Full test suite compiling the snaps for the 3 architectures using this PR is available here: https://dev.azure.com/certbot/certbot/_build/results?buildId=2596&view=results

Copy link
Copy Markdown
Member

@bmw bmw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @adferrand! This LGTM.

I created #8256 to talk about additional work we may want to do here.

@bmw bmw merged commit 3ce87d1 into certbot:master Sep 2, 2020
@bmw bmw added this to the 1.8.0 milestone Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix Certbot snap builds on ARM

2 participants