Upstream Android V8 change.

Upstream Android V8 change.

Committed: http://code.google.com/p/v8/source/detail?r=9904

[danno, 2011-11-03 08:21:39]

Jakob, can you please also take a look at the gyp changes?

[danno, 2011-11-03 08:23:49]

lgtm pending ok from jkummerow. I'll land the CL for you when I get that.

[Jakob Kummerow, 2011-11-03 08:30:56]

LGTM if you confirm that the two things I commented on were done on purpose.

http://codereview.chromium.org/8351073/diff/1/tools/gyp/v8.gyp
File tools/gyp/v8.gyp (right):

http://codereview.chromium.org/8351073/diff/1/tools/gyp/v8.gyp#newcode30
tools/gyp/v8.gyp:30: 'target_conditions': [
Note that these settings will not be applied to d8, cctest and any other targets
not defined in this file. If you want to build those for Android, too, please
move this target_condition to ../../build/common.gypi.

http://codereview.chromium.org/8351073/diff/1/tools/gyp/v8.gyp#newcode748
tools/gyp/v8.gyp:748: 'v8_base',
Since v8_nosnapshot depends on v8_base anyway, I don't see why this addition is
necessary.

[Søren Gjesse, 2011-11-03 11:45:13]

http://codereview.chromium.org/8351073/diff/1/tools/gyp/v8.gyp
File tools/gyp/v8.gyp (right):

http://codereview.chromium.org/8351073/diff/1/tools/gyp/v8.gyp#newcode748
tools/gyp/v8.gyp:748: 'v8_base',
On 2011/11/03 08:30:56, Jakob wrote:
> Since v8_nosnapshot depends on v8_base anyway, I don't see why this addition
is
> necessary.

As far as I remember this is a workaround for a GYP issue where the linker
grouping on Linux is does not happen without this. Maybe it is only for the
patched version of GYP used by Android.

[Jing Zhao, 2011-11-03 15:53:36]

http://codereview.chromium.org/8351073/diff/1/tools/gyp/v8.gyp
File tools/gyp/v8.gyp (right):

http://codereview.chromium.org/8351073/diff/1/tools/gyp/v8.gyp#newcode30
tools/gyp/v8.gyp:30: 'target_conditions': [
On 2011/11/03 08:30:56, Jakob wrote:
> Note that these settings will not be applied to d8, cctest and any other
targets
> not defined in this file. If you want to build those for Android, too, please
> move this target_condition to ../../build/common.gypi.

I think that's a good idea because we'll want to build many things for Android.
I'll do that. Thanks.

http://codereview.chromium.org/8351073/diff/1/tools/gyp/v8.gyp#newcode748
tools/gyp/v8.gyp:748: 'v8_base',
On 2011/11/03 11:45:13, Søren Gjesse wrote:
> On 2011/11/03 08:30:56, Jakob wrote:
> > Since v8_nosnapshot depends on v8_base anyway, I don't see why this addition
> is
> > necessary.
> 
> As far as I remember this is a workaround for a GYP issue where the linker
> grouping on Linux is does not happen without this. Maybe it is only for the
> patched version of GYP used by Android.

When building for Linux, the linking command line was like:
  g++ -pthread -Wl,-z,noexecstack  -o out/Debug/mksnapshot -Wl,--start-group
out/Debug/obj.host/mksnapshot/v8/src/mksnapshot.o
out/Debug/obj.host/v8/tools/gyp/libv8_nosnapshot.a
out/Debug/obj.host/v8/tools/gyp/libv8_base.a -Wl,--end-group 

When building for Android, the linking command line was like:
  g++ -pthread -m32  -o out/Release/mksnapshot
out/Release/obj.host/mksnapshot/v8/src/mksnapshot.o
out/Release/obj.host/v8/tools/gyp/libv8_nosnapshot.a
out/Release/obj.host/v8/tools/gyp/libv8_base.a

The second one doesn't have "start-group" and "end-group" so it failed. Adding
v8_base in dependencies makes libv8_base.a come before libv8_nosnapshot.a, and
then it can succeed.

I don't know what happened to gyp, though. I run android_gyp to generate
makefiles, as defined in build/android/envsetup.sh, which passes some DEFINES to
gyp. Is it because "OS=android"?

[Jakob Kummerow, 2011-11-03 16:35:17]

Jing: Looks like you forgot to add common.gypi to the CL.

[Jing Zhao, 2011-11-03 17:55:30]

On 2011/11/03 16:35:17, Jakob wrote:
> Jing: Looks like you forgot to add common.gypi to the CL.

I had it in another CL, but then I noticed I was wrong. We don't need llog for
all targets, so I moved it back to v8.gyp, because v8 needs it.

[Jakob Kummerow, 2011-11-04 08:21:48]

OK, gyp changes LGTM.