Build certbot snap without the isolation build#8255
Merged
bmw merged 1 commit intocertbot:masterfrom Sep 2, 2020
Merged
Conversation
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 |
bmw
approved these changes
Sep 2, 2020
Member
bmw
left a comment
There was a problem hiding this comment.
Thanks @adferrand! This LGTM.
I created #8256 to talk about additional work we may want to do here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8252
With @bmw we digged quite a lot on why the failure happens on ARM snap, and here we what we understood:
core20base snap, because the build occurs in avenvcreated with--system-site-packagescryptography, pip installs the most recent version of setuptools on a separate path for the build (becausecryptographyjust asks for a minimal version ofsetuptools), then features of this version conflict with the old version ofsetuptoolsinitially presentsetuptoolstriggers some hacks, that are then applied at runtime on the old version ofsetuptoolsthat is also still available insys.pathat this point, and breaks the build.To fix that, one can disable the isolation build on cryptography, by passing
PIP_NO_ISOLATION_BUILD=noto pip. It is the purpose of this PR.This will have the consequence to not be PEP 517 compliant: if needed the
cryptographylibrary will be built using thesetuptoolsavailable 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 inpyproject.toml.I think as of now that it is a safe move if we keep using the most recent version of
setuptoolsavailable 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 thansetuptoolsand do not provide a fallback to asetuptoolsbuild. For the record,dns-lexicon, that I maintain, usespoetryand so a PEP 517 compliant definition of a build system, but provides also this fallback (https://github.com/AnalogJ/lexicon/blob/master/setup.py).