Bug 55019

Summary: Add a GYP project for JavaScriptGlue
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, mark, ojan, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 55018    
Attachments:
Description Flags
Patch eric: review+

Description Adam Barth 2011-02-22 20:38:30 PST 
Add a GYP project for JavaScriptGlue
Comment 1 Adam Barth 2011-02-22 20:41:44 PST 
Created attachment 83437 [details]
Patch
Comment 2 Eric Seidel (no email) 2011-02-22 20:43:51 PST 
Comment on attachment 83437 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=83437&action=review

Seems useful as a first try.  Thanks.

> Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:24
> +      'configurations': {
> +        'Debug': {},
> +      },

Only Debug?

> Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:28
> +      'defines': [
> +        'WEBKIT_VERSION_MIN_REQUIRED=WEBKIT_VERSION_LATEST',
> +      ],

I assume you're going to rip these all out into shared gypis later? :)
Comment 3 Adam Barth 2011-02-22 20:45:18 PST 
Comment on attachment 83437 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=83437&action=review

>> Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:24
>> +      },
> 
> Only Debug?

For now.  I'm not even sure it builds a proper Debug binary, but that's a solvable problem.

>> Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:28
>> +      ],
> 
> I assume you're going to rip these all out into shared gypis later? :)

For sure.  :)
Comment 4 Adam Barth 2011-02-22 20:49:40 PST 
Committed r79393: <http://trac.webkit.org/changeset/79393>
Comment 5 Tony Chang 2011-02-23 09:30:38 PST 
Comment on attachment 83437 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=83437&action=review

> Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:1
> +{

License block?

> Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:2
> +  'includes': [

Nit: We normally use 4 space indents for gyp/gypi files in WebKit.

> Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:33
> +          'xcode_settings': {
> +            'DEAD_CODE_STRIPPING': 'YES',

I bet mark can help you determine what doesn't need to be here.

> Source/JavaScriptGlue/gyp/JavaScriptGlue.gypi:1
> +{

License block?
Comment 6 Eric Seidel (no email) 2011-02-23 12:48:30 PST 
(In reply to comment #5)
> (From update of attachment 83437 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=83437&action=review
> 
> > Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:1
> > +{
> 
> License block?

Do any other build files have a license block?
Comment 7 Tony Chang 2011-02-23 13:29:44 PST 
(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 83437 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=83437&action=review
> > 
> > > Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:1
> > > +{
> > 
> > License block?
> 
> Do any other build files have a license block?

Some do some don't.  It looks like all our .gyp files do and some .gypi files do.  I also see a license in Android.mk, wscript and a few other places.
Comment 8 Adam Barth 2011-02-23 14:04:19 PST 
> > Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:2
> > +  'includes': [
> 
> Nit: We normally use 4 space indents for gyp/gypi files in WebKit.

Hum...  Two-space is so much more readable.

> > Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:33
> > +          'xcode_settings': {
> > +            'DEAD_CODE_STRIPPING': 'YES',
> 
> I bet mark can help you determine what doesn't need to be here.

Yeah.  I bet a lot of it is overkill.  At the moment, I'm just trying to get a proof-of-concept working.