Enable Compile Option to Build Android Executables

Enable Compile Option to Build Android Executables

Since we can't debug APKs on 4.1, we sometimes need to
build Skia the old way for Android.  This CL enables
a switch to do so.
Committed: https://code.google.com/p/skia/source/detail?r=4653

[EricB, 2012-07-17 17:17:32]

http://codereview.appspot.com/6408053/diff/1/gyp/effects.gyp
File gyp/effects.gyp (right):

http://codereview.appspot.com/6408053/diff/1/gyp/effects.gyp#newcode94
gyp/effects.gyp:94: ],
I was getting linker errors until I added this.  Should this be required?

[epoger, 2012-07-17 18:28:23]

http://codereview.appspot.com/6408053/diff/1/gyp/effects.gyp
File gyp/effects.gyp (right):

http://codereview.appspot.com/6408053/diff/1/gyp/effects.gyp#newcode94
gyp/effects.gyp:94: ],
On 2012/07/17 17:17:32, EricB wrote:
> I was getting linker errors until I added this.  Should this be required?

You might want to consult with Ben Wagner; he has chased down a lot of the gyp
dependencies.

It seems suspicious to me that this gypfile uses the gpu include_dirs but does
not depend on gpu.gyp.  So I can imagine that yes, it should be depended on (in
all cases, not just android_make_apk==0)

[EricB, 2012-07-18 12:01:51]

(From Ben's email)
> I'd say add gpu as an actual dependency. Previously effects didn't
> depend on it, but if it does now (I haven't checked that it does), it
> should do so. Also, I think it depends on ports, but that isn't
> listed either. Eventually I'll get around to actually cleaning these 
> up (I almost had it a while ago).

Since there are _no_ dependencies listed, I'm wondering if the intention was to
have the executables list the dependencies.

There are no problems building for Linux, which is strange.

[EricB, 2012-07-18 12:02:56]

+bungeman@

[DerekS, 2012-07-18 12:10:28]

http://codereview.appspot.com/6408053/diff/3001/gyp/effects.gyp
File gyp/effects.gyp (right):

http://codereview.appspot.com/6408053/diff/3001/gyp/effects.gyp#newcode10
gyp/effects.gyp:10: '../include/gpu',
we should be able to remove this line if we include the dependency all the time.

http://codereview.appspot.com/6408053/diff/3001/gyp/effects.gyp#newcode93
gyp/effects.gyp:93: 'gpu.gyp:gr',
if it really is a dependency shouldn't we include it all the time?

[bungeman, 2012-07-18 14:01:01]

On 2012/07/18 12:01:51, EricB wrote:
> (From Ben's email)
> > I'd say add gpu as an actual dependency. Previously effects didn't
> > depend on it, but if it does now (I haven't checked that it does), it
> > should do so. Also, I think it depends on ports, but that isn't
> > listed either. Eventually I'll get around to actually cleaning these 
> > up (I almost had it a while ago).
> 
> Since there are _no_ dependencies listed, I'm wondering if the intention was
to
> have the executables list the dependencies.
> 
> There are no problems building for Linux, which is strange.

The dependencies should be listed. They currently aren't simply due to the
hackish way these gyp files were created. In general, every (even static) lib
should list all of its code dependencies (if it calls into another library, that
library should be listed). Gyp currently tries to 'help out' by putting all libs
in a group, but I wish it wouldn't. Because of this 'help' sometimes things just
magically work when they probably shouldn't.

[EricB, 2012-07-18 14:19:56]

http://codereview.appspot.com/6408053/diff/3001/gyp/effects.gyp
File gyp/effects.gyp (right):

http://codereview.appspot.com/6408053/diff/3001/gyp/effects.gyp#newcode10
gyp/effects.gyp:10: '../include/gpu',
On 2012/07/18 12:10:28, DerekS wrote:
> we should be able to remove this line if we include the dependency all the
time.

I was able to remove '../include/gpu' but not '../src/gpu'.  Maybe this
indicates that we need some reorganization?

http://codereview.appspot.com/6408053/diff/14001/gyp/effects.gyp
File gyp/effects.gyp (right):

http://codereview.appspot.com/6408053/diff/14001/gyp/effects.gyp#newcode91
gyp/effects.gyp:91: 'gpu.gyp:gr',
Added these as "normal" dependencies and all looks well on my machine.  I can
add others if necessary.

[EricB, 2012-07-18 14:49:27]

http://codereview.appspot.com/6408053/diff/14001/gyp/effects.gyp
File gyp/effects.gyp (right):

http://codereview.appspot.com/6408053/diff/14001/gyp/effects.gyp#newcode8
gyp/effects.gyp:8: '../include/core',
Removed as well.

[DerekS, 2012-07-18 14:59:11]

On 2012/07/18 14:49:27, EricB wrote:
> http://codereview.appspot.com/6408053/diff/14001/gyp/effects.gyp
> File gyp/effects.gyp (right):
> 
> http://codereview.appspot.com/6408053/diff/14001/gyp/effects.gyp#newcode8
> gyp/effects.gyp:8: '../include/core',
> Removed as well.

lgtm