Adding use_system_v8 gyp option

[Paweł Hajdan, Jr.]

What do you think about adding use_system_v8 option to v8.gyp? I'd create the patch, just asking first if it has a chance to get accepted.

Gentoo Linux is building with system V8 successfully as an option, and Debian also builds with system V8 afaik.

[Søren Gjesse]

Pawel,

Why do you want to add this option to v8.gyp? lease explain as I would have expected this option to be handled in each dependent target like Chromium where the actual V8 to use should be defined.

How is this handled by other libraries used by e.g. Chromium?

Regards,

Søren

On Mon, Jan 3, 2011 at 21:39, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

What do you think about adding use_system_v8 option to v8.gyp? I'd create the patch, just asking first if it has a chance to get accepted.

Gentoo Linux is building with system V8 successfully as an option, and Debian also builds with system V8 afaik.

--
v8-users mailing list
v8-u...@googlegroups.com
http://groups.google.com/group/v8-users

[Paweł Hajdan, Jr.]

2011/1/4 Søren Gjesse <sgj...@chromium.org>

Why do you want to add this option to v8.gyp? lease explain as I would have expected this option to be handled in each dependent target like Chromium where the actual V8 to use should be defined.

v8.gyp is referenced in many places, see http://codesearch.google.com/codesearch?as_q=v8.gyp&btnG=Search+Code&hl=en&vert=chromium&filesuggest=&as_lang=&as_filename=&as_class=&as_function=&as_case=

It seems easier to make the change just in v8.gyp, and not all the places where it is referenced. For an example of the second approach, see Debian's patch at http://patch-tracker.debian.org/patch/series/view/chromium-browser/7.0.544.0~r61416-1/system_v8.patch . Note: it also changes how v8's headers are #included. For now I'm not going to change that.

By the way, the change is not complicated at all. For now Gentoo just replaces the entire gyp file, see http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/www-client/chromium/files/v8.gyp?revision=1.1&view=markup . For upstreaming, I'd just make a conditional block (if use_system_v8 then the current targets would be used, otherwise the -lv8 targets from Gentoo's v8.gyp).

 

How is this handled by other libraries used by e.g. Chromium?

See use_system_sqlite in http://src.chromium.org/viewvc/chrome/trunk/src/third_party/sqlite/sqlite.gyp?revision=68531&view=markup, use_system_icu in http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/icu42/icu.gyp?revision=69864&view=markup and use_system_libevent in http://src.chromium.org/viewvc/chrome/trunk/src/third_party/libevent/libevent.gyp?revision=39362&view=markup 

[Søren Gjesse]

I see - it will be OK to add this to v8.gyp. I wonder why all the targets are required in the Gentoo gyp file. For a system V8 then only the v8 target should be required, as that is the only think that you can link with. Whether to use snapshots or not is determined by the system library and not by the consumer.

By the way why are values 0/1 used instead of false/true? I would prefer real booleans as is used by the variable v8_use_snapshot, but of cause consistency also matters the other way.

Regards,

Søren

--

[Paweł Hajdan, Jr.]

2011/1/4 Søren Gjesse <sgj...@chromium.org>

I see - it will be OK to add this to v8.gyp.

Great - submitted http://codereview.chromium.org/6092006/ for review.

 

I wonder why all the targets are required in the Gentoo gyp file. For a system V8 then only the v8 target should be required, as that is the only think that you can link with. Whether to use snapshots or not is determined by the system library and not by the consumer.

Oh, right. I've removed those from my CL.

 

By the way why are values 0/1 used instead of false/true? I would prefer real booleans as is used by the variable v8_use_snapshot, but of cause consistency also matters the other way.

I've submitted with 0/1, for consistency with other use_system_ gyp options. Please note that in the current version of v8.gyp there seem to be two bool variables, one using 0/1 and one using true/false. Either way use_system_v8 would be inconsistent, so I opted for "global" consistency instead. Of course we can discuss this more in the review issue, I'm fine with either solution.