linux: Don't force-export V8 symbols unless v8 is built as a shared library....

linux: Don't force-export V8 symbols unless v8 is built as a shared library.

That allows clients (chrome, o3d) to not have the symbols exported if v8 is
built with -fvisibility=hidden, so that when the 2 are put together bad
don't happen.

See also http://code.google.com/p/chromium/issues/detail?id=17557

[Evan Martin, 2009-08-03 23:48:35]

+dean, asac

http://codereview.chromium.org/160556/diff/1/4
File SConstruct (right):

http://codereview.chromium.org/160556/diff/1/4#newcode129
Line 129: 'CPPDEFINES':   ['V8_SHARED'],
whitespace on this is weird

http://codereview.chromium.org/160556/diff/1/2
File include/v8.h (right):

http://codereview.chromium.org/160556/diff/1/2#newcode88
Line 88: // Setup for Linux shared library export. There is no need to
destinguish
s/destinguish/distinguish/

[piman, 2009-08-04 00:00:38]

http://codereview.chromium.org/160556/diff/1/4
File SConstruct (right):

http://codereview.chromium.org/160556/diff/1/4#newcode129
Line 129: 'CPPDEFINES':   ['V8_SHARED'],
On 2009/08/03 23:48:35, Evan Martin wrote:
> whitespace on this is weird

Yeah, sorry, the entire file is weird.

http://codereview.chromium.org/160556/diff/1/2
File include/v8.h (right):

http://codereview.chromium.org/160556/diff/1/2#newcode88
Line 88: // Setup for Linux shared library export. There is no need to
destinguish
On 2009/08/03 23:48:35, Evan Martin wrote:
> s/destinguish/distinguish/

Done.

[Christian Plesner Hansen, 2009-08-04 07:28:55]

Lgtm

[Søren Thygesen Gjesse, 2009-08-04 07:30:55]

LGTM, except

This change should go on branches/bleeding_edge and then merged to branches/1.2.

http://codereview.chromium.org/160556/diff/7/1009
File SConstruct (right):

http://codereview.chromium.org/160556/diff/7/1009#newcode129
Line 129: 'CPPDEFINES': ['V8_SHARED'],
How about using BUILDING_V8_SHARED to use the same define as on Windows?

[piman, 2009-08-04 07:43:53]

http://codereview.chromium.org/160556/diff/7/1009
File SConstruct (right):

http://codereview.chromium.org/160556/diff/7/1009#newcode129
Line 129: 'CPPDEFINES': ['V8_SHARED'],
On 2009/08/04 07:30:55, Søren Gjesse wrote:
> How about using BUILDING_V8_SHARED to use the same define as on Windows?

It's defined both when compiled and used, which is why I chose a different
symbol, but it doesn't matter to me.

[Søren Thygesen Gjesse, 2009-08-04 08:00:42]

http://codereview.chromium.org/160556/diff/7/1009
File SConstruct (right):

http://codereview.chromium.org/160556/diff/7/1009#newcode129
Line 129: 'CPPDEFINES': ['V8_SHARED'],
On 2009/08/04 07:43:53, piman wrote:
> On 2009/08/04 07:30:55, Søren Gjesse wrote:
> > How about using BUILDING_V8_SHARED to use the same define as on Windows?
> 
> It's defined both when compiled and used, which is why I chose a different
> symbol, but it doesn't matter to me.

If you want it for both then I suggest to use BUILDING_V8_SHARED when building
the shared library (that will mean moving it to V8_EXTRA_FLAGS in SConstruct),
and use USING_V8_SHARED when building the client. In v8.h both these defines
should end up have the same effect.

If you move it from  to V8_EXTRA_FLAGS it will only be defined when building V8
not when using it.
You are right - keep it as V8_SHARED

[piman, 2009-08-04 21:04:30]

On 2009/08/04 08:00:42, Søren Gjesse wrote:
> http://codereview.chromium.org/160556/diff/7/1009
> File SConstruct (right):
> 
> http://codereview.chromium.org/160556/diff/7/1009#newcode129
> Line 129: 'CPPDEFINES': ['V8_SHARED'],
> On 2009/08/04 07:43:53, piman wrote:
> > On 2009/08/04 07:30:55, Søren Gjesse wrote:
> > > How about using BUILDING_V8_SHARED to use the same define as on Windows?
> > 
> > It's defined both when compiled and used, which is why I chose a different
> > symbol, but it doesn't matter to me.
> 
> If you want it for both then I suggest to use BUILDING_V8_SHARED when building
> the shared library (that will mean moving it to V8_EXTRA_FLAGS in SConstruct),
> and use USING_V8_SHARED when building the client. In v8.h both these defines
> should end up have the same effect.
> 
> If you move it from  to V8_EXTRA_FLAGS it will only be defined when building
V8
> not when using it.
> You are right - keep it as V8_SHARED

I updated to bleeding_edge and uploaded a new patch, although rietveld doesn't
pick that up.

Since I don't have commit access, could anyone of you please apply it ? Thanks.