Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix python 3 compatibility in server_http.py #350

Merged
merged 2 commits into from
Feb 16, 2024
Merged

Fix python 3 compatibility in server_http.py #350

merged 2 commits into from
Feb 16, 2024

Conversation

ThinLinc-Zeijlon
Copy link
Contributor

Building osslsigncode fails on systems with older versions of Python 3. This is due to the server_http.py script, that runs as a part of the test procedure. It imports the ThreadingHTTPServer-module which was added to Python in version 3.7.

Replacing ThreadingHTTPServer with a plain HTTPServer allows us to build for older versions of Python 3 without excluding the tests from the build.

Building osslsigncode fails on systems with older versions of Python 3.
This is due to the server_http.py script, that runs as a part of the
test procedure. It imports the ThreadingHTTPServer-module which was
added to Python in version 3.7.

Replacing ThreadingHTTPServer with a plain HTTPServer allows us to build
for older versions of Python 3 without excluding the tests from the
build.
@mtrojnar
Copy link
Owner

Our tests don't require HTTP server concurrency indeed, because we only issue one HTTP request at a time.

@mtrojnar
Copy link
Owner

The PR causes Linux and OSX tests to hang. It can't be merged before the hang is fixed..

@olszomal
Copy link
Collaborator

@ThinLinc-Zeijlon Shutdown for HTTPServer hangs.
What do you think about this fix:

threading.Thread(target=server.shutdown, daemon=True).start()

@mtrojnar OSX tests are permanently broken.

@mtrojnar
Copy link
Owner

@mtrojnar OSX tests are permanently broken.

I did notice that. Although they were failing without this PR, this PR makes them also hanging, which was not the case before.

This reverts the previous commit. Turns out the ThreadingMixIn class,
used by ThreadingHTTPServer in Python >= 3.7 has existed for a long
time. Let's use it to create a ThreadingHTTPServer "locally" to remain
backward compatible with older versions of Python.
@ThinLinc-Zeijlon
Copy link
Contributor Author

@ThinLinc-Zeijlon Shutdown for HTTPServer hangs. What do you think about this fix:

threading.Thread(target=server.shutdown, daemon=True).start()

Yes, maybe that could work.

Please see my latest commit also, turns out it was easy to replicate the the ThreadingHTTPServer locally, maybe that can be a way forward also.

@mtrojnar
Copy link
Owner

Please see my latest commit also, turns out it was easy to replicate the the ThreadingHTTPServer locally, maybe that can be a way forward also.

I like this solution.

@mtrojnar mtrojnar merged commit 42e9733 into mtrojnar:master Feb 16, 2024
8 of 9 checks passed
@ThinLinc-Zeijlon ThinLinc-Zeijlon deleted the fix_test_python_compat branch February 21, 2024 12:00
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.

3 participants