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

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Philippe Normand
<pnormand at igalia.com>'s request for 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 129240: Patch
https://bugs.webkit.org/attachment.cgi?id=129240&action=review

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


There's a lot of stuff going on in this patch... Are these all required to fix
this particular issue or is it just extra left over from experimentation?

> Source/WebKit2/GNUmakefile.am:1517
> +Programs/WebKitPluginProcess: libWebCoreGtk2.la

This needs to include the EXE suffix.

> Source/WebKit2/UIProcess/API/gtk/tests/GNUmakefile.am:144
> +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

I'm really suspicious of this. Libraries/libWebKit2APITestCore.la is already in
webkit2_tests_ldadd, so there should already be an explicit dependency.

> Tools/GNUmakefile.am:41
> +webcore_internal_sources = \

Better call this libwebcoreinternals_sources or rename
libwebcoreinternals_built_sources to webcoreinternals_built_sources and call
this webcoreinternals_sources.

> Tools/GNUmakefile.am:60
> +webcore_internal_built_sources = \
>	DerivedSources/WebCore/JSInternals.cpp \
>	DerivedSources/WebCore/JSInternals.h   \
>	DerivedSources/WebCore/JSInternalSettings.cpp \
>	DerivedSources/WebCore/JSInternalSettings.h
>  
> +libwebcoreinternals_built_sources = $(webcore_internal_built_sources)

Why add webcore_internal_built_sources, when libwebcoreinternals_built_sources
already exists?