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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years ago by Steve Block
Modified:
2 years, 12 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.
3 years 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 ...
3 years 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 ...
3 years ago (2012-07-02 09:23:41 UTC) #3
navabi
lgtm
3 years 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 ...
3 years ago (2012-07-02 19:28:06 UTC) #5
joth
lgtm
3 years 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
3 years ago (2012-07-02 19:59:00 UTC) #7
commit-bot: I haz the power
Change committed as 145195
3 years ago (2012-07-02 21:26:04 UTC) #8
rwalker
Seems to break these instructions: http://code.google.com/p/chromium/wiki/AndroidBuildInstructions
3 years ago (2012-07-02 22:37:17 UTC) #9
John Grabowski
LGTM Thanks for paying attention to the small things like this.
3 years 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 ...
3 years 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 ...
3 years 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 ...
3 years ago (2012-07-03 17:14:40 UTC) #13
Steve Block
2 years, 12 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 1f9106d