-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Make GitHub Actions work on Windows #4324
Make GitHub Actions work on Windows #4324
Conversation
Tools/ci-run.sh
Outdated
# 4127 warns that conditional expression is constant, should be fixed here https://github.com/cython/cython/pull/4317 | ||
# (off by default) 5045 warns that Compiler will insert Spectre mitigation for memory load if /Qspectre switch specified | ||
# (off by default) 4820 warns about the code in Python\3.9.6\x64\include ... | ||
WARNARGS="/W4 /wd4711 /wd4127 /wd5045 /wd4820" |
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.
/W4
is recommended and gives an adequate number of warnings, but we can use /Wall
+ ignore unnecessary warnings (additionally to those /wd
s)
And of course those 2.7 ascii fails are here, they are even represented in main, so not the fault of this PR |
Perhaps break this into a number of PRs: the first one to add one version of python on windows (choose the easiest), then in subsequent PRs add more versions of windows. |
Triggering a fresh CI build. |
hmp, the problem seems to be with the path given to disutils?
|
Ah, yeah, there are problems on Windows with long file names, I heard that before. That's unfortunate. But we can probably do something smarter anyhow, like making sure that we detect when an absolute path is within the working directory and truncate the common prefix, before we create the long temp/working directory path from it. That would probably help in a lot of real world cases. (I think we already do that somewhere – should make sure that we didn't just forget some places.) |
@scoder Do you have any suggestions where to look for this? If the problem is really in this then if we'll find the place where we do it we could fix it .. sounds like something captain obvious would say ;) but yeah |
ping. I am hitting the "too long" file names in windows runs of CI over at pypy/binary-testing. In NumPy testing, we override the distutils build command to cd into the directory and then compile. Maybe pxyimport could do the same? |
You can enable long file names in CI |
@blink1073 I tried that here but am still getting the same error. According to the error documentation there is a hard limit of 260 characters based on |
Ouch |
We should all just go back to 8.3 filenames. That would make our lives so much easier.
|
Merging #4630 to test it with the windows CI |
#4630 was merged, so this can move forward ever so slightly |
Well, yeah, although all of the tests are borken at the point, it's probably nice to mergre this, so it would be easier to work on those problems. |
Let's get this merged. It's more than what we currently have, and we already know that some tests are failing on appveyor as well, difference being that we look there less often. Thanks @0dminnimda. |
I created a follow-up ticket regarding the slow build times on Windows. #4963 |
I added tests for all versions of python to windows and solved some problems to let them work, but some don't:
2.7, 3.4 - since no suitable compiler version is installed
3.9 - inline, details: #3450, #4379
3.5 - one cpp-only test fails, output
I also uncommented the coverage tests and added some code to the .sh to make them work. They work for a decent amount of time, which is the opposite of what was said in the comments.
If anyone wants to try to fix the problems and run 2.7 (and maybe 3.4), then you will need to use this, probably relying on both versions of this answer.