WebKit should build successfully even with -DENABLE_UNIFIED_BUILDS=OFF
Created attachment 367273 [details] Patch (with linking errors for Tools on WinCairo)
Confirmed that this patch builds successfully on GTK. Yay! Using lld as a linker, this patch almost builds successfully on WinCairo, but some strange linking errors occur at the very end: > lld-link.exe: error: undefined symbol: __imp_?toJS@WebCore@@YA?AVJSValue@JSC@@PEAVExecState@3@PEAVJSDOMGlobalObject@1@AEAVTextTrackCueGeneric@1@@Z > >>> referenced by C:\GitHub\neko\WebKitBuild\Release\DerivedSources\WebCore\JSInternals.cpp:6803 > >>> WebCoreTestSupport.lib(JSInternals.cpp.obj):(?jsInternalsPrototypeFunctionCreateGenericCue@WebCore@@YA_JPEAVExecState@JSC@@@Z) > > > lld-link.exe: error: undefined symbol: ?notifyNetworkStateChange@ServiceWorkerThreadProxy@WebCore@@QEAAX_N@Z > >>> referenced by C:\GitHub\neko\WebKitBuild\Release\WTF\Headers\wtf\Function.h:102 > >>> WebCoreTestSupport.lib(ServiceWorkerInternals.cpp.obj):(?call@?$CallableWrapper@V<lambda_0>@?0??setOnline@ServiceWorkerInternals@WebCore@@QEAAX_N@Z@@?$Function@$$A6AXXZ@WTF@@UEAAXXZ) > > > lld-link.exe: error: undefined symbol: __imp_?parseHEVCCodecParameters@WebCore@@YA?AV?$Optional@UHEVCParameterSet@WebCore@@@WTF@@AEBVString@3@@Z > >>> referenced by C:\GitHub\neko\Source\WebCore\testing\Internals.cpp:4962 > >>> WebCoreTestSupport.lib(Internals.cpp.obj):(?parseHEVCCodecParameters@Internals@WebCore@@QEAA?AV?$Optional@UHEVCParameterSet@WebCore@@@WTF@@AEBVString@4@@Z) > > > lld-link.exe: error: undefined symbol: __imp_?doDispatchMessageOnFrontendPage@InspectorClient@WebCore@@SAXPEAVPage@2@AEBVString@WTF@@@Z > >>> referenced by C:\GitHub\neko\Source\WebCore\testing\Internals.cpp:372 > >>> WebCoreTestSupport.lib(Internals.cpp.obj):(?sendMessageToFrontend@InspectorStubFrontend@WebCore@@EEAAXAEBVString@WTF@@@Z) I spent the entire afternoon trying to understand how these are possible but I remain confounded.
Created attachment 367274 [details] Patch (fails to link Tools on WinCairo)
Comment on attachment 367274 [details] Patch (fails to link Tools on WinCairo) View in context: https://bugs.webkit.org/attachment.cgi?id=367274&action=review > Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:325 > -inline void CanvasRenderingContext2DBase::FontProxy::initialize(FontSelector& fontSelector, const RenderStyle& newStyle) > +void CanvasRenderingContext2DBase::FontProxy::initialize(FontSelector& fontSelector, const RenderStyle& newStyle) Why are you removing inline keywords here? That seems unrelated. > Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.h:29 > +#include <wtf/Forward.h> Why do we need Forward.h?
(In reply to Ryosuke Niwa from comment #4) > Comment on attachment 367274 [details] > Patch (fails to link Tools on WinCairo) > > View in context: > https://bugs.webkit.org/attachment.cgi?id=367274&action=review > > > Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:325 > > -inline void CanvasRenderingContext2DBase::FontProxy::initialize(FontSelector& fontSelector, const RenderStyle& newStyle) > > +void CanvasRenderingContext2DBase::FontProxy::initialize(FontSelector& fontSelector, const RenderStyle& newStyle) > > Why are you removing inline keywords here? That seems unrelated. All of them are demanded by linking errors like this: lib/libWebCoreGTK.a(lib/../Source/WebCore/CMakeFiles/WebCore.dir/html/canvas/CanvasRenderingContext2D.cpp.o):CanvasRenderingContext2D.cpp:function WebCore::CanvasRenderingContext2D::font() const: error: undefined reference to 'WebCore::CanvasRenderingContext2DBase::FontProxy::fontDescription() const' Seems that an inline specifier in an .cpp file interferes with the creation of a symbol. > > Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.h:29 > > +#include <wtf/Forward.h> > > Why do we need Forward.h? We need forward declarations for String and CompletionHandler -- I thought this was the preferred style?
(In reply to Ross Kirsling from comment #5) > (In reply to Ryosuke Niwa from comment #4) > > Comment on attachment 367274 [details] > > Patch (fails to link Tools on WinCairo) > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=367274&action=review > > > > > Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:325 > > > -inline void CanvasRenderingContext2DBase::FontProxy::initialize(FontSelector& fontSelector, const RenderStyle& newStyle) > > > +void CanvasRenderingContext2DBase::FontProxy::initialize(FontSelector& fontSelector, const RenderStyle& newStyle) > > > > Why are you removing inline keywords here? That seems unrelated. > > All of them are demanded by linking errors like this: > > lib/libWebCoreGTK.a(lib/../Source/WebCore/CMakeFiles/WebCore.dir/html/canvas/ > CanvasRenderingContext2D.cpp.o):CanvasRenderingContext2D.cpp:function > WebCore::CanvasRenderingContext2D::font() const: error: undefined reference > to 'WebCore::CanvasRenderingContext2DBase::FontProxy::fontDescription() > const' > > Seems that an inline specifier in an .cpp file interferes with the creation > of a symbol. I see. > > > Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.h:29 > > > +#include <wtf/Forward.h> > > > > Why do we need Forward.h? > > We need forward declarations for String and CompletionHandler -- I thought > this was the preferred style? Okay. I just wanted the clarification as to why it's needed. I think a comment explaining why in the change log helps.
Absolutely, I'll make the ChangeLog more informative before landing. Thanks!
Opened bug 196866 to track the issue mentioned in comment 2.
Created attachment 367331 [details] Patch for landing
Comment on attachment 367331 [details] Patch for landing Clearing flags on attachment: 367331 Committed r244225: <https://trac.webkit.org/changeset/244225>
All reviewed patches have been landed. Closing bug.
<rdar://problem/49857545>