-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
bpo-28459: Fix _pyio on Cygwin where the msvcrt module is not built #14013
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
Conversation
Allow the UnixCCompiler.find_library_file to correctly find DLL import libs (.dll.a).
Instead use ctypes to access the setmode function provided by Cygwin.
|
Pinging core devs on this and #4153. Also I fixed the networking problems with my Cygwin buildbot worker so it's actually producing meaningful results again, and this will fix a number of tests. |
benjaminp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be an extra distutils change mixed in here.
| if sys.platform == 'win32': | ||
| from msvcrt import setmode as _setmode | ||
| elif sys.platform == 'cygwin': | ||
| import ctypes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should avoid ctypes in the io module. There probably should cygwin equivalent for msvcrt instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benjaminp This is the _pyio module used primarily only in testing. There is no "cygwin equivalent for msvcrt"--why should there be? The closest I can think of is my own PyCygwin package, and it's not part of the standard lib, though I'd be willing to make it so; I'm hardly interested in fighting for a new stdlib package just to fix one bug though.
As was discussed on the issue it would be possible to expose Cygwin's setmode() as os.setmode() but it could be confusing since it does not function the same the FreeBSD setmode().
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Thank you for looking at this issue, though #4153 is the one that really needs attention from a core dev. |
|
The bpo was closed under bpo-45537 (Cygwin is unsupported). |
|
This bug does not seem exist any longer. At least I could not replicate it with 3.9 on Cygwin 10.0. $ uname -a
CYGWIN_NT-10.0-22631 xps-ne 3.5.4-1.x86_64 2024-08-25 16:52 UTC x86_64 Cygwin
$ python pyio-example.py
io from '/usr/lib/python3.9/_pyio.py'
Hello, world!
import _pyio as io
with io.StringIO() as f:
f.write("Hello, world!")
f.seek(0)
content = f.read()
print(f"io from '{io.__file__}'")
print(content) # Output: Hello, world! |
Patch for bpo-28459, a long-standing issue on Cygwin especially for running some of the test suite since it uses
_pyio.This PR is based on top of #4153 which needs to be merged before this issue can be fixed, as the fix relies on ctypes, and the ctypes module can't be built without #4153.
https://bugs.python.org/issue28459