Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(155)

Issue 10830012: [Android] Add chrome_java target for building Java code in the chromium layer. (Closed)

Created:
8 years, 5 months ago by Yaron
Modified:
8 years, 4 months ago
Reviewers:
shashi, f(malita), jam, Torne, nilesh
CC:
chromium-reviews, cbentzel+watch_chromium.org, erikwright (departed), jam, joi+watch-content_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, brettw-cc_chromium.org, shashi, cjhopman, Satish, aurimas (slooooooooow)
Base URL:
http://git.chromium.org/chromium/src.git@reland
Visibility:
Public.

Description

[Android] Add chrome_java target for building Java code in the chromium layer. Also includes a refactoring of the Ant xml scripts to use a common template (contributed by shashishekhar@chromium.org). As part of this, I also continued Torne's effort of removing our reliance on environment variables. Unfortunately this currently means that you have to specify 5 gyp flags: ANDROID_SDK, ANDROID_SDK_ROOT, ANDROID_SDK_TOOLS, ANDROID_SDK_VERSION, ANDROID_TOOLCHAIN. This'll get better as we make further use of the checked in sdk both upstream and downstream. The problem stems from the android tree and released sdk having different configurations. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=150172

Patch Set 1 #

Total comments: 22

Patch Set 2 : formatting #

Total comments: 2

Patch Set 3 : nilesh's nits #

Patch Set 4 : Rebase after 10821105, remove env dependencies. #

Patch Set 5 : rebase #

Patch Set 6 : #

Patch Set 7 : submodules :S #

Total comments: 1

Patch Set 8 : rebase #

Patch Set 9 : fix java build error #

Unified diffs Side-by-side diffs Delta from patch set Stats (+391 lines, -216 lines) Patch
M base/android/java/base.xml View 1 2 3 1 chunk +9 lines, -46 lines 0 comments Download
A build/android/ant/chromium-jars.xml View 1 1 chunk +64 lines, -0 lines 0 comments Download
A build/android/ant/common.xml View 1 2 3 1 chunk +78 lines, -0 lines 0 comments Download
A build/android/ant/sdk-targets.xml View 1 1 chunk +97 lines, -0 lines 0 comments Download
M build/apk_test.gypi View 1 2 3 4 5 6 1 chunk +12 lines, -1 line 0 comments Download
M build/java.gypi View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
A chrome/android/java/chrome.xml View 1 2 3 1 chunk +19 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/IntentHelper.java View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome.gyp View 1 2 3 4 5 6 1 chunk +19 lines, -0 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M content/content_shell.gypi View 1 2 3 4 5 6 1 chunk +5 lines, -0 lines 0 comments Download
M content/content_tests.gypi View 1 2 3 4 5 6 7 1 chunk +5 lines, -0 lines 0 comments Download
M content/public/android/java/content.xml View 1 2 3 1 chunk +12 lines, -47 lines 0 comments Download
M content/shell/android/java/content_shell_apk.xml View 1 4 chunks +20 lines, -19 lines 0 comments Download
M content/shell/android/javatests/content_shell_test_apk.xml View 1 2 3 2 chunks +6 lines, -5 lines 0 comments Download
M media/base/android/java/media.xml View 1 2 3 1 chunk +9 lines, -43 lines 0 comments Download
M net/android/java/net.xml View 1 2 3 1 chunk +10 lines, -39 lines 0 comments Download
M testing/android/generate_native_test.py View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M testing/android/native_test.gyp View 1 2 3 1 chunk +5 lines, -0 lines 0 comments Download
M testing/android/native_test_apk.xml View 1 2 3 1 chunk +13 lines, -13 lines 0 comments Download

Messages

