Chromium Code Reviews
Help | Chromium Project | Sign in
(17)

Issue 10696047: Pass android_sdk as a variable to gyp (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 11 months ago by Steve Block
Modified:
2 years, 10 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

Pass android_sdk as a variable to gyp This avoids the need for shell script in the gyp file. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=145195

Patch Set 1 #

Patch Set 2 : Fixed path #

Patch Set 3 : Struggling with escaping #

Patch Set 4 : Fix style #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -8 lines) Patch
M build/android/envsetup.sh View 1 2 3 1 chunk +2 lines, -0 lines 2 comments Download
M build/java_aidl.gypi View 1 chunk +0 lines, -8 lines 0 comments Download
Commit: CQ not working?

Messages

Total messages: 14 (0 generated)
Steve Block
I can resolve the 'ANDROID_BUILD_TOP' case when this merges downstream.
2 years, 11 months ago (2012-06-29 12:30:41 UTC) #1
navabi
lgtm but someone else must approve. http://codereview.chromium.org/10696047/diff/3003/build/android/envsetup.sh File build/android/envsetup.sh (right): http://codereview.chromium.org/10696047/diff/3003/build/android/envsetup.sh#newcode154 build/android/envsetup.sh:154: " android_sdk=${ANDROID_SDK_ROOT}/platforms/android-${ANDROID_SDK_VERSION}" This ...
2 years, 11 months ago (2012-06-29 20:29:52 UTC) #2
Steve Block
http://codereview.chromium.org/10696047/diff/3003/build/android/envsetup.sh File build/android/envsetup.sh (right): http://codereview.chromium.org/10696047/diff/3003/build/android/envsetup.sh#newcode154 build/android/envsetup.sh:154: " android_sdk=${ANDROID_SDK_ROOT}/platforms/android-${ANDROID_SDK_VERSION}" Right, I'll resolve the 'ANDROID_BUILD_TOP' case when ...
2 years, 10 months ago (2012-07-02 09:23:41 UTC) #3
navabi
lgtm
2 years, 10 months ago (2012-07-02 19:27:22 UTC) #4
commit-bot: I haz the power
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an ...
2 years, 10 months ago (2012-07-02 19:28:06 UTC) #5
joth
lgtm
2 years, 10 months ago (2012-07-02 19:58:06 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/steveblock@chromium.org/10696047/3003
2 years, 10 months ago (2012-07-02 19:59:00 UTC) #7
commit-bot: I haz the power
Change committed as 145195
2 years, 10 months ago (2012-07-02 21:26:04 UTC) #8
rwalker
Seems to break these instructions: http://code.google.com/p/chromium/wiki/AndroidBuildInstructions
2 years, 10 months ago (2012-07-02 22:37:17 UTC) #9
John Grabowski
LGTM Thanks for paying attention to the small things like this.
2 years, 10 months ago (2012-07-03 05:56:59 UTC) #10
Steve Block
> Seems to break these instructions: Can you be more specific as to the problem ...
2 years, 10 months ago (2012-07-03 09:53:16 UTC) #11
rwalker
On 2012/07/03 09:53:16, Steve Block wrote: > > Seems to break these instructions: > Can ...
2 years, 10 months ago (2012-07-03 16:39:58 UTC) #12
joth
chromium on Linux has the same boot strap issue ( http://code.google.com/p/chromium/wiki/UsingNewGit#Initial_checkout -- "Do this to ...
2 years, 10 months ago (2012-07-03 17:14:40 UTC) #13
Steve Block
2 years, 10 months ago (2012-07-04 15:19:33 UTC) #14
Got it, I see what you mean. In fact, this problem existed before my change,
only it didn't manifest itself as an error from gclient sync.

I've updated the documentation.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld aa3c3d4