Fix Android framework build

Fix Android framework build

The Android framework was failing on conditions in
libjpeg-turbo.gyp, even though libjpeg-turbo is listed
in dependencies! for the framework (maybe because I
forgot to add export_dependent_settings!).  This is fixed
by rearranging the gyp file.

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/551369163fcf149a53f26f82039f0d23854ddc94

[msarett, 2015-07-22 14:27:41]

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

[msarett, 2015-07-22 14:27:41]

https://codereview.chromium.org/1249003002/diff/1/platform_tools/android/gyp_...
File platform_tools/android/gyp_gen/android_framework_gyp.py (right):

https://codereview.chromium.org/1249003002/diff/1/platform_tools/android/gyp_...
platform_tools/android/gyp_gen/android_framework_gyp.py:63: host_os =
os.uname()[0].lower()
Maybe it doesn't need to be this complicated and host_os will always be linux?

[scroggo, 2015-07-22 14:37:08]

On 2015/07/22 14:27:41, msarett wrote:
>
https://codereview.chromium.org/1249003002/diff/1/platform_tools/android/gyp_...
> File platform_tools/android/gyp_gen/android_framework_gyp.py (right):
> 
>
https://codereview.chromium.org/1249003002/diff/1/platform_tools/android/gyp_...
> platform_tools/android/gyp_gen/android_framework_gyp.py:63: host_os =
> os.uname()[0].lower()
> Maybe it doesn't need to be this complicated and host_os will always be linux?

I think this is the wrong way to solve the problem.

The problem with this approach is that we're building a makefile, which will be
built later (potentially on a different host_os).

Further, when we do that build, we're no longer in a pure Skia checkout - we're
inside the Android checkout, where we do not have externals. Where you use
host_os is in determining whether to build particular files located in Skia's
third_party/externals.

When building for the Android framework, we should instead depend on whatever
the system has. Android currently does not have libjpeg-turbo, so I think we
need to just skip over the gyp file (or the condition) that uses host_os. Does
that make sense?

[msarett, 2015-07-22 15:05:53]

Patchset #2 (id:20001) has been deleted

[msarett, 2015-07-22 15:06:27]

I think this is more along the lines of the right approach
but I still have questions about it.

My first thought was to just not compile turbo on the
framework, but we need it to compile codec given that
I have mangled the API names.

Instead I short-circuited the conditions to always compile
the assembly code on the framework.  I'm not sure if this is
safe?  Maybe it is better to always turn it off until we
have turbo on Android?

[scroggo, 2015-07-22 15:15:55]

On 2015/07/22 15:06:27, msarett wrote:
> I think this is more along the lines of the right approach
> but I still have questions about it.
> 
> My first thought was to just not compile turbo on the
> framework, but we need it to compile codec given that
> I have mangled the API names.
> 
> Instead I short-circuited the conditions to always compile
> the assembly code on the framework.  I'm not sure if this is
> safe?  Maybe it is better to always turn it off until we
> have turbo on Android?

I don't think so. Again, the assembly code you're telling it to compile is not
available on the framework (it's in Skia's third_party/externals, which is
pulled in by DEPS, and therefore not merged into Android).

Looking at codec.gyp, it appears that we don't even build libjpeg-turbo (or even
SkJpegCodec) for the Android framework (which we'll fix once the Android
framework has libjpeg-turbo). So it seems like we should not even reach this
file?

Here's a theory - maybe it's trying to evaluate this file because we add
libjpeg-turbo to the dependencies, and then remove it for the android framework.
Maybe if we only add it when not building for the android framework that would
fix the problem?

I think you'll need to try testing it out from within a master-skia build. I can
help you with that if you need me to.

[msarett, 2015-07-22 15:41:52]

You are right on all counts.

I had forgotten that we already disable SkJpegCodec on the
android framework.  The strange part was that it was trying
to compile libjpeg-turbo at all.

Make the switch you suggested - only putting turbo in
non-framework dependencies - seems to have worked.

Feel free to turn around and chat with me if there is
additional testing I should do.

[scroggo, 2015-07-22 15:51:11]

Please update the description to match the current change.

Otherwise lgtm

https://codereview.chromium.org/1249003002/diff/60001/gyp/codec.gyp
File gyp/codec.gyp (right):

https://codereview.chromium.org/1249003002/diff/60001/gyp/codec.gyp#newcode71
gyp/codec.gyp:71: 'libjpeg-turbo.gyp:libjpeg-turbo',
Similar TODO here?

[msarett, 2015-07-22 16:00:06]

https://codereview.chromium.org/1249003002/diff/60001/gyp/codec.gyp
File gyp/codec.gyp (right):

https://codereview.chromium.org/1249003002/diff/60001/gyp/codec.gyp#newcode71
gyp/codec.gyp:71: 'libjpeg-turbo.gyp:libjpeg-turbo',
On 2015/07/22 15:51:11, scroggo wrote:
> Similar TODO here?

Done.

[msarett, 2015-07-22 16:00:11]

The CQ bit was checked by msarett@google.com

[msarett, 2015-07-22 16:00:11]

The patchset sent to the CQ was uploaded after l-g-t-m from scroggo@google.com
Link to the patchset: https://codereview.chromium.org/1249003002/#ps80001
(title: " ")

[commit-bot: I haz the power, 2015-07-22 16:00:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1249003002/80001

[commit-bot: I haz the power, 2015-07-22 16:08:31]

Committed patchset #4 (id:80001) as
https://skia.googlesource.com/skia/+/551369163fcf149a53f26f82039f0d23854ddc94