Skip to content

Add some missing type libraries for mypy#9657

Merged
alexzorin merged 10 commits intomasterfrom
test-mypy-imports
Apr 9, 2023
Merged

Add some missing type libraries for mypy#9657
alexzorin merged 10 commits intomasterfrom
test-mypy-imports

Conversation

@bmw
Copy link
Copy Markdown
Member

@bmw bmw commented Apr 4, 2023

If this is merged, we should update #9606 to mention that the pinning back of setuptools should be removed when that issue is resolved. I'll do this myself if the reviewer does not.

pkg-config is needed as an additional dependency to build new versions of cryptography. See https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst#4000---2023-03-24. I added this dependency everywhere except for our Azure config where I instead removed the installation of a number of packages. My thinking was Azure always runs on x86 where pre-built wheels are available, but I think the other installation methods could potentially need these build dependencies. Needing them is unlikely, but I think just always installing them in that case and moving on is easiest for everyone.

@bmw bmw self-assigned this Apr 4, 2023
@bmw bmw marked this pull request as draft April 4, 2023 20:36
@bmw bmw marked this pull request as ready for review April 5, 2023 12:41
@bmw bmw removed their assignment Apr 5, 2023
Comment on lines +45 to +50
libffi-dev ca-certificates openssl pkg-config
# For RPM-based distributions (e.g. Fedora, CentOS ...)
# NB1: old distributions will use yum instead of dnf
# NB2: RHEL-based distributions use python3X-devel instead of python3-devel (e.g. python36-devel)
sudo dnf install python3-devel gcc augeas-libs openssl-devel libffi-devel \
redhat-rpm-config ca-certificates openssl
redhat-rpm-config ca-certificates openssl pkg-config
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If I understand correctly, we are mentioning pkg-config here because of the possibility that a cryptography wheel is not available on their platform. In that case, should we also mention that the Rust toolchain is required?

I'd also be happy if we did the same thing as https://certbot.eff.org/instructions?ws=other&os=pip, which is to link to the cryptography documentation where it lists per-platform dependencies. Or saying nothing at all but not including the superfluous dependencies.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If I understand correctly, we are mentioning pkg-config here because of the possibility that a cryptography wheel is not available on their platform.

That's correct. That's actually what many of these dependencies are listed for and it just looks like we never updated them after cryptography added the rust dependency.

I'm not 100% sure all tests will pass with the changes I just made, but if they do, how about something like that?

@alexzorin alexzorin self-assigned this Apr 5, 2023
nginx-light
sudo systemctl stop nginx
sudo sysctl net.ipv4.ip_unprivileged_port_start=0
condition: startswith(variables['IMAGE_NAME'], 'ubuntu')
Copy link
Copy Markdown
Member Author

@bmw bmw Apr 6, 2023

Choose a reason for hiding this comment

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

Python is getting installed in the next step in this file.

Copy link
Copy Markdown
Collaborator

@alexzorin alexzorin left a comment

Choose a reason for hiding this comment

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

Cool! LGTM.

@alexzorin alexzorin merged commit 5149dfd into master Apr 9, 2023
@alexzorin alexzorin deleted the test-mypy-imports branch April 9, 2023 01:49
@bmw bmw mentioned this pull request Apr 10, 2023
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.

2 participants