Bug 109360 – [GTK] Connect the gyp build to autoconf

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Martin Robinson]

Autoconf helps us find packages and figure out compiler flags. We should connect the gyp build to autoconf.

[Martin Robinson]

Created attachment 187486 [details]
Patch

[Build Bot]

Comment on attachment 187486 [details]
Patch

Attachment 187486 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16477856

New failing tests:
compositing/checkerboard.html
accessibility/anchor-linked-anonymous-block-crash.html
http/tests/cache/cancel-multiple-post-xhrs.html
animations/3d/state-at-end-event-transform.html
animations/animation-add-events-in-handler.html
animations/3d/replace-filling-transform.html
http/tests/cache/history-only-cached-subresource-loads.html
compositing/bounds-in-flipped-writing-mode.html
accessibility/accessibility-node-reparent.html
animations/animation-border-overflow.html
accessibility/accessibility-object-detached.html
accessibility/anonymous-render-block-in-continuation-causes-crash.html
animations/animation-controller-drt-api.html
compositing/absolute-position-changed-with-composited-parent-layer.html
compositing/absolute-position-changed-in-composited-layer.html
http/tests/cache/iframe-304-crash.html
animations/3d/transform-perspective.html
http/tests/cache/cancel-during-failure-crash.html
canvas/philip/tests/2d.clearRect+fillRect.alpha0.5.html
canvas/philip/tests/2d.clearRect.basic.html
animations/3d/transform-origin-vs-functions.html
animations/animation-css-rule-types.html
canvas/philip/tests/2d.clearRect+fillRect.basic.html
canvas/philip/tests/2d.clearRect+fillRect.alpha0.html

[Martin Robinson]

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

I'm fairly certain this is unrelated. :)

[Dirk Pranke]

Comment on attachment 187486 [details]
Patch

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

This basically looks okay to me, at least from the gyp-side. I'm not much of an autotools guru and don't know the gtk build, so I'm less comfortable r+'ing the autotools side of things and would appreciate someone else more familiar w/ that aspect taking a look.

> Source/WebKit/gtk/gyp/WTF.gyp:-31
> -    {

As we discussed on #webkit, I think it might make more sense to leave these as separate targets. In my experience, you want the dependent targets to just be able to say "I depend on icu" and leave the details of what that means to the maintainer of the icu target, rather than embedding the dependencies into the dependents directly.

[Martin Robinson]

Comment on attachment 187486 [details]
Patch

There are some issues with this one.

[Martin Robinson]

Created attachment 187906 [details]
Use gyp dependencies again and fix an issue with re-running gyp

[Dirk Pranke]

Comment on attachment 187906 [details]
Use gyp dependencies again and fix an issue with re-running gyp

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

looks fine. I'll r+ this for now since no one commented on the autotools stuff but I doubt it'll break anything so we can always fix it down the road.

> Source/WebKit/gtk/gyp/Configuration.gypi.in:22
> +    ],

If you are going to want these defined in every (C/C++) target, you might consider using a 'target_defaults' block here so that you don't have to repeat yourself all over the place. See an example in Chromium's src/build/common.gypi.

> Source/WebKit/gtk/gyp/Configuration.gypi.in:26
> +    # '.' to the directory containing the gyp file.

"the" gyp file? which gyp file? The top-level one, as passed to run-gyp, or *every* gyp file? I think it's actually the latter, but I'm not sure. At any rate, the comment should be more explicit, since this is one of the less-clear aspects of gyp. In the current patch, all of your gyp files are in the same directory, but that'll likely change as you get more of the project working.

[Martin Robinson]

(In reply to comment #7)

Thanks for the review! This shouldn't break anything in the existing build, since it doesn't share anything.

> > Source/WebKit/gtk/gyp/Configuration.gypi.in:22
> > +    ],
> 
> If you are going to want these defined in every (C/C++) target, you might consider using a 'target_defaults' block here so that you don't have to repeat yourself all over the place. See an example in Chromium's src/build/common.gypi.

Oh great! I was pretty sure there was a way to do this, but it wasn't obvious from the documentation. I'll do this before landing.

> 
> > Source/WebKit/gtk/gyp/Configuration.gypi.in:26
> > +    # '.' to the directory containing the gyp file.
> 
> "the" gyp file? which gyp file? The top-level one, as passed to run-gyp, or *every* gyp file? I think it's actually the latter, but I'm not sure. At any rate, the comment should be more explicit, since this is one of the less-clear aspects of gyp. In the current patch, all of your gyp files are in the same directory, but that'll likely change as you get more of the project working.

All the gyp files are in the same directory now except for Configuration.gyp, which is realized in build directory by the configure script. The '.' does not seem to be relative to Configuration.gypi, but instead relative to the file that uses the variable in a list of includes. So it seems like the code in gyp that handles the list of includes understands the semantic meaning of '.', but does not trace back to where that variable was defined. I'll clarify the comment.

[Martin Robinson]

Committed r142706: <http://trac.webkit.org/changeset/142706>