Skip to content

Revert "Disable build isolation during snap dns plugins build (#8319)"#8323

Merged
bmw merged 1 commit intomasterfrom
use-build-isolation
Sep 28, 2020
Merged

Revert "Disable build isolation during snap dns plugins build (#8319)"#8323
bmw merged 1 commit intomasterfrom
use-build-isolation

Conversation

@bmw
Copy link
Copy Markdown
Member

@bmw bmw commented Sep 28, 2020 •
edited
Loading

This reverts commit feca125.

Since this change landed, ARM builds for many of the DNS plugins have failed every night. See https://dev.azure.com/certbot/certbot/_build?definitionId=5 or our public Mattermost channel.

I quickly tried to fix this myself and wasn't trivially able to do so. I tried setting SNAPCRAFT_PYTHON_VENV_ARGS: --system-site-packages and adding python3-wheel as a build dependency, but it didn't work for some reason. The python3-wheel package didn't seem to be installed.

I still suspect something like this is the approach we should take, however, I want to fix the failing tests now so things are no longer broken in master and those of us on the Certbot team at EFF stop getting spammed with 54 (!!) emails about failed builds from launchpad every night.

Unfortunately, while I was working on this the queue for ARM machines on Launchpad jumped up to an estimated ~20 hour wait, but I confirmed that this fixes the problem by building on an ARM AMI using the instructions at https://github.com/certbot/certbot/blob/master/tools/snap/README.md#use-testing-and-development. If whoever reviews this would like an ARM machine to test on themselves, please let me know.

@adferrand or @alex @alexzorin, are you able to review this?

@alex
Copy link
Copy Markdown
Collaborator

alex commented Sep 28, 2020

cc: @reaperhulk

It looks like build isolation was originally disabled to fix ARM builds... but I don't understand why/how! Certainly using build isolation seems like the forward looking/modern packaging approach to me.

@bmw
Copy link
Copy Markdown
Member Author

bmw commented Sep 28, 2020 •
edited
Loading

Ah sorry for the noise! I tagged the wrong Alex. I meant to tag @alexzorin for review.

There is some fun going on with build isolation that we may want to talk with pyca and/or the pip maintainers about at some point, but we're just landing a workaround on our end for now. If you're really curious, you can check out #8256, but we were planning on only bringing things up with you all when/if we had a more concrete proposal or questions.

@adferrand adferrand self-assigned this Sep 28, 2020
adferrand
adferrand approved these changes Sep 28, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@adferrand adferrand left a comment

Choose a reason for hiding this comment

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

OK. So we need to find a more elegant solution. In the mean time, setuptools is fixed, so we can hope for few months before the next broken release of pip or setuptools!

@bmw bmw merged commit e066766 into master Sep 28, 2020
@bmw
Copy link
Copy Markdown
Member Author

bmw commented Sep 28, 2020

Thanks for the review!

@bmw bmw deleted the use-build-isolation branch September 28, 2020 21:27
@bmw bmw mentioned this pull request Sep 28, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adferrand adferrand adferrand approved these changes

Assignees

@adferrand adferrand

Labels

area: pkging area: snaps area: tooling

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bmw @alex @adferrand