Bug 109003 – [GTK] Add an experimental gyp build

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Martin Robinson]

Interest grows in sharing source lists via gyp [1]. Additionally we have many issues with the autotools build including broken make dist steps and artificial splits in the source list due to automake trying to stuff the entire source list into a command-line invocation. Maybe we have outgrown our current build tool.

On the other hand, gyp has no support for 'make dist' and 'make distcheck,' so we'd need to write that ourselves or use some other tool. The first step to exploring alternatives is to do some testing.

1. https://lists.webkit.org/pipermail/webkit-dev/2013-January/023538.html

[Martin Robinson]

Created attachment 186760 [details]
Patch

[Build Bot]

Comment on attachment 186760 [details]
Patch

Attachment 186760 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16391152

[Martin Robinson]

(In reply to comment #2)
> (From update of attachment 186760 [details])
> Attachment 186760 [details] did not pass win-ews (win):
> Output: http://queues.webkit.org/results/16391152

Looks like a false alarm. I didn't touch Platform.h at all.

[Gustavo Noronha (kov)]

I'd rs=me this, but I prefer to have someone who has more experience with the existing gyp build system have a look at it, I do not fully understand the consequences to the chromium build of adding those files to the JSC gyp.

[Eric Seidel (no email)]

You're my hero, mrobinson.

[Nico Weber]

Comment on attachment 186760 [details]
Patch

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

Looks really good! A few suggestions below: (i'm not a reviewer)

> Source/WebKit/gtk/ChangeLog:12
> +        $ gyp --generator-output=build --depth=. Source/WebKit/gtk/gyp/JavaScriptCore.gyp

FYI: Tools/Scripts/update-webkit still has a --gyp flag back from abarth played with this. Maybe you could use that eventually.

> Source/WebKit/gtk/gyp/JavaScriptCore.gyp:9
> +      '<(srcdir)/JavaScriptCore',

I think we need this variable in chromium because we don't know the absolute path to the build dir since we support both webkit standalone builds and webkit-in-chromium builds. You can probably just write ../../JavaScriptCore here?

(also below)

> Source/WebKit/gtk/gyp/JavaScriptCore.gyp:34
> +        'dependencies': [ 'WTF.gyp:wtf', 'WTF.gyp:glib', 'WTF.gyp:icu' ],

nit: I'd use one line per dep

> Source/WebKit/gtk/gyp/WTF.gyp:3
> +    'WebKitGTK+.gyp',

This should probably have extension .gypi?

> Source/WebKit/gtk/gyp/generate-derived-sources.sh:14
> +make --no-builtin-rules -f "$JavaScriptCore/DerivedSources.make" JavaScriptCore=$JavaScriptCore

I think we use a 'rule' instead of an 'action' for derived sources. This has the advantage that the build system can schedule the work on a finer level. Not necessary for v1 though.

[Eric Seidel (no email)]

(In reply to comment #6)
> (From update of attachment 186760 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186760&action=review
> 
> Looks really good! A few suggestions below: (i'm not a reviewer)

Actually you are. :)  Congrats.

[Martin Robinson]

(In reply to comment #6)

Thanks so much for the review.

> (From update of attachment 186760 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186760&action=review
> 
> Looks really good! A few suggestions below: (i'm not a reviewer)
> 
> > Source/WebKit/gtk/ChangeLog:12
> > +        $ gyp --generator-output=build --depth=. Source/WebKit/gtk/gyp/JavaScriptCore.gyp
> 
> FYI: Tools/Scripts/update-webkit still has a --gyp flag back from abarth played with this. Maybe you could use that eventually.

For sure. The long term plan is to add an option to build-webkit to allow building with gyp.

> > Source/WebKit/gtk/gyp/JavaScriptCore.gyp:9
> > +      '<(srcdir)/JavaScriptCore',
> 
> I think we need this variable in chromium because we don't know the absolute path to the build dir since we support both webkit standalone builds and webkit-in-chromium builds. You can probably just write ../../JavaScriptCore here?

I used a variable here, because the location of the .gyp files is quite likely to change. I'm happy to use a relative path though.

> (also below)
> 
> > Source/WebKit/gtk/gyp/JavaScriptCore.gyp:34
> > +        'dependencies': [ 'WTF.gyp:wtf', 'WTF.gyp:glib', 'WTF.gyp:icu' ],
> 
> nit: I'd use one line per dep

Okay. I'll fix this all over.

> > Source/WebKit/gtk/gyp/WTF.gyp:3
> > +    'WebKitGTK+.gyp',
> 
> This should probably have extension .gypi?

Definitely. At some point this may become a full-blown gyp file with targets, but for now it's just an include.
> 
> > Source/WebKit/gtk/gyp/generate-derived-sources.sh:14
> > +make --no-builtin-rules -f "$JavaScriptCore/DerivedSources.make" JavaScriptCore=$JavaScriptCore
> 
> I think we use a 'rule' instead of an 'action' for derived sources. This has the advantage that the build system can schedule the work on a finer level. Not necessary for v1 though.

One issue I noticed was that the rules did not seem to support the kind of files that JSC builds. For example I wanted to build a lot of .lut.h files and setting extension=.lut.h didn't seem to work properly. I was hoping that this target would build all derived sources in one pass.

[Dirk Pranke]

Comment on attachment 186760 [details]
Patch

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

>>> Source/WebKit/gtk/ChangeLog:12
>>> +        $ gyp --generator-output=build --depth=. Source/WebKit/gtk/gyp/JavaScriptCore.gyp
>> 
>> FYI: Tools/Scripts/update-webkit still has a --gyp flag back from abarth played with this. Maybe you could use that eventually.
> 
> For sure. The long term plan is to add an option to build-webkit to allow building with gyp.

I would think the long-long term plan would be to just make build-webkit --gtk do the right thing automatically, right?

>>> Source/WebKit/gtk/gyp/JavaScriptCore.gyp:9
>>> +      '<(srcdir)/JavaScriptCore',
>> 
>> I think we need this variable in chromium because we don't know the absolute path to the build dir since we support both webkit standalone builds and webkit-in-chromium builds. You can probably just write ../../JavaScriptCore here?
>> 
>> (also below)
> 
> I used a variable here, because the location of the .gyp files is quite likely to change. I'm happy to use a relative path though.

I actually stylistically prefer <(srcdir) ... I think it tends to be a lot clearer than relative paths. of course, maybe eventually this should be <(Source) or something.

> Source/WebKit/gtk/gyp/JavaScriptCore.gyp:64
> +        'type': 'shared_library',

Do you always build this as a .so, or do you support (or want to support) both shared and static library variants?

>>> Source/WebKit/gtk/gyp/WTF.gyp:3
>>> +    'WebKitGTK+.gyp',
>> 
>> This should probably have extension .gypi?
> 
> Definitely. At some point this may become a full-blown gyp file with targets, but for now it's just an include.

see comment below ...

> Source/WebKit/gtk/gyp/WebKitGTK+.gyp:15
> +}

I think there's two different concepts going on here between your comments above re: needing full targets and what's actually in this file. You have the need for a "top-level" GYP file (like we have in Source/WebKit/chromium/WebKit.gyp) which defines the targets that make up 'all', and then you have a common set of definitions for feature configuration, etc (which is what you actually have in this file). In chromium, we tend to keep the feature configuration in a separate .gypi file (which lives in the chromium repo, in build/common.gypi).

Then, we have a wrapper for gyp (gyp_webkit, for example) that passes -I build/common.gypi so that the feature configuration gyp file is automatically included into every gyp invocation, and that way you don't necessarily have to worry about including that gypi from every .gyp file.

I can't actually say for sure that I like the way chromium does things. It does sometimes feel a little weird to me but I haven't really tried alternatives, either. Perhaps Nico has some thoughts on this as well ...


In chromium we tend to have two different files for the

>>> Source/WebKit/gtk/gyp/generate-derived-sources.sh:14
>>> +make --no-builtin-rules -f "$JavaScriptCore/DerivedSources.make" JavaScriptCore=$JavaScriptCore
>> 
>> I think we use a 'rule' instead of an 'action' for derived sources. This has the advantage that the build system can schedule the work on a finer level. Not necessary for v1 though.
> 
> One issue I noticed was that the rules did not seem to support the kind of files that JSC builds. For example I wanted to build a lot of .lut.h files and setting extension=.lut.h didn't seem to work properly. I was hoping that this target would build all derived sources in one pass.

I'm not sure what the right incantation is to get that to work. There might be a bug here or the syntax might be different/unintuitive. I can look into it.

[Dirk Pranke]

So, I was hoping to actually apply this patch and try it out to build gtk, but I had no success here, I ran into various problems.

First, I attempted to just get a "vanilla" gtk build to work, following the directions at http://trac.webkit.org/wiki/BuildingGtk . I am using a linux machine that is more-or-less ubuntu precise pangolin, and I could not find a new enough version of libsoup, nor a package for harfbuzz. I also ran into problems w/ opus and libsecret .

Then, I tried to see if I could just get the gyp patch to build (hoping I wouldn't need all those dependencies). As discussed on #webkit, the version of ICU I have didn't work with pkg-config, so I needed to change to icu-config. Even after I got that working (and gyp generated the makefiles properly), I got a whole slew of compile warnings trying to compile WTF, and things eventually keeled over with compile failures. 

So, I'm not sure if this patch actually works, or if these were all environmental issues w/ my config (i.e., I"m not sure how interesting the compile warnings and failures I got were). Anyone know if I should be able to get gtk to build on a precise-based machine, or do I need to upgrade to something newer?

apart from that, I can confirm that the patch didn't appear to affect my chromium build, at least :).

[Martin Robinson]

(In reply to comment #9)

> I would think the long-long term plan would be to just make build-webkit --gtk do the right thing automatically, right?

Yes, though for a while any new build system will have to co-exist somewhat with the old system until I can get enough buy-in from the rest of the developers. I'm hoping to win them over with build speeds.

> I actually stylistically prefer <(srcdir) ... I think it tends to be a lot clearer than relative paths. of course, maybe eventually this should be <(Source) or something.

Okay. I've changed it all to say Source. I like the look of that since it matches the directory structure of WebKit. :)

> > Source/WebKit/gtk/gyp/JavaScriptCore.gyp:64
> > +        'type': 'shared_library',
> 
> Do you always build this as a .so, or do you support (or want to support) both shared and static library variants?

We do always build JavaScript core as a shared object, though the naming will have to be a bit more sophisticated to support Windows in the future.

> I think there's two different concepts going on here between your comments above re: needing full targets and what's actually in this file. You have the need for a "top-level" GYP file (like we have in Source/WebKit/chromium/WebKit.gyp) which defines the targets that make up 'all', and then you have a common set of definitions for feature configuration, etc (which is what you actually have in this file). In chromium, we tend to keep the feature configuration in a separate .gypi file (which lives in the chromium repo, in build/common.gypi).
> 
> Then, we have a wrapper for gyp (gyp_webkit, for example) that passes -I build/common.gypi so that the feature configuration gyp file is automatically included into every gyp invocation, and that way you don't necessarily have to worry about including that gypi from every .gyp file.
> 
> I can't actually say for sure that I like the way chromium does things. It does sometimes feel a little weird to me but I haven't really tried alternatives, either. Perhaps Nico has some thoughts on this as well ...

You're definitely right about this. In the new patch I've modified all these includes to reference the new file Configuration.gypi. In the next phase of this work, Configuration.gypi will move to the build directory and be generated by autoconf. Because of this, I will likely need a wrapper that includes it with -I.

> > One issue I noticed was that the rules did not seem to support the kind of files that JSC builds. For example I wanted to build a lot of .lut.h files and setting extension=.lut.h didn't seem to work properly. I was hoping that this target would build all derived sources in one pass.
> 
> I'm not sure what the right incantation is to get that to work. There might be a bug here or the syntax might be different/unintuitive. I can look into it.

I really appreciate that. The DerivedSources.make solution is tempting simply because any potential Mac gyp build will likely use this as well (judging from Adam and Eric's previous work).

(In reply to comment #10)
> So, I was hoping to actually apply this patch and try it out to build gtk, but I had no success here, I ran into various problems.
> 
> First, I attempted to just get a "vanilla" gtk build to work, following the directions at http://trac.webkit.org/wiki/BuildingGtk . I am using a linux machine that is more-or-less ubuntu precise pangolin, and I could not find a new enough version of libsoup, nor a package for harfbuzz. I also ran into problems w/ opus and libsecret .

libsoup and harfbuzz should be provided by the jhbuild setup. To ensure you have that active, run ./Tools/Scripts/update-webkitgtk-libs. You may have to delete your WebKitBuild/Release directory and start over after that. :( I should have a patch posted tonight eliminating the dependency on libsecret.

> Then, I tried to see if I could just get the gyp patch to build (hoping I wouldn't need all those dependencies). As discussed on #webkit, the version of ICU I have didn't work with pkg-config, so I needed to change to icu-config. Even after I got that working (and gyp generated the makefiles properly), I got a whole slew of compile warnings trying to compile WTF, and things eventually keeled over with compile failures. 

I think the compilation errors are due to using icu-config --cflags instead of icu-config --cppflags. I've corrected all this in the new version of the patch. Thanks for looking it over!

[Martin Robinson]

Created attachment 186984 [details]
Patch

[Mark Rowe (bdash)]

Can you clarify why JavaScriptCore.gyp in WebKit/gtk is what defines how to build JavaScriptCore? Surely those rules, particularly the ones related to derived sources, will be common to all platforms building JavaScriptCore.

[Martin Robinson]

(In reply to comment #13)
> Can you clarify why JavaScriptCore.gyp in WebKit/gtk is what defines how to build JavaScriptCore? Surely those rules, particularly the ones related to derived sources, will be common to all platforms building JavaScriptCore.

There are a couple reasons I did this. Right now Source/JavaScriptCore/JavaScriptCore.gyp/JavaScriptCore.gyp is very Chromium specific. The Chromium assumption is baked very deeply within the gyp build. There's very little we care share at the moment with Chromium (since it doesn't use JSC).

The other reason is that if you look back into the history you'll find that the short-lived Mac gyp build files had a very particular structure, made so that resulting XCode project looked as much like current Mac XCode projects as possible. I can't really maintain that consistency on my non-Mac Machine.

My thought was that when more ports start using Gyp they could move shared bits to the JavaScript core directory. I'm open to other strategies.

[Dirk Pranke]

right, I think the idea with this patch was to try and get *something* working w/ gtk while minimizing the risk/impact to chromium.

I do think we'd refactor things soonish to put the right rules in JavaScriptCore and share them between the two (or more) builds.

[Mark Rowe (bdash)]

(In reply to comment #14)
> (In reply to comment #13)
> > Can you clarify why JavaScriptCore.gyp in WebKit/gtk is what defines how to build JavaScriptCore? Surely those rules, particularly the ones related to derived sources, will be common to all platforms building JavaScriptCore.
> 
> There are a couple reasons I did this. Right now Source/JavaScriptCore/JavaScriptCore.gyp/JavaScriptCore.gyp is very Chromium specific. The Chromium assumption is baked very deeply within the gyp build. There's very little we care share at the moment with Chromium (since it doesn't use JSC).
> 
> The other reason is that if you look back into the history you'll find that the short-lived Mac gyp build files had a very particular structure, made so that resulting XCode project looked as much like current Mac XCode projects as possible. I can't really maintain that consistency on my non-Mac Machine.

Yeah, the different ports will certainly want control over how built products are named and located. There are also many differences in how search paths and linking are set up between ports and platforms, so the general structure of the project may also differ between ports. My concern is primarily around the various rules since for the most part they're going to be virtually identical across ports.

> My thought was that when more ports start using Gyp they could move shared bits to the JavaScript core directory. I'm open to other strategies.

Pulling the various rules and actions out in to separate files as we move forward seems like a reasonable enough approach. I'm not hugely familiar with how gyp functions so I wasn't sure how practical doing that was. I've spent a few minutes futzing around with it on my local machine and it seems like it'll be easy enough to do when we need it.

[WebKit Review Bot]

Comment on attachment 186984 [details]
Patch

Clearing flags on attachment: 186984

Committed r142298: <http://trac.webkit.org/changeset/142298>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Nico Weber]

Sorry for the delayed reply, I was away the last two days. Looks like this made it through in the meantime, but I wanted to comment on one point below:

(In reply to comment #8)
> One issue I noticed was that the rules did not seem to support the kind of files that JSC builds. For example I wanted to build a lot of .lut.h files and setting extension=.lut.h didn't seem to work properly. I was hoping that this target would build all derived sources in one pass.

'extension' is the input extension, not the output extension. Here's how the rule would look:

  {
    'targets': [
      {
        'target_name': 'genluts',
        'type': 'none',
        'sources': [
          'runtime/BooleanPrototype.cpp',
          'runtime/NumberConstructor.cpp',
          # ...
        ],
        'rules': [
          {
            'rule_name': 'genlut',
            'extension': 'cpp',
            'inputs': [ 'create_hash_table', ],
            'outputs': [
              '<(SHARED_INTERMEDIATE_DIR)/jsc_headers/<(RULE_INPUT_ROOT).h',
            ],
            'action': [
              './redirect-stdout.sh',
              'perl create_hash_table <(RULE_INPUT_PATH)',
              '<(SHARED_INTERMEDIATE_DIR)/jsc_headers/<(RULE_INPUT_ROOT).h',
            ],
            'message': 'Generating lut for <(RULE_INPUT_PATH)',
          },
        ]
      },
    ],
  }


gyp has no way to redirect stdout to a file, so redirect-stdout.sh just looks like this:

  #!/bin/sh
  exec $1 > $2

You would then make your jsc target depend on genlut and add "-I<(SHARED_INTERMEDIATE_DIR)/jsc_headers/" to the compiler flags.


This has two advantages about what you're doing:

1. You don't end up with a build system that shells out to another build system.
2. The generated source files are not generated in-tree but in the build output directory.


Depending on your personal taste, that's either not a big deal or way, way better than what you currently have :-)

[Martin Robinson]

(In reply to comment #19)
> Depending on your personal taste, that's either not a big deal or way, way better than what you currently have :-)

Thanks for the hint! I'll try to implement this soon.