Update Android framework makefile to build static and shared libs

Update Android framework makefile to build static and shared libs

Also use static lib for testing tools and expose includes needed for
other framework testing tools to statically link in Skia.
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1696483002

Committed: https://skia.googlesource.com/skia/+/2a8fcb4647115ce5e9b6127a716b6f50b2a9fd55

[djsollen, 2016-02-11 21:24:26]

Description was changed from

==========
Update Android framework makefile to build static and shared libs

Also use static lib for testing tools and expose includes needed for
other framework testing tools to statically link in Skia.
==========

to

==========
Update Android framework makefile to build static and shared libs

Also use static lib for testing tools and expose includes needed for
other framework testing tools to statically link in Skia.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[djsollen, 2016-02-11 21:25:03]

djsollen@google.com changed reviewers:
+ scroggo@google.com

[scroggo, 2016-02-11 21:35:33]

I think you'll need to update some of the test expectations.

Run

python platform_tools/android/tests/run_all.py

If something fails, you'll need to rerun the test with a parameter like
--rebaseline.

https://codereview.chromium.org/1696483002/diff/1/platform_tools/android/gyp_...
File platform_tools/android/gyp_gen/makefile_writer.py (left):

https://codereview.chromium.org/1696483002/diff/1/platform_tools/android/gyp_...
platform_tools/android/gyp_gen/makefile_writer.py:147: def write_local_path(f):
Why did you inline this?

https://codereview.chromium.org/1696483002/diff/1/platform_tools/android/gyp_...
platform_tools/android/gyp_gen/makefile_writer.py:155: def write_clear_vars(f):
Ditto.

[djsollen, 2016-02-12 13:36:38]

https://codereview.chromium.org/1696483002/diff/1/platform_tools/android/gyp_...
File platform_tools/android/gyp_gen/makefile_writer.py (left):

https://codereview.chromium.org/1696483002/diff/1/platform_tools/android/gyp_...
platform_tools/android/gyp_gen/makefile_writer.py:147: def write_local_path(f):
On 2016/02/11 21:35:33, scroggo wrote:
> Why did you inline this?

In both cases reading through the code I had to go from one file to another to
see what this function was doing and make sure it was only issuing the one line
call.

I didn't see the point for the function since it doesn't do any interesting
work, is only used 2 places, and the variable names it abstracts away haven't
changed since the beginning of time.

[scroggo, 2016-02-12 15:00:31]

https://codereview.chromium.org/1696483002/diff/1/platform_tools/android/gyp_...
File platform_tools/android/gyp_gen/makefile_writer.py (left):

https://codereview.chromium.org/1696483002/diff/1/platform_tools/android/gyp_...
platform_tools/android/gyp_gen/makefile_writer.py:147: def write_local_path(f):
On 2016/02/12 13:36:38, djsollen wrote:
> On 2016/02/11 21:35:33, scroggo wrote:
> > Why did you inline this?
> 
> In both cases reading through the code I had to go from one file to another to
> see what this function was doing and make sure it was only issuing the one
line
> call.
> 
> I didn't see the point for the function since it doesn't do any interesting
> work, is only used 2 places, and the variable names it abstracts away haven't
> changed since the beginning of time.

The point is that the interpreter will catch you if you write it incorrectly in
a new place.

https://codereview.chromium.org/1696483002/diff/20001/gyp/bench.gyp
File gyp/bench.gyp (left):

https://codereview.chromium.org/1696483002/diff/20001/gyp/bench.gyp#oldcode35
gyp/bench.gyp:35: '-landroid',
So these libraries are no longer needed?

https://codereview.chromium.org/1696483002/diff/20001/platform_tools/android/...
File platform_tools/android/gyp_gen/makefile_writer.py (left):

