WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(5.04 KB, patch)
2012-08-02 08:19 PDT
,
Yaron Friedman
no flags
Details
Formatted Diff
Diff
Patch
(5.05 KB, patch)
2012-08-02 08:26 PDT
,
Yaron Friedman
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Yaron Friedman
Comment 1
2012-08-01 18:49:15 PDT
Created
attachment 155946
[details]
Patch
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
Created
attachment 156090
[details]
Patch
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
Created
attachment 156092
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug