Bug 67035

Summary: Modify the build bot master's script to pass --chromium-android where appropriate
Product: WebKit Reporter: Peter Beverloo <peter>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aroben, steveblock, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 67032    
Attachments:
Description Flags
Patch none

Peter Beverloo
Reported 2011-08-26 06:17:46 PDT
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.
Attachments
Patch (2.93 KB, patch)
2011-08-26 06:17 PDT, Peter Beverloo 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Adam Barth
Comment 1 2011-08-26 12:17:44 PDT
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 2 2011-08-26 12:21:37 PDT
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
Comment 3 2011-08-27 10:16:13 PDT
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
Comment 4 2011-08-27 10:54:17 PDT
Yeah, you're right that we're not consistent enough to make this work well.
Peter Beverloo
Comment 5 2011-08-27 11:02:40 PDT
Comment on attachment 105356 [details] Patch Ok. I'm marking this as cq? then, thanks.
WebKit Review Bot
Comment 6 2011-08-27 12:05:27 PDT
Comment on attachment 105356 [details] Patch Clearing flags on attachment: 105356 Committed r93942: <http://trac.webkit.org/changeset/93942>
WebKit Review Bot
Comment 7 2011-08-27 12:05:31 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.