[webkit-reviews] review denied: [Bug 31442] Chromium Build Slaves : [Attachment 43109] buildbot configuration changes

[bugzilla-daemon at webkit.org]

Mark Rowe (bdash) <mrowe at apple.com> has denied Yaar Schnitman
<yaar at chromium.org>'s request for review:
Bug 31442: Chromium Build Slaves
https://bugs.webkit.org/show_bug.cgi?id=31442

Attachment 43109: buildbot configuration changes
https://bugs.webkit.org/attachment.cgi?id=43109&action=review

------- Additional Comments from Mark Rowe (bdash) <mrowe at apple.com>
> 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.