Issue31883
Created on 2017-10-27 13:42 by erik.bray, last changed 2022-04-11 14:58 by admin. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 4151 | closed | erik.bray, 2017-10-27 13:45 | |
| Messages (7) | |||
|---|---|---|---|
| msg305120 - (view) | Author: Erik Bray (erik.bray) * ![]() |
Date: 2017-10-27 13:42 | |
There is an acknowledged bug in Cygwin's implementation of wcsxfrm() [1] that can cause heap corruption in certain cases. This bug has since been fixed in Cygwin 2.8.1-1 [2] and all current and future releases. However, that was relatively recent (July 2017) so it may still crop up. I also have a workaround for this from the Python side, but rather than clutter the code with workarounds for platform-specific bugs I think it suffices just to skip the test in this case. [1] https://cygwin.com/ml/cygwin/2017-05/msg00149.html [2] https://cygwin.com/ml/cygwin-announce/2017-07/msg00002.html |
|||
| msg305126 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2017-10-27 16:56 | |
Since this bug has been fixed in Cygwin, I don't think we should add a workaround for it. The expected date of Python 3.7 release is 2018-06-15, this is far from the date of releasing the fixed Cygwin. |
|||
| msg305131 - (view) | Author: Erik Bray (erik.bray) * ![]() |
Date: 2017-10-27 18:20 | |
To be clear, are you saying there shouldn't be a workaround to the underlying issue (I agree), or are you saying the test skip shouldn't even be added? I'm in favor of just skipping the test since it's still a problem on (currently) recent Cygwin versions. And it's not a very critical test so I'm happy to just skip it in this case. |
|||
| msg305133 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2017-10-27 19:42 | |
According to your reference a problem is fixed in recent Cygwin versions, isn't? Skipping the test on Cygwin means that wcsxfrm() will be untested on this platform. Future regressions will be unnoticed. |
|||
| msg305215 - (view) | Author: Erik Bray (erik.bray) * ![]() |
Date: 2017-10-30 10:05 | |
Well, I agree there's an unfortunate trade-off here: One can skip the test, allowing the test suite to work on slightly older versions of Cygwin, from before this issue was fixed. However, I then lose regression testing on newer versions. My personal feeling in this case is that it's not a very important test and can be skipped in the future (although the bug that this test caught was fairly serious and one would want regression testing for it...) As an alternative I could be more fine-grained and only skip the test on older versions of Cygwin that are affected. I was hoping to avoid putting in too much Cygwin-specific test utilities, though adding a version check for Cygwin is not terribly hard (I do the same for my Cygwin port of psutil). For reference, the current version of Cygwin that comes installed on AppVeyor (for which I'm trying to get a Cygwin build set up) is 2.8.0, which *is* (just barely) affected by this bug. |
|||
| msg305217 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2017-10-30 10:14 | |
This is a problem of AppVeyor. It should update Cygwin to a bugfix release. 2.8.0 contains other bugs. |
|||
| msg305224 - (view) | Author: Erik Bray (erik.bray) * ![]() |
Date: 2017-10-30 11:38 | |
Well, we'll see how long it takes me to get them to respond on that. If it's not too much trouble then I'll be happy to drop this issue. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:53 | admin | set | github: 76064 |
| 2017-10-30 11:38:25 | erik.bray | set | messages: + msg305224 |
| 2017-10-30 10:14:51 | serhiy.storchaka | set | status: open -> closed resolution: out of date messages: + msg305217 stage: patch review -> resolved |
| 2017-10-30 10:05:08 | erik.bray | set | messages: + msg305215 |
| 2017-10-27 19:42:06 | serhiy.storchaka | set | messages: + msg305133 |
| 2017-10-27 18:20:03 | erik.bray | set | messages: + msg305131 |
| 2017-10-27 16:56:30 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages: + msg305126 |
| 2017-10-27 13:45:28 | erik.bray | set | keywords:
+ patch stage: patch review pull_requests: + pull_request4116 |
| 2017-10-27 13:42:11 | erik.bray | create | |

