Updates to the Android.mk writer.

Updates to the Android.mk writer.

Keeping up with changes made to the Android.mk file that is
currently checked in.

Committed: http://code.google.com/p/skia/source/detail?r=13626

[scroggo, 2014-02-27 22:53:01]

https://codereview.chromium.org/183953002/diff/1/platform_tools/android/gyp_g...
File platform_tools/android/gyp_gen/makefile_writer.py (right):

https://codereview.chromium.org/183953002/diff/1/platform_tools/android/gyp_g...
platform_tools/android/gyp_gen/makefile_writer.py:149: f.write('LOCAL_CFLAGS +=
-DANDROID_LARGE_MEMORY_DEVICE\n')
Derek, I think you said it was okay to just define this all the time. This makes
things much simpler for my generator, but let me know if I'm mistaken and we
still need to support older small memory devices. (The only thing that reads
this define is in our decoder wrapper for libjpeg, FWIW.)

[scroggo, 2014-02-28 12:49:10]

https://codereview.chromium.org/183953002/diff/1/platform_tools/android/gyp_g...
File platform_tools/android/gyp_gen/makefile_writer.py (right):

https://codereview.chromium.org/183953002/diff/1/platform_tools/android/gyp_g...
platform_tools/android/gyp_gen/makefile_writer.py:149: f.write('LOCAL_CFLAGS +=
-DANDROID_LARGE_MEMORY_DEVICE\n')
On 2014/02/27 22:53:01, scroggo wrote:
> Derek, I think you said it was okay to just define this all the time. This
makes
> things much simpler for my generator, but let me know if I'm mistaken and we
> still need to support older small memory devices. (The only thing that reads
> this define is in our decoder wrapper for libjpeg, FWIW.)

Actually, it's not so hard to define the flag for all platforms except arm
without the VFP. New patch up.

[djsollen, 2014-02-28 13:58:59]

I'm good with the change but I'll defer on the python correctness.

[epoger, 2014-02-28 15:56:43]

Python looks pretty decent, a few comments/suggestions below.

https://codereview.chromium.org/183953002/diff/40001/platform_tools/android/b...
File platform_tools/android/bin/gyp_to_android.py (right):

https://codereview.chromium.org/183953002/diff/40001/platform_tools/android/b...
platform_tools/android/bin/gyp_to_android.py:121: li = list()
Please use a more descriptive variable name.  Maybe deviations_from_common ?

Much less importantly, I think it's more common to initialize the empty list
like so:
li = []

https://codereview.chromium.org/183953002/diff/40001/platform_tools/android/g...
File platform_tools/android/gyp_gen/makefile_writer.py (right):

https://codereview.chromium.org/183953002/diff/40001/platform_tools/android/g...
platform_tools/android/gyp_gen/makefile_writer.py:128: @param the_rest List of
VarsDictData, one for each possible configuration.
Again, I think a name like deviations, deviations_from_common,
additions_to_common, etc. might be more descriptive.

https://codereview.chromium.org/183953002/diff/40001/platform_tools/android/g...
platform_tools/android/gyp_gen/makefile_writer.py:131: be written to the
makefile as a condition to determine
I don't think it's a big deal, but
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Comments calls
for a different docstring format.

Something to think about for future changes, perhaps.

https://codereview.chromium.org/183953002/diff/40001/platform_tools/android/g...
platform_tools/android/gyp_gen/makefile_writer.py:170: if data.condition:
Please consider adding unittests at some point.  This code is getting reasonably
complicated.

[scroggo, 2014-02-28 16:16:57]

https://codereview.chromium.org/183953002/diff/40001/platform_tools/android/b...
File platform_tools/android/bin/gyp_to_android.py (right):

https://codereview.chromium.org/183953002/diff/40001/platform_tools/android/b...
platform_tools/android/bin/gyp_to_android.py:121: li = list()
On 2014/02/28 15:56:43, epoger wrote:
> Please use a more descriptive variable name.  Maybe deviations_from_common ?
> 
> Much less importantly, I think it's more common to initialize the empty list
> like so:
> li = []
> 

Done.

https://codereview.chromium.org/183953002/diff/40001/platform_tools/android/g...
File platform_tools/android/gyp_gen/makefile_writer.py (right):

https://codereview.chromium.org/183953002/diff/40001/platform_tools/android/g...
platform_tools/android/gyp_gen/makefile_writer.py:128: @param the_rest List of
VarsDictData, one for each possible configuration.
On 2014/02/28 15:56:43, epoger wrote:
> Again, I think a name like deviations, deviations_from_common,
> additions_to_common, etc. might be more descriptive.

Done.

https://codereview.chromium.org/183953002/diff/40001/platform_tools/android/g...
platform_tools/android/gyp_gen/makefile_writer.py:131: be written to the
makefile as a condition to determine
On 2014/02/28 15:56:43, epoger wrote:
> I don't think it's a big deal, but
> http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Comments calls
> for a different docstring format.
> 
> Something to think about for future changes, perhaps.

For now, I'd like to stick with the surrounding code, if that's okay. I'll clean
it up in some future CL.

https://codereview.chromium.org/183953002/diff/40001/platform_tools/android/g...
platform_tools/android/gyp_gen/makefile_writer.py:170: if data.condition:
On 2014/02/28 15:56:43, epoger wrote:
> Please consider adding unittests at some point.  This code is getting
reasonably
> complicated.

Will do. Added a TODO.

[epoger, 2014-02-28 16:19:19]

lgtm

[scroggo, 2014-02-28 16:24:56]

The CQ bit was checked by scroggo@google.com

[commit-bot: I haz the power, 2014-02-28 16:25:00]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/scroggo@google.com/183953002/80001

[commit-bot: I haz the power, 2014-02-28 17:24:35]

Change committed as 13626