Total messages: 28 (0 generated)
Yaron
8 years, 5 months ago (2012-07-25 15:55:05 UTC) #1
Yaron
http://codereview.chromium.org/10830012/diff/1/chrome/chrome_tests.gypi File chrome/chrome_tests.gypi (right): http://codereview.chromium.org/10830012/diff/1/chrome/chrome_tests.gypi#newcode4695 chrome/chrome_tests.gypi:4695: '<(PRODUCT_DIR)/lib.java/chromium_base.jar', I didn't actually include the jar here. I ...
8 years, 5 months ago (2012-07-25 15:59:29 UTC) #2
nilesh
Sorry for nitpicking :) But I see this as an opportunity to get our xml ...
8 years, 5 months ago (2012-07-25 16:57:12 UTC) #3
nilesh
http://codereview.chromium.org/10830012/diff/1/build/android/ant/chromium-jars.xml File build/android/ant/chromium-jars.xml (right): http://codereview.chromium.org/10830012/diff/1/build/android/ant/chromium-jars.xml#newcode35 build/android/ant/chromium-jars.xml:35: description="Compiles source." > looks like description will fit on ...
8 years, 5 months ago (2012-07-25 16:57:21 UTC) #4
shashi
Ahh.. I realize now so many white space errors I made during the refactoring.
8 years, 5 months ago (2012-07-25 17:17:11 UTC) #5
Yaron
ptal http://codereview.chromium.org/10830012/diff/1/build/android/ant/chromium-jars.xml File build/android/ant/chromium-jars.xml (right): http://codereview.chromium.org/10830012/diff/1/build/android/ant/chromium-jars.xml#newcode35 build/android/ant/chromium-jars.xml:35: description="Compiles source." > On 2012/07/25 16:57:22, nileshagrawal1 wrote: ...
8 years, 5 months ago (2012-07-25 18:12:23 UTC) #6
nilesh
LGTM. Thanks for fixing everything. A couple of minor nits. http://codereview.chromium.org/10830012/diff/9001/chrome/chrome.gyp File chrome/chrome.gyp (right): http://codereview.chromium.org/10830012/diff/9001/chrome/chrome.gyp#newcode1074 ...
8 years, 5 months ago (2012-07-25 18:19:39 UTC) #7
Yaron
jrg: need OWNERS stamp (apparently you didn't add me to a few owners files)
8 years, 5 months ago (2012-07-25 18:34:33 UTC) #8
Yaron
hmm. the new configs require sdk tools version 20 so this might have to wait ...
8 years, 5 months ago (2012-07-25 18:59:16 UTC) #9
John Grabowski
On 2012/07/25 18:59:16, Yaron wrote: > hmm. the new configs require sdk tools version 20 ...
8 years, 5 months ago (2012-07-25 21:50:13 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yfriedman@chromium.org/10830012/8004
8 years, 4 months ago (2012-07-31 23:16:47 UTC) #11
commit-bot: I haz the power
Presubmit check for 10830012-8004 failed and returned exit status 1. Running presubmit commit checks ...
8 years, 4 months ago (2012-07-31 23:16:53 UTC) #12
Yaron
ben: can you rubberstamp for chrome/OWNERS
8 years, 4 months ago (2012-07-31 23:21:11 UTC) #13
Yaron
+Torne as this got more interesting due to http://codereview.chromium.org/10821105/. Ok, ptal Nilesh/Torne for correctness. Unfortunately, ...
8 years, 4 months ago (2012-08-01 23:44:54 UTC) #14
nilesh
LGTM http://codereview.chromium.org/10830012/diff/9027/testing/android/native_test_apk.xml File testing/android/native_test_apk.xml (right): http://codereview.chromium.org/10830012/diff/9027/testing/android/native_test_apk.xml#newcode15 testing/android/native_test_apk.xml:15: TODO(yfriedman): This target can be invoked from multiple ...
8 years, 4 months ago (2012-08-02 00:13:12 UTC) #15
shashi
lgtm
8 years, 4 months ago (2012-08-02 01:46:25 UTC) #16
Yaron
On 2012/08/02 01:46:25, shashi wrote: > lgtm The try failure is addressed by having https://bugs.webkit.org/show_bug.cgi?id=92931 ...
8 years, 4 months ago (2012-08-02 01:56:56 UTC) #17
Yaron
On 2012/08/02 01:56:56, Yaron wrote: > On 2012/08/02 01:46:25, shashi wrote: > > lgtm > ...
8 years, 4 months ago (2012-08-06 16:11:20 UTC) #18
jam
lgtm
8 years, 4 months ago (2012-08-06 19:03:15 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yfriedman@chromium.org/10830012/10005
8 years, 4 months ago (2012-08-06 20:13:25 UTC) #20
commit-bot: I haz the power
Try job failure for 10830012-10005 (retry) on linux_chromeos for step "compile" (clobber build). It's a ...
8 years, 4 months ago (2012-08-06 20:44:30 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yfriedman@chromium.org/10830012/10005
8 years, 4 months ago (2012-08-06 20:45:25 UTC) #22
commit-bot: I haz the power
Try job failure for 10830012-10005 on win_rel for step "update". http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=50114 Step "update" is always ...
8 years, 4 months ago (2012-08-06 21:58:54 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yfriedman@chromium.org/10830012/10005
8 years, 4 months ago (2012-08-06 22:09:47 UTC) #24
commit-bot: I haz the power
Try job failure for 10830012-10005 (retry) on linux_chromeos for step "compile" (clobber build). It's a ...
8 years, 4 months ago (2012-08-06 22:52:10 UTC) #25
f(malita)
This seems to be causing Android build failures after rolling the CR deps: http://build.webkit.org/builders/Chromium%20Android%20Release/builds/23333/steps/compile-webkit/logs/stdio BUILD ...
8 years, 4 months ago (2012-08-11 14:51:18 UTC) #26
Yaron
On 2012/08/11 14:51:18, Florin Malita wrote: > This seems to be causing Android build failures ...
8 years, 4 months ago (2012-08-11 18:18:18 UTC) #27
Yaron
8 years, 4 months ago (2012-08-11 18:21:09 UTC) #28
On 2012/08/11 18:18:18, Yaron wrote:
> On 2012/08/11 14:51:18, Florin Malita wrote:
> > This seems to be causing Android build failures after rolling the CR deps:
> > 
> >
>
http://build.webkit.org/builders/Chromium%252520Android%252520Release/builds/...
> > 
> > 
> > BUILD FAILED
> >
>
/mnt/data/b/WebKit-BuildSlave/chromium-linux-android/build/out/Release/DumpRenderTree_apk/native_test_apk.xml:17:
> > Cannot find
> >
>
/mnt/data/b/WebKit-BuildSlave/chromium-linux-android/build/build/android/ant/common.xml
> > imported from
> >
>
/mnt/data/b/WebKit-BuildSlave/chromium-linux-android/build/out/Release/DumpRenderTree_apk/native_test_apk.xml
> 
> Hrmm. That's puzzling as the files are clearly in this CL and the path listed
> should exist. However, there were other issues with the roll about the path
not
> being set correctly. Maybe there's a common problem (see #chromium from last
> night)

Looks like Peter already has a proposed fix:
https://chromiumcodereview.appspot.com/10829291/

Powered by Google App Engine
This is Rietveld 408576698