r? @aneeshusa @edunham

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

buildbot/master/files/config/master.cfg, line 141 [r1] (raw file):

    """
    Builder which uses DynamicServoIntreeYAMLFactory to run steps from a 
    yaml file in the Servo source tree.

This comment is unclear to me -- I expect that the yaml builder would only parse the YAML file from the Servo repo then hand that config back to the master, yet the comment makes it sound like the yaml builder will be what then runs the loaded steps.

Comments from Reviewable

Since we're just testing the YAML loading, let's get rid of linux-rel-yaml and rename linux-yaml to linux-dev-yaml. Using the debug build will be faster and use less extra resources.

You'll also need to make a PR to servo/servo to add a linux-dev-yaml section in the etc/ci/buildbot_steps.yml file, which will need to land before this does.

DynamicServoIntreeYAMLFactory is a pretty long name - once we confirm this is working we should rename it to DynamicServoFactory to replace the current version in a follow-up, but we may want to use something shorter now as well.

I believe we need to create and invoke a Buildbot RemoteCommand(http://docs.buildbot.net/current/developer/cls-remotecommands.html) (or possibly RemoteShellCommand?). From my understanding, steps are run on the master, and use BuildStep.runCommand(https://docs.buildbot.net/current/developer/cls-buildsteps.html#buildbot.process.buildstep.BuildStep.runCommand) to actually run commands on the worker. (N.B.: RemoteCommand is just a Buildbot class representing a command to run on the worker, not a Buildbot Step, i.e. it's not a special type of ShellCommand. Same for RemoteShellCommand. This tripped me up a bunch.) Because the buildbot_steps.yml file is on the worker, we need to invoke runCommand to extract the contents of the file on the worker and send them back. I haven't looked into this 100%, but https://github.com/isotoma/buildbot_travis/blob/master/buildbot_travis/steps/base.py should be a decent example. I'll try to look into this more to provide more concrete advice.

Please also address the test suite failures from Travis.

@aneeshusa So in my StepsYAMLParsingStep::run(), I should run a RemoteCommand to get the content of the YAML file. (Something like cat /etc/ci/buildbot_steps.yml), then parse it and generate the dynamic steps?

@anneeshusa Also I have a question regarding @edunham 's comment. If I change the StepsYAMLParsingStep::run() to get the YAML back with RemoteCommand, and then I call the self.build.steps += ..., does that effectively make the DynamicServoYAMLBuilder (the old DynamicServoIntreeYAMLBuilder) run the parsed step? Or the self.build.steps is run by other Builders?

Or perhaps I should just return the parsed YAML content back to the master, and let the master start another builder with the parsed steps? But I don't see a clear place in the master.cfg to do this?

As far as I understand, you're correct that you'll need to runCommand a RemoteCommand (or maybe RemoteShellCommand), then parse the output and generate the dynamic steps. I highly recommend cloning the buildbot source tree, checking out the eight branch, and reading the Buildbot code to figure out exactly how this works - I'm not 100% sure.

DynamicServoYAMLBuilder should pick up on the new steps and continue to run them. Buildbot 9 (next major version) explicitly adds support for this behavior: docs here and code here.

I think the end result should look pretty close to the GenerateStagesCommand in the linked docs.

☔ The latest upstream changes (presumably #373) made this pull request unmergeable. Please resolve the merge conflicts.

@aneeshusa Thanks! Let me read that and get back to you.

@aneeshusa I ran the script throught REPL and didn't see any error, but I might need a full setup to varify it works.