Skip to content
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

bpo-36511: Fix Windows arm32 buildbot, pythoninfo, scp, and ssh #91

Merged
merged 2 commits into from
Jun 13, 2019

Conversation

paulmon
Copy link
Contributor

@paulmon paulmon commented May 20, 2019

When running the buildbot for Windows ARM32 test.pythoninfo must be run remotely.
Also seperating deploy step from both pythoninfo and test.bat
This requires a cpython PR to work: python/cpython#13454
I think this should work. Feedback is appreciated.

@zooba @zware

@zooba
Copy link
Member

zooba commented May 20, 2019

I think this could also be done by handling the flag specially in the test.bat file, yes? Rather than making more modifications here.

I'll leave it to @zware as to where he'd like the split to be - I don't have a strong preference.

@paulmon
Copy link
Contributor Author

paulmon commented May 20, 2019

I found that test.pythoninfo is called directly from a buildbot step without calling any of the .bat files in tools\buildbot currently. This is why I didn't catch it before.

ShellCommand(
name="pythoninfo",
description="pythoninfo",
command=self.python_command + ["-m", "test.pythoninfo"],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line doesn't call any .bat files. It ends up executing an arm32 binary on the x86/x64 build host which doesn't work

@paulmon
Copy link
Contributor Author

paulmon commented May 22, 2019

The must-have parts of this PR are that the test files must be copied to the test device before pythoninfo and pythoninfo must be run on the test device, not the build host. Currently it's not possible to change where pythoninfo runs without changing the buildmaster-config

@paulmon
Copy link
Contributor Author

paulmon commented May 29, 2019

In my manual testing it looks to me like this change with these 2 changes will get the arm32 buildbot to zero test failures
python/cpython#13454
python/cpython#13073

I will be away until Mon Jun 3, but I'd like to get this wrapped up soon if possible.

Thanks,
Paul

@zware @zooba

@zooba
Copy link
Member

zooba commented Jun 11, 2019

@zware Need you to approve this, but it looks okay to me.

@zware
Copy link
Member

zware commented Jun 13, 2019

Sorry for the delay. This was not the solution I'd have preferred for this issue, but I'm not going to have time to work that out and it's not fair to try to make you do it when I'm not even sure it would work :). This will work and will solve the issue, so we'll go with it.

@zware zware merged commit ed943fc into python:master Jun 13, 2019
@paulmon
Copy link
Contributor Author

paulmon commented Jun 13, 2019

Thanks! If you have suggestions for making it better I am interested in helping out.

@zware
Copy link
Member

zware commented Jun 13, 2019

What I think I'd like to see is separate builders for the 'build' and 'test' portions running on separate workers with the 'test' builders triggered by success on the 'build' builders, so that everything can just be native commands without dealing with SSH or special batch files. This does require buildbot-worker to actually run on an ARM host which could be a bit of a challenge, and I'm not sure how to transfer the built files from one worker to another (possibly packaged up installer-like, and also made available via the buildbot master's web server?), but it seems rather cleaner to me :)

@paulmon
Copy link
Contributor Author

paulmon commented Jun 19, 2019

That sounds like a great idea. I can't get to right now, but I'll keep it mind

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants