Define V8_EXPORT to nothing for clients of v8. (issue 10399036)

50 views
Skip to first unread message

tha...@chromium.org

unread,
May 15, 2012, 4:35:17 PM5/15/12
to ag...@chromium.org, sgj...@chromium.org, v8-...@googlegroups.com
Reviewers: Mads Sig Ager, Søren Thygesen Gjesse,

Description:
Define V8_EXPORT to nothing for clients of v8.

This is to make sure that inline functions are only exported by
libv8.so and not also by all clients. This is the v8 version of
https://chromiumcodereview.appspot.com/10386108/

This CL depends on http://codereview.chromium.org/10310156/ landing
first.

BUG=chromium:90078

Please review this at https://chromiumcodereview.appspot.com/10399036/

SVN Base: http://v8.googlecode.com/svn/trunk/

Affected files:
M include/v8.h


Index: include/v8.h
===================================================================
--- include/v8.h (revision 11566)
+++ include/v8.h (working copy)
@@ -66,7 +66,11 @@
// between building or using the V8 shared library, but we should not
// export symbols when we are building a static library.
#if defined(__GNUC__) && (__GNUC__ >= 4) && defined(V8_SHARED)
+#ifdef BUILDING_V8_SHARED
#define V8EXPORT __attribute__ ((visibility("default")))
+#else
+#define V8EXPORT
+#endif
#else // defined(__GNUC__) && (__GNUC__ >= 4)
#define V8EXPORT
#endif // defined(__GNUC__) && (__GNUC__ >= 4)


ag...@chromium.org

unread,
May 16, 2012, 3:55:36 AM5/16/12
to tha...@chromium.org, sgj...@chromium.org, jkum...@chromium.org, v8-...@googlegroups.com
LGTM


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

http://codereview.chromium.org/10399036/diff/1/include/v8.h#newcode65
include/v8.h:65: // Setup for Linux shared library export. There is no
need to distinguish
Please update the comment as well. :)

http://codereview.chromium.org/10399036/

sgj...@google.com

unread,
May 16, 2012, 4:01:48 AM5/16/12
to tha...@chromium.org, ag...@chromium.org, sgj...@chromium.org, jkum...@chromium.org, v8-...@googlegroups.com

tha...@chromium.org

unread,
May 16, 2012, 12:16:04 PM5/16/12
to ag...@chromium.org, sgj...@chromium.org, jkum...@chromium.org, sgj...@google.com, v8-...@googlegroups.com
Thanks!

The prerequisite landed in r11586 – could someone land this CL?


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

http://codereview.chromium.org/10399036/diff/1/include/v8.h#newcode65
include/v8.h:65: // Setup for Linux shared library export. There is no
need to distinguish
On 2012/05/16 07:55:36, Mads Sig Ager wrote:
> Please update the comment as well. :)

Done.

http://codereview.chromium.org/10399036/

yan...@chromium.org

unread,
May 18, 2012, 5:48:52 AM5/18/12
to tha...@chromium.org, ag...@chromium.org, sgj...@chromium.org, jkum...@chromium.org, sgj...@google.com, v8-...@googlegroups.com

yan...@chromium.org

unread,
May 18, 2012, 5:54:29 AM5/18/12
to tha...@chromium.org, ag...@chromium.org, sgj...@chromium.org, jkum...@chromium.org, sgj...@google.com, v8-...@googlegroups.com
Landed as r11590. Please base your future CLs on svn/bleeding_edge instead
of
svn/trunk :)

http://codereview.chromium.org/10399036/

Nico Weber

unread,
May 18, 2012, 9:57:01 AM5/18/12
to tha...@chromium.org, ag...@chromium.org, sgj...@chromium.org, jkum...@chromium.org, sgj...@google.com, v8-...@googlegroups.com
On Fri, May 18, 2012 at 2:54 AM, <yan...@chromium.org> wrote:
On 2012/05/18 09:48:52, Yang wrote:
On 2012/05/16 16:16:04, Nico wrote:
> Thanks!
>
> The prerequisite landed in r11586 – could someone land this CL?
>
> http://codereview.chromium.org/10399036/diff/1/include/v8.h
> File include/v8.h (right):
>
> http://codereview.chromium.org/10399036/diff/1/include/v8.h#newcode65
> include/v8.h:65: // Setup for Linux shared library export. There is no need
to
> distinguish
> On 2012/05/16 07:55:36, Mads Sig Ager wrote:
> > Please update the comment as well. :)
>
> Done.

I'll land this then.

Landed as r11590.

Thanks!
 
Please base your future CLs on svn/bleeding_edge instead of
svn/trunk :)

Will do. Is there a wiki page somewhere that explains how to send out patches to v8 when using the v8 directory in a chromium checkout? I just did `cd v8; gcl change blah; gcl upload blah;`, and chromium seems to pull from svn/trunk.

Jakob Kummerow

unread,
May 18, 2012, 10:05:37 AM5/18/12
to Nico Weber, ag...@chromium.org, sgj...@chromium.org, sgj...@google.com, v8-...@googlegroups.com

On Fri, May 18, 2012 at 3:57 PM, Nico Weber <tha...@chromium.org> wrote:
Will do. Is there a wiki page somewhere that explains how to send out patches to v8 when using the v8 directory in a chromium checkout? I just did `cd v8; gcl change blah; gcl upload blah;`, and chromium seems to pull from svn/trunk.

There isn't, because that's not supported.

V8 development happens on svn/branches/bleeding_edge; a few times per week we push a reasonably stable snapshot of that to svn/trunk, and roll that into Chromium.

So, to contribute to V8 (which you are most welcome to!), you should get a checkout of bleeding_edge. See http://code.google.com/p/v8/wiki/Source (SVN) or http://code.google.com/p/v8/wiki/UsingGit (git). These checkouts work with gcl/git-cl just fine, so you'll get pretty close to your usual Chromium workflow (biggest difference: there are no try bots for V8).

Reply all
Reply to author
Forward
0 new messages