-
-
Notifications
You must be signed in to change notification settings - Fork 55
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
assign Windows arm32 worker #89
Conversation
master/custom/factories.py
Outdated
@@ -369,3 +369,17 @@ class Windows64ICCReleaseBuild(Windows64ReleaseBuild): | |||
buildersuffix = ".nondebug" | |||
buildFlags = Windows64ReleaseBuild.buildFlags + windows_icc_build_flags | |||
factory_tags = ["win64", "icc", "nondebug"] | |||
|
|||
class WindowsArm32Build(WindowsBuild): | |||
buildFlags = ["-p", "ARM", "--no-tkinter", "--no-ssl"] |
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.
Didn't we get SSL working?
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.
@zooba
Yes. But it's not merged yet.
The PR is here: python/cpython#11779
And that depends on this patch python/cpython-source-deps#13 which has already been merged upstream in OpenSSL
Okay, I'm making a new OpenSSL build with that fix. If the only problem with the buildbot is that the tests fail, I'd rather go in with the failure. It's marked unstable for now, so it's okay, and it'll both save updating the config when it's fixed and will force us to make the fixes :) |
@zooba, Are you saying to remove the --no-ssl parameter now and let it fail until the ssl PR is merged? |
Yep, exactly. |
Ping @zware @pablogsal @vstinner - look like you all have commit rights? We're about to get the SSL changes in (I'm running them through the current buildbot fleet), and at that point we have no reason not to include Paul's machines in the unstable fleet. |
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.
One very minor change I'd like to see, otherwise looks OK.
master/custom/factories.py
Outdated
# skip test_multiprocessing_spawn for now | ||
testFlags = ["-arm32", "-j2", "-x", "test_multiprocessing_spawn", "-x", "test_winconsoleio", "-x", "test_distutils"] | ||
cleanFlags = ["-p", "ARM", "--no-tkinter"] | ||
factory_tags = ["arm32"] |
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 need some manner of Windows tag here too. Maybe win-arm32
instead of arm32
, or a separate windows
tag? Same below.
@zware Changes have been made - anything else? |
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.
One more change that I missed last time.
Co-Authored-By: Zachary Ware <zachary.ware@gmail.com>
@zware Thanks for reviewing. I merged your suggested change |
I expect this worker to fail at first.
For example tools/buildbot/test.bat needs to changed to support this worker: python/cpython#12917
There are also 10 more tests that need skips or fixes that I'm aware of.