Created on 2016-10-17 10:58 by erik.bray, last changed 2016-11-07 15:25 by erik.bray.
|cygwin-pyio-setmode.patch||masamoto, 2016-10-20 22:03||included other parts||review|
|msg278802 - (view)||Author: Erik Bray (erik.bray) *||Date: 2016-10-17 10:58|
Since the patch to #24881 was merged the _pyio module has been non-importable on Cygwin, due to an attempt to import from the msvcrt module. However, the msvcrt module is not built on Cygwin (Cygwin does not use MSVCRT). Cygwin's libc does, however, have _setmode. The problem this is trying to solve is to call _setmode (https://msdn.microsoft.com/en-us/library/tw4k6df8.aspx) on file descriptors to ensure that they are O_BINARY instead of O_TEXT (a distinction that needs to made on Windows). Fortunately, on Cygwin, it is fairly rare that a file descriptor will have mode O_TEXT, but it it is possible. See https://cygwin.com/faq/faq.html#faq.programming.msvcrt-and-cygwin This could be tricky to solve though. Removing setmode() call entirely works for me as far as the test suite is concerned. But it leaves _pyio slightly incongruous with the C implementation in this one small aspect, and it *is* a bug. I would propose for Python 3.7 adding an os.setmode() function. On Windows this could be simply an alias for msvcrt.setmode() (or vice-versa). But because setmode() is available also in Cygwin (and technically some other platforms too, though none that are currently supported by Python) it would be a good candidate for inclusion in the os module I think (for those platforms that have it).
|msg279086 - (view)||Author: Masayuki Yamamoto (masamoto) *||Date: 2016-10-20 22:03|
I agree to use setmode() at _pyio module for Cygwin. However, I have two reasons that I disagree to the implementation of os.setmode(). First, FreeBSD has another setmode() that operate file permission bits . Therefore the implementation of os.setmode() will be confused to those users. Second, msvcrt.setmode() isn't used almost in standard library, the function is just used by _pyio module and subprocess test. Thus I think there aren't much need that implements setmode() to os module. Easy way for a solution of import error on Cygwin, setmode() is implemented on _pyio using ctypes. then Cygwin CPython forget setmode(). Otherwise it's hard way, For avoiding confliction to other platforms, I'd propose to create the cygwin module. I thought a solution implementing imitation msvcrt module, but it is unreasonable because Cygwin doesn't have other Windows API. I tried written the patch of easy way that implements setmode() into _pyio. This patch is included parts to succeed import _pyio.  https://www.freebsd.org/cgi/man.cgi?query=setmode&sektion=3
|msg280203 - (view)||Author: Erik Bray (erik.bray) *||Date: 2016-11-07 15:25|
Hi Masayuki, thanks for the response (I thought I replied to this earlier but maybe I only imagined it--I've been on vacation). I agree with just about everything you write here. I'm aware of the FreeBSD setmode(), but I figured os.setmode() could do different things on different platforms as applicable. But that would also be very confusing. Your patch using ctypes looks fine to me--I considered doing the same, but what I'm not sure is if it's kosher to use ctypes here, given that it's techically an optional module. Since _pyio is, as I understand it, mainly used for testing it's probably fine? But I think we'll need some core maintainers' comments here... Implementing a cygwin module would be really nice, actually, even if it isn't included in the stdlib. There are a few other cygwin-specific APIs that it is useful to have wrappers around: https://cygwin.com/cygwin-api/cygwin-functions.html But that would be outside the scope of fixing this issue.
|2016-11-07 15:25:03||erik.bray||set||messages: + msg280203|
components: + Library (Lib)
versions: + Python 3.5, Python 3.6, Python 3.7
keywords: + patch
nosy: + masamoto
messages: + msg279086