RESOLVED FIXED 92931
[Chrome-Android] - Prepare apk tests for switch to checked in SDK.
https://bugs.webkit.org/show_bug.cgi?id=92931
Summary [Chrome-Android] - Prepare apk tests for switch to checked in SDK.
Yaron Friedman
Reported 2012-08-01 18:48:16 PDT
[Chrome-Android] - Prepare apk tests for switch to checked in SDK.
Attachments
Patch (5.00 KB, patch)
2012-08-01 18:49 PDT, Yaron Friedman
no flags
Patch (5.04 KB, patch)
2012-08-02 08:19 PDT, Yaron Friedman
no flags
Patch (5.05 KB, patch)
2012-08-02 08:26 PDT, Yaron Friedman
no flags
Yaron Friedman
Comment 1 2012-08-01 18:49:15 PDT
Yaron Friedman
Comment 2 2012-08-01 18:50:28 PDT
These are backwards compatible but add a few variable definitions so that ant builds will work after http://codereview.chromium.org/10830012/. I've tested inside my chromium tree before and after the change.
Yaron Friedman
Comment 3 2012-08-01 18:52:34 PDT
As an aside: it seems like these rules would benefit from using build/apk_test.gypi as it would save a bunch of boilerplate. I'm guessing they aren't typically used in webkit?
Peter Beverloo
Comment 4 2012-08-02 06:24:01 PDT
Comment on attachment 155946 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=155946&action=review While this SGTM after the ChangeLog updates, it'll cause a build failure right now because the android_sdk_* variables are not yet available. You'll need to split your patch in three parts.. First land the envsetup_function.sh changes which introduce the variables, then roll Chromium into WebKit, then update WebKit with these changes, then roll WebKit into Chromium, then land the rest of your patch. Lovely, innit? If you land the variables on the Chromium side and update your patch here, I'll make sure that anything up to rolling WebKit into Chromium is done before you come in tomorrow. You'll be able to pick it up after that. As an aside, I think it's fine for WebKit to use gypi files in build/ (after all, we already use common.gypi too), but we can do that in a later patch, as this blocks your's on the Chromium side. > Source/WebKit/chromium/ChangeLog:8 > + Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!). Please add a brief description of what's changing. Something among the lines of "Pass Android-specific gyp variables to the native test generator, avoiding any dependencies on environment variables during build time." would be perfect. > Tools/ChangeLog:8 > + Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!). dito.
Yaron Friedman
Comment 5 2012-08-02 08:19:42 PDT
WebKit Review Bot
Comment 6 2012-08-02 08:23:01 PDT
Attachment 156090 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1 Source/WebKit/chromium/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] Tools/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] Total errors found: 2 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yaron Friedman
Comment 7 2012-08-02 08:26:56 PDT
Yaron Friedman
Comment 8 2012-08-03 09:18:27 PDT
Ok. Looks like chromium LKGR is passed my revision. Can we roll chromium into webkit now? Do I just join #webkit and ask sheriffbot (http://www.chromium.org/developers/how-tos/webkit-gardening)
Adam Barth
Comment 9 2012-08-03 10:17:56 PDT
> Can we roll chromium into webkit now? Yes. > Do I just join #webkit and ask sheriffbot (http://www.chromium.org/developers/how-tos/webkit-gardening) Yep! You say something like: sheriffbot: roll-chromium-deps NNNNN
Yaron Friedman
Comment 10 2012-08-03 16:12:12 PDT
Ok, roll is in. Can we land this guy?
Adam Barth
Comment 11 2012-08-03 16:15:37 PDT
Comment on attachment 156092 [details] Patch Fine with me as long as Peter says it's ok.
WebKit Review Bot
Comment 12 2012-08-03 17:31:48 PDT
Comment on attachment 156092 [details] Patch Clearing flags on attachment: 156092 Committed r124676: <http://trac.webkit.org/changeset/124676>
WebKit Review Bot
Comment 13 2012-08-03 17:31:51 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.