Created attachment 43106 [details] Patch These 3 new build slaves obsolete the existing chromium slave, and build the webkit chromium port.
Created attachment 43109 [details] buildbot configuration changes
Comment on attachment 43109 [details] buildbot configuration changes > > +class InstallChromiumDependencies(shell.ShellCommand): > + name = "gclient" > + description = ["updating chromium dependencies"] > + descriptionDone = ["updated chromium dependencies"] > + command = ["perl", "./WebKitTools/Scripts/update-webkit-chromium"] this should be ["perl", "./WebKitTools/Scripts/update-webkit", "--chromium"] right?
Comment on attachment 43109 [details] buildbot configuration changes > diff --git a/WebKitTools/BuildSlaveSupport/build.webkit.org-config/config.json b/WebKitTools/BuildSlaveSupport/build.webkit.org-config/config.json > index 2b8faf6..04e7ad9 100644 > --- a/WebKitTools/BuildSlaveSupport/build.webkit.org-config/config.json > +++ b/WebKitTools/BuildSlaveSupport/build.webkit.org-config/config.json > @@ -25,7 +25,9 @@ > > { "name": "szeged-linux-1", "platform": "qt"}, > > - { "name": "google-slave-1", "platform": "chromium-win" } > + { "name": "chromium-win-slave-1", "platform": "chromium" }, > + { "name": "chromium-mac-slave-1", "platform": "chromium" }, > + { "name": "chromium-linux-slave-1", "platform": "chromium” } Per my recent email these aren’t the slave names that will be used. > ], > > "builders": [ { "name": "Tiger Intel Release", "type": "BuildAndTest", "builddir": "tiger-intel-release", > @@ -98,9 +100,22 @@ > "slavenames": ["szeged-linux-1"] > }, > { > - "name": "Chromium Win Release", "type": "ChromiumBuild", "builddir": "chromium-win-release", > - "platform": "chromium-win", "configuration": "release", "architectures": ["i386"], > - "slavenames": ["google-slave-1"] > + "name": "Chromium Win Release", "type": "Build", "builddir": "chromium-win-release", > + "platform": "chromium", "configuration": "release", "architectures": ["i386"], > + "triggers": ["chromium-win-release"], > + "slavenames": ["chromium-win-slave-1"] > + }, > + { > + "name": "Chromium Mac Release", "type": "Build", "builddir": "chromium-mac-release", > + "platform": "chromium", "configuration": "release", "architectures": ["i386"], > + "triggers": ["chromium-mac-release"], > + "slavenames": ["chromium-mac-slave-1"] > + }, > + { > + "name": "Chromium Linux Release", "type": "Build", "builddir": "chromium-linux-release", > + "platform": "chromium", "configuration": "release", "architectures": ["i386"], > + "triggers": ["chromium-linux-release"], > + "slavenames": ["chromium-linux-slave-1"] > } > ], The “triggers” values here seem bogus given that they don’t attempt to trigger anything and there’s no corresponding triggerable configuration. > +class InstallChromiumDependencies(shell.ShellCommand): > + name = "gclient" > + description = ["updating chromium dependencies"] > + descriptionDone = ["updated chromium dependencies"] > + command = ["perl", "./WebKitTools/Scripts/update-webkit-chromium"] > + haltOnFailure = True This script doesn’t exist. Is that the right command to run? > def appendCustomBuildFlags(step, platform): > if platform in ('gtk', 'wx', 'qt'): > step.setCommand(step.command + ['--' + platform]) > @@ -119,6 +78,8 @@ class CompileWebKit(shell.Compile): > def start(self): > platform = self.getProperty('platform') > buildOnly = self.getProperty('buildOnly') > + if platform == 'chromium': > + self.setCommand(self.command + ['--chromium']) > if platform == 'mac' and buildOnly: > self.setCommand(self.command + ['DEBUG_INFORMATION_FORMAT=dwarf-with-dsym’]) This should be handled in the same way it is for other platforms: in appendCustomBuildFlags.
Created attachment 43115 [details] buildbot configuration changes
Thanks for reviewing this. New patch is attached. See comments below: > (From update of attachment 43109 [details]) > > diff --git a/WebKitTools/BuildSlaveSupport/build.webkit.org-config/config.json b/WebKitTools/BuildSlaveSupport/build.webkit.org-config/config.json > > index 2b8faf6..04e7ad9 100644 > > --- a/WebKitTools/BuildSlaveSupport/build.webkit.org-config/config.json > > +++ b/WebKitTools/BuildSlaveSupport/build.webkit.org-config/config.json > > @@ -25,7 +25,9 @@ > > > > { "name": "szeged-linux-1", "platform": "qt"}, > > > > - { "name": "google-slave-1", "platform": "chromium-win" } > > + { "name": "chromium-win-slave-1", "platform": "chromium" }, > > + { "name": "chromium-mac-slave-1", "platform": "chromium" }, > > + { "name": "chromium-linux-slave-1", "platform": "chromium” } > > Per my recent email these aren’t the slave names that will be used. > Fixed. > > > ], > > > > "builders": [ { "name": "Tiger Intel Release", "type": "BuildAndTest", "builddir": "tiger-intel-release", > > @@ -98,9 +100,22 @@ > > "slavenames": ["szeged-linux-1"] > > }, > > { > > - "name": "Chromium Win Release", "type": "ChromiumBuild", "builddir": "chromium-win-release", > > - "platform": "chromium-win", "configuration": "release", "architectures": ["i386"], > > - "slavenames": ["google-slave-1"] > > + "name": "Chromium Win Release", "type": "Build", "builddir": "chromium-win-release", > > + "platform": "chromium", "configuration": "release", "architectures": ["i386"], > > + "triggers": ["chromium-win-release"], > > + "slavenames": ["chromium-win-slave-1"] > > + }, > > + { > > + "name": "Chromium Mac Release", "type": "Build", "builddir": "chromium-mac-release", > > + "platform": "chromium", "configuration": "release", "architectures": ["i386"], > > + "triggers": ["chromium-mac-release"], > > + "slavenames": ["chromium-mac-slave-1"] > > + }, > > + { > > + "name": "Chromium Linux Release", "type": "Build", "builddir": "chromium-linux-release", > > + "platform": "chromium", "configuration": "release", "architectures": ["i386"], > > + "triggers": ["chromium-linux-release"], > > + "slavenames": ["chromium-linux-slave-1"] > > } > > ], > > The “triggers” values here seem bogus given that they don’t attempt to trigger > anything and there’s no corresponding triggerable configuration. > I know, but BuildFactory require triggers, so I had to put something there. Once we also have testing harness, we will connect them to the triggers. > > +class InstallChromiumDependencies(shell.ShellCommand): > > + name = "gclient" > > + description = ["updating chromium dependencies"] > > + descriptionDone = ["updated chromium dependencies"] > > + command = ["perl", "./WebKitTools/Scripts/update-webkit-chromium"] > > + haltOnFailure = True > > This script doesn’t exist. Is that the right command to run? Actually, update-webkit-chromium does exist and is reused by "update-webkit --chromium" too. > > > def appendCustomBuildFlags(step, platform): > > if platform in ('gtk', 'wx', 'qt'): > > step.setCommand(step.command + ['--' + platform]) > > @@ -119,6 +78,8 @@ class CompileWebKit(shell.Compile): > > def start(self): > > platform = self.getProperty('platform') > > buildOnly = self.getProperty('buildOnly') > > + if platform == 'chromium': > > + self.setCommand(self.command + ['--chromium']) > > if platform == 'mac' and buildOnly: > > self.setCommand(self.command + ['DEBUG_INFORMATION_FORMAT=dwarf-with-dsym’]) > > This should be handled in the same way it is for other platforms: in > appendCustomBuildFlags. Done.
(In reply to comment #5) > Thanks for reviewing this. New patch is attached. See comments below: > > > The “triggers” values here seem bogus given that they don’t attempt to trigger > > anything and there’s no corresponding triggerable configuration. > > > I know, but BuildFactory require triggers, so I had to put something there. > Once we also have testing harness, we will connect them to the triggers. A better fix would be to make the triggers optional. This would convert some Chromium-specific logic in master.cfg in to logic that is specific to whether or not BuildFactory has anything to trigger. > > > > +class InstallChromiumDependencies(shell.ShellCommand): > > > + name = "gclient" > > > + description = ["updating chromium dependencies"] > > > + descriptionDone = ["updated chromium dependencies"] > > > + command = ["perl", "./WebKitTools/Scripts/update-webkit-chromium"] > > > + haltOnFailure = True > > > > This script doesn’t exist. Is that the right command to run? > > Actually, update-webkit-chromium does exist and is reused by "update-webkit > --chromium" too. My mistake.
And at least one of the slave names doesn’t match the ones from my email.
Comment on attachment 43115 [details] buildbot configuration changes Marking as r- per last two comments. There’s no point marking this as commit-queue? since it needs to be manually landed anyway.
Created attachment 43118 [details] buildbot configuration changes
Created attachment 43120 [details] buildbot configuration changes
Comment on attachment 43120 [details] buildbot configuration changes Fixed the win slave names and made triggers optional.
Comment on attachment 43120 [details] buildbot configuration changes > class BuildFactory(Factory): > - def __init__(self, platform, configuration, architectures, triggers): > + def __init__(self, platform, configuration, architectures, triggers=None): > Factory.__init__(self, platform, configuration, architectures, True) > self.addStep(CompileWebKit) > + if platform == "chromium": > + # FIXME: Chromium should archive and trigger tests too. > + return > self.addStep(ArchiveBuiltProduct) > self.addStep(UploadBuiltProduct) > - self.addStep(trigger.Trigger, schedulerNames=triggers) > + if triggers: > + self.addStep(trigger.Trigger, schedulerNames=triggers) Since there’s no point in archiving or uploading the built product unless you’re going to do something that uses it, this would make more sense if it were expressed as: self.addStep(CompileWebKit) if triggers: self.addStep(ArchiveBuiltProduct) self.addStep(UploadBuiltProduct) self.addStep(trigger.Trigger, schedulerNames=triggers) It also removes logic that special-cases Chromium. I’m going to say r=me but this should be fixed before it is landed.
Created attachment 43464 [details] buildbot configuration changes
Comment on attachment 43464 [details] buildbot configuration changes r=me based on Mark's earlier review and comment
Thank you!
Comment on attachment 43464 [details] buildbot configuration changes Yaar is not a committer quite yet, so marking cq+. I beleive Mark Rowe will have to push the config to the master manually once this is in. You can work that out with him. :)
Comment on attachment 43464 [details] buildbot configuration changes Clearing flags on attachment: 43464 Committed r51167: <http://trac.webkit.org/changeset/51167>
All reviewed patches have been landed. Closing bug.