https://codereview.chromium.org/1696483002/diff/20001/platform_tools/android/...
platform_tools/android/gyp_gen/makefile_writer.py:203: f.write('\tLOCAL_CFLAGS
+= -DNO_FALLBACK_FONT\n')
No longer needed?

https://codereview.chromium.org/1696483002/diff/20001/platform_tools/android/...
File platform_tools/android/gyp_gen/makefile_writer.py (right):

https://codereview.chromium.org/1696483002/diff/20001/platform_tools/android/...
platform_tools/android/gyp_gen/makefile_writer.py:184: def
write_static_includes_mk(target_dir, common, deviations_from_common):
Can you add a test/expectations for this new makefile as well?

https://codereview.chromium.org/1696483002/diff/20001/platform_tools/android/...
File platform_tools/android/tests/expectations/Android.mk (right):

https://codereview.chromium.org/1696483002/diff/20001/platform_tools/android/...
platform_tools/android/tests/expectations/Android.mk:86: local_module_static
It looks like this is the only one that uses a tab instead of spaces. Is that
okay?

[djsollen, 2016-02-12 15:57:56]

https://codereview.chromium.org/1696483002/diff/20001/gyp/bench.gyp
File gyp/bench.gyp (left):

https://codereview.chromium.org/1696483002/diff/20001/gyp/bench.gyp#oldcode35
gyp/bench.gyp:35: '-landroid',
On 2016/02/12 15:00:31, scroggo wrote:
> So these libraries are no longer needed?

right. Tom moved the files that depended on them into HWUI as part of an earlier
refactor, but these were not cleaned up.

https://codereview.chromium.org/1696483002/diff/20001/platform_tools/android/...
File platform_tools/android/gyp_gen/makefile_writer.py (left):

https://codereview.chromium.org/1696483002/diff/20001/platform_tools/android/...
platform_tools/android/gyp_gen/makefile_writer.py:203: f.write('\tLOCAL_CFLAGS
+= -DNO_FALLBACK_FONT\n')
On 2016/02/12 15:00:31, scroggo wrote:
> No longer needed?

correct.  That flag no longer appears in our source.

https://codereview.chromium.org/1696483002/diff/20001/platform_tools/android/...
File platform_tools/android/tests/expectations/Android.mk (right):

https://codereview.chromium.org/1696483002/diff/20001/platform_tools/android/...
platform_tools/android/tests/expectations/Android.mk:86: local_module_static
On 2016/02/12 15:00:31, scroggo wrote:
> It looks like this is the only one that uses a tab instead of spaces. Is that
> okay?

I didn't change the behavior and looking at the code I don't see why it chooses
to use a tab here.

[scroggo, 2016-02-12 16:11:58]

lgtm

https://codereview.chromium.org/1696483002/diff/20001/platform_tools/android/...
File platform_tools/android/tests/expectations/Android.mk (right):

https://codereview.chromium.org/1696483002/diff/20001/platform_tools/android/...
platform_tools/android/tests/expectations/Android.mk:86: local_module_static
On 2016/02/12 15:57:56, djsollen wrote:
> On 2016/02/12 15:00:31, scroggo wrote:
> > It looks like this is the only one that uses a tab instead of spaces. Is
that
> > okay?
> 
> I didn't change the behavior and looking at the code I don't see why it
chooses
> to use a tab here.

My bad - they're all tabs. reitveld is just showing it differently...

[djsollen, 2016-02-12 16:17:21]

The CQ bit was checked by djsollen@google.com

[commit-bot: I haz the power, 2016-02-12 16:17:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1696483002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1696483002/40001

[commit-bot: I haz the power, 2016-02-12 16:44:41]

Description was changed from

==========
Update Android framework makefile to build static and shared libs

Also use static lib for testing tools and expose includes needed for
other framework testing tools to statically link in Skia.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Update Android framework makefile to build static and shared libs

Also use static lib for testing tools and expose includes needed for
other framework testing tools to statically link in Skia.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/2a8fcb4647115ce5e9b6127a716b6f50b2a9fd55
==========

[commit-bot: I haz the power, 2016-02-12 16:44:42]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/2a8fcb4647115ce5e9b6127a716b6f50b2a9fd55