Bug 67035 – Modify the build bot master's script to pass --chromium-android where appropriate

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Peter Beverloo]

Created attachment 105356 [details]
Patch

In order to invoke the script changes from bug 67034, the build bot master has to be able to pass the --chromium-android argument for the update-webkit-chromium and build-webkit scripts.

[Adam Barth]

Comment on attachment 105356 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=105356&action=review

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:91
> +def appendCustomBuildFlags(step, platform, fullPlatform=""):

I see.  The issue here is that we're cross-compiling.  Maybe rather than "fullPlatform" we should say target_os ?

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:92
> +    if fullPlatform == "chromium-android":

This this becomes platform == "chromium" && target_os == "android".

That will generalize better if we want to support other sorts of cross compiling (e.g., building on Linux but targeting Mac).

[Adam Barth]

Comment on attachment 105356 [details]
Patch

3434        self.platform = platform.split('-', 1)[0]
3535        self.fullPlatform = platform

^^ maybe put self.target_os = platform.split('-', 2)[1] ?

You can also leave this as fullPlatform if you think that's better.

[Peter Beverloo]

There are naming issues with generalizing it to a target OS.  Various other ports (e.g. win, qt and efl) don't use such a suffix, so we'd have to check for the presence of a dash first, as an IndexError would occur otherwise.  Furthermore, Chromium's Mac bots use "chromium-cg-mac" as their platform, meaning that the target OS would be "cg".

I agree that generalizing the approach would be desirable, but don't really see a way to do that without either loosing chromium-cg-mac's history by renaming it, or rather, adding an exception for their naming scheme and specifying future platform strings to be [port]-[target_os].

[Adam Barth]

Yeah, you're right that we're not consistent enough to make this work well.

[Peter Beverloo]

Comment on attachment 105356 [details]
Patch

Ok. I'm marking this as cq? then, thanks.

[WebKit Review Bot]

Comment on attachment 105356 [details]
Patch

Clearing flags on attachment: 105356

Committed r93942: <http://trac.webkit.org/changeset/93942>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.