Skip to content

Conversation

@embray
Copy link
Contributor

@embray embray commented Jun 12, 2019

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

ma8ma and others added 4 commits June 12, 2019 13:55
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.
@brettcannon brettcannon added the type-bug An unexpected behavior, bug, or error label Jun 21, 2019
@embray
Copy link
Contributor Author

embray commented Jul 2, 2019

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.

Copy link
Contributor

@benjaminp benjaminp left a 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
Copy link
Contributor

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.

Copy link
Contributor Author

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().

@bedevere-bot
Copy link

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@embray
Copy link
Contributor Author

embray commented Sep 18, 2019

@benjaminp

There seems to be an extra distutils change mixed in here.

See issue description

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.

@embray
Copy link
Contributor Author

embray commented Sep 18, 2019

Thank you for looking at this issue, though #4153 is the one that really needs attention from a core dev.

@iritkatriel
Copy link
Member

iritkatriel commented Oct 20, 2021

The bpo was closed under bpo-45537 (Cygwin is unsupported).

@phdye
Copy link

phdye commented Sep 9, 2024

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!

pyio-example.py :

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting changes type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants