◐ Shell
reader mode source ↗
Skip to content

bpo-43811: Test multiple OpenSSL versions on GHA#25360

Merged
miss-islington merged 2 commits into
python:masterfrom
tiran:bpo-43811-gha-multissl
Apr 13, 2021
Merged

bpo-43811: Test multiple OpenSSL versions on GHA#25360
miss-islington merged 2 commits into
python:masterfrom
tiran:bpo-43811-gha-multissl

Conversation

@tiran

@tiran tiran commented Apr 12, 2021 •
edited by miss-islington
Loading

Copy link
Copy Markdown
Member

The new checks are only executed when one or more OpenSSL-related files are modified. The checks run a handful of networking and hashing test suites. All SSL checks are optional. This PR also introduces ccache to speed up compilation. In common cases it speeds up configure and compile time from about 90 seconds to less than 30 seconds.

Signed-off-by: Christian Heimes christian@python.org

https://bugs.python.org/issue43811

Automerge-Triggered-By: GH:tiran

@tiran tiran requested review from vstinner and zooba April 12, 2021 10:02
@tiran

tiran commented Apr 12, 2021

Copy link
Copy Markdown
Member Author

#25361 addresses the unrelated test failures.

@tiran tiran force-pushed the bpo-43811-gha-multissl branch from a741341 to 302c398 Compare April 12, 2021 11:13
vstinner
vstinner reviewed Apr 12, 2021
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

Please don't add a new CI until all issues are fixed, like this one:

"GitHub Actions / Ubuntu SSL tests with OpenSSL 3.0.0-alpha14
‘ERR_func_error_string’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]"

@tiran

tiran commented Apr 12, 2021

Copy link
Copy Markdown
Member Author

Please don't add a new CI until all issues are fixed, like this one:

"GitHub Actions / Ubuntu SSL tests with OpenSSL 3.0.0-alpha14
‘ERR_func_error_string’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]"

This is a chicken and egg problem. #25329 needs this PR to verify that the code works.

@vstinner

vstinner commented Apr 12, 2021

Copy link
Copy Markdown
Member

This is a chicken and egg problem. #25329 needs this PR to verify that the code works.

I'm spending a lot of time to fix every single warning in Python. I would prefer to not add new warnings, especially if you know that a lot of will be needed to fix them.

Can't you run this CI job locally, fix warnings, and then automate checks on PRs?

I would prefer not annoy contributors who don't work on OpenSSL support with these new deprecation warnings.

@tiran

tiran commented Apr 12, 2021

Copy link
Copy Markdown
Member Author

This is a chicken and egg problem. #25329 needs this PR to verify that the code works.

I'm spending a lot of time to fix every single warning in Python. I would prefer to not add new warnings, especially if you know that a lot of will be needed to fix them.

Can't you run this CI job locally, fix warnings, and then automate checks on PRs?

I would prefer not annoy contributors who don't work on OpenSSL support with these new deprecation warnings.

I prefer to add CI first, so I have a stable baseline. Then I can fix deprecation warnings.

@tiran

tiran commented Apr 12, 2021

Copy link
Copy Markdown
Member Author

I have added a new check_source instruction. The additional OpenSSL tests are skipped unless the commit touches any file with ssl, hmac, hashlib or ^.github in the file name.

@tiran tiran force-pushed the bpo-43811-gha-multissl branch from 7e18dda to 3669c9f Compare April 13, 2021 06:46
@tiran

tiran commented Apr 13, 2021

Copy link
Copy Markdown
Member Author

@vstinner @pablogsal I have reworked the PR. Additional checks are optional and only executed when GHA detects a modification of an OpenSSL-related file or a file in .github directory. I'm postponing PEP 644 PR until I have finalized OpenSSL 3.0.0 support. It should be ready by next week.

@tiran tiran requested a review from vstinner April 13, 2021 16:26
vstinner
vstinner approved these changes Apr 13, 2021
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

LGTM.

Additional checks are optional and only executed when GHA detects a modification of an OpenSSL-related file

Ok, I'm fine with this PR in this case.

@miss-islington miss-islington merged commit 8fa1489 into python:master Apr 13, 2021
@miss-islington

miss-islington commented Apr 13, 2021

Copy link
Copy Markdown
Contributor

Thanks @tiran for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 13, 2021
@tiran @miss-islington
bpo-43811: Test multiple OpenSSL versions on GHA (pythonGH-25360) …
8e228c5 
The new checks are only executed when one or more OpenSSL-related files are modified. The checks run a handful of networking and hashing test suites. All SSL checks are optional. This PR also introduces ccache to speed up compilation. In common cases it speeds up configure and compile time from about 90 seconds to less than 30 seconds.

Signed-off-by: Christian Heimes <christian@python.org>
(cherry picked from commit 8fa1489)

Co-authored-by: Christian Heimes <christian@python.org>
@bedevere-bot

bedevere-bot commented Apr 13, 2021

Copy link
Copy Markdown

GH-25391 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 13, 2021
@tiran @miss-islington
bpo-43811: Test multiple OpenSSL versions on GHA (pythonGH-25360) …
65655ca 
The new checks are only executed when one or more OpenSSL-related files are modified. The checks run a handful of networking and hashing test suites. All SSL checks are optional. This PR also introduces ccache to speed up compilation. In common cases it speeds up configure and compile time from about 90 seconds to less than 30 seconds.

Signed-off-by: Christian Heimes <christian@python.org>
(cherry picked from commit 8fa1489)

Co-authored-by: Christian Heimes <christian@python.org>
@bedevere-bot

bedevere-bot commented Apr 13, 2021

Copy link
Copy Markdown

GH-25392 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Apr 13, 2021
@miss-islington
[3.9] bpo-43811: Test multiple OpenSSL versions on GHA (GH-25360) (GH… …
5ce227f 
…-25391)

The new checks are only executed when one or more OpenSSL-related files are modified. The checks run a handful of networking and hashing test suites. All SSL checks are optional. This PR also introduces ccache to speed up compilation. In common cases it speeds up configure and compile time from about 90 seconds to less than 30 seconds.

Signed-off-by: Christian Heimes <christian@python.org>
(cherry picked from commit 8fa1489)


Co-authored-by: Christian Heimes <christian@python.org>

Automerge-Triggered-By: GH:tiran
miss-islington added a commit that referenced this pull request Apr 13, 2021
@miss-islington
[3.8] bpo-43811: Test multiple OpenSSL versions on GHA (GH-25360) (GH… …
a607815 
…-25392)

The new checks are only executed when one or more OpenSSL-related files are modified. The checks run a handful of networking and hashing test suites. All SSL checks are optional. This PR also introduces ccache to speed up compilation. In common cases it speeds up configure and compile time from about 90 seconds to less than 30 seconds.

Signed-off-by: Christian Heimes <christian@python.org>
(cherry picked from commit 8fa1489)


Co-authored-by: Christian Heimes <christian@python.org>

Automerge-Triggered-By: GH:tiran
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

@zooba zooba

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tiran @vstinner @miss-islington @bedevere-bot @the-knights-who-say-ni