Bug 55196 - Add necessary build steps to JavaScriptGlue GYP project.
Summary: Add necessary build steps to JavaScriptGlue GYP project.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Dimitri Glazkov (Google)
URL:
Keywords:
Depends on:
Blocks: 55018
  Show dependency treegraph
 
Reported: 2011-02-24 16:54 PST by Dimitri Glazkov (Google)
Modified: 2011-02-28 11:50 PST (History)
1 user (show)

See Also:


Attachments
WIP: Not building yet, capturing progress so far. (3.00 KB, patch)
2011-02-24 16:55 PST, Dimitri Glazkov (Google)
no flags Details | Formatted Diff | Diff
Patch (4.33 KB, patch)
2011-02-25 10:11 PST, Dimitri Glazkov (Google)
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitri Glazkov (Google) 2011-02-24 16:54:55 PST
Add necessary build steps to JavaScriptGlue GYP project.
Comment 1 Dimitri Glazkov (Google) 2011-02-24 16:55:17 PST
Created attachment 83750 [details]
WIP: Not building yet, capturing progress so far.
Comment 2 Adam Barth 2011-02-24 18:12:17 PST
Comment on attachment 83750 [details]
WIP: Not building yet, capturing progress so far.

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

> Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:20
> +        '../Info.plist',

Should this be in JavaScriptGlue.gypi?

> Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:36
> +          'action': [ 'if [ -f ../../Tools/Scripts/check-for-global-initializers ]; then\n    ../../Tools/Scripts/check-for-global-initializers || exit $?\nfi";' ],

I'm not sure whether it's better to put these inline in the gyp or to having them be external sh files.  In JavaScriptCore.gyp, I went with the external sh files.  I'm not sure this matters at this stage.  Just thinking out loud.

> Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:217
> +          'inputs': [ '$(SRCROOT)/Configurations/Version.xcconfig' ],

This won't work properly because the current plan is to suck all the xcconfig files into the gyp file.  If there's a way to grab the xcconfig files from the file system, that might be better.

> Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:218
> +          'outputs': [ '$(SRCROOT)/Info.plist' ],

Probably better to not use $ variables in inputs and outputs.  $ variables are only understood by Xcode.  If we use < variables, they'll be understood by gyp.
Comment 3 Dimitri Glazkov (Google) 2011-02-25 10:11:52 PST
Created attachment 83830 [details]
Patch
Comment 4 Dimitri Glazkov (Google) 2011-02-25 11:50:09 PST
(In reply to comment #2)
> (From update of attachment 83750 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=83750&action=review
> 
> > Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:20
> > +        '../Info.plist',
> 
> Should this be in JavaScriptGlue.gypi?

Done.

> 
> > Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:36
> > +          'action': [ 'if [ -f ../../Tools/Scripts/check-for-global-initializers ]; then\n    ../../Tools/Scripts/check-for-global-initializers || exit $?\nfi";' ],
> 
> I'm not sure whether it's better to put these inline in the gyp or to having them be external sh files.  In JavaScriptCore.gyp, I went with the external sh files.  I'm not sure this matters at this stage.  Just thinking out loud.

Pulled out.

> 
> > Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:217
> > +          'inputs': [ '$(SRCROOT)/Configurations/Version.xcconfig' ],
> 
> This won't work properly because the current plan is to suck all the xcconfig files into the gyp file.  If there's a way to grab the xcconfig files from the file system, that might be better.

Turns out, it's not necessary for the script. I was just mimicking the original project file.

> 
> > Source/JavaScriptGlue/gyp/JavaScriptGlue.gyp:218
> > +          'outputs': [ '$(SRCROOT)/Info.plist' ],
> 
> Probably better to not use $ variables in inputs and outputs.  $ variables are only understood by Xcode.  If we use < variables, they'll be understood by gyp.

Done.
Comment 5 Adam Barth 2011-02-25 14:26:55 PST
Comment on attachment 83830 [details]
Patch

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

This is great!  Thanks.

> Source/JavaScriptGlue/ChangeLog:12
> +        * gyp/remove-headers-if-needed.sh: Added.
> +        * gyp/run-if-exists.sh: Added.
> +        * gyp/update-info-plist.sh: Added.

We'll probably want to move these up in the source tree so they can be shared by multiple projects, but we can do that once we tackle the sharing problems.

> Source/JavaScriptGlue/gyp/run-if-exists.sh:4
> +if [ -f ../../Tools/Scripts/$1 ]; then
> +    ../../Tools/Scripts/$1 || exit $?;

It would be nice if we didn't have to hard-code the "../.." here.  Maybe run-if-exists should take the fully relative path and then we can let GYP magic figure out the depth?
Comment 6 Adam Barth 2011-02-28 11:50:42 PST
Committed r79890: <http://trac.webkit.org/changeset/79890>