LGTM,

but I think you should try it in chromium to trin away the options which are
inferred. If you run build/gyp_chromium you should be able to run 'make
v8_preparser'.


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

http://codereview.chromium.org/5716001/diff/1/tools/gyp/v8.gyp#newcode33
tools/gyp/v8.gyp:33: 'host_arch%': '<(target_arch)',
I don't think we should set host_arch here it should have been supplied
by the consumer of v8.gyp. When used in Chromium this should all be set
up.

http://codereview.chromium.org/5716001/diff/1/tools/gyp/v8.gyp#newcode200
tools/gyp/v8.gyp:200: 'target_name': 'v8-preparser',
- -> _, other targets use underscore.

http://codereview.chromium.org/5716001/diff/1/tools/gyp/v8.gyp#newcode227
tools/gyp/v8.gyp:227: ['OS=="linux" and library=="shared_library"', {
I also think -fPIC should be provided by the consumer of v8.gyp. We
don't set -fPIC for any other targets in this file.

http://codereview.chromium.org/5716001/diff/1/tools/gyp/v8.gyp#newcode230
tools/gyp/v8.gyp:230: ['v8_target_arch!="x64" and host_arch=="x64"', {
Does the preparser actually care about the v8_target_arch?

Also I think that the only case where v8_target_arch != target_arch
makes sense is when v8_target_arch is arm and target_arch is ia32
(independent of host_arch).

http://codereview.chromium.org/5716001/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to