[webkit-reviews] review denied: [Bug 79498] [GTK] Webkit-gtk-1.7.90 tarball fails to build with MAKEOPTS=-jN : [Attachment 131681] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied  review:
Bug 79498: [GTK] Webkit-gtk-1.7.90 tarball fails to build with MAKEOPTS=-jN
https://bugs.webkit.org/show_bug.cgi?id=79498

Attachment 131681: Patch
https://bugs.webkit.org/attachment.cgi?id=131681&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=131681&action=review


Manually making the binaries depend on a convenience library that they
explicitly link against seems very weird. Ifpossible, split out those parts of
your patch into a different patch. I'd like to get the other fixes (that look
great, by the way) committed and focus on the stuff that looks kind of hacky
later.

> Source/WebKit2/GNUmakefile.am:1530
> +Programs/WebKitPluginProcess$(EXEEXT): libWebCoreGtk2.la

I'm still astounded that this should be necessary. What build failure occurs
when you omit them?

> Source/WebKit2/UIProcess/API/gtk/tests/GNUmakefile.am:116
> +Programs/WebKit2APITests/AccessibilityTestServer:
Libraries/libWebKit2APITestCore.la
> +Programs/WebKit2APITests/TestWebKitAccessibility:
Libraries/libWebKit2APITestCore.la

Ditto and you are missing the EXE suffix variable as well.

> Source/WebKit2/UIProcess/API/gtk/tests/GNUmakefile.am:153
> +Programs/WebKit2APITests/TestPrinting: Libraries/libWebKit2APITestCore.la
> +Programs/WebKit2APITests/TestWebKitWebContext:
Libraries/libWebKit2APITestCore.la
> +Programs/WebKit2APITests/TestWebKitWebView:
Libraries/libWebKit2APITestCore.la
> +Programs/WebKit2APITests/TestLoaderClient:
Libraries/libWebKit2APITestCore.la
> +Programs/WebKit2APITests/TestWebKitSettings:
Libraries/libWebKit2APITestCore.la
> +Programs/WebKit2APITests/TestBackForwardList:
Libraries/libWebKit2APITestCore.la
> +Programs/WebKit2APITests/TestDownloads: Libraries/libWebKit2APITestCore.la
> +Programs/WebKit2APITests/TestWebKitPolicyClient:
Libraries/libWebKit2APITestCore.la
> +Programs/WebKit2APITests/TestWebViewEditor:
Libraries/libWebKit2APITestCore.la
> +Programs/WebKit2APITests/TestWebKitFindController:
Libraries/libWebKit2APITestCore.la

Ditto.