140608 2015-01-19 01:18:28 -0800 REGRESSION(r178527): It broke the !ENABLE(INSPECTOR) build 2015-01-21 09:39:02 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED WONTFIX P1 Critical --- 140478 1 ossy webkit-unassigned ap burg joepeck ossy timothy oldest_to_newest 1062291 0 ossy 2015-01-19 01:18:28 -0800 ../../Source/WebCore/page/PageConsoleClient.cpp: In member function 'void WebCore::PageConsoleClient::addMessage(JSC::MessageSource, JSC::MessageLevel, const WTF::String&, const WTF::String&, unsigned int, unsigned int, WTF::RefPtr<Inspe ctor::ScriptCallStack>&&, JSC::ExecState*, long unsigned int)': ../../Source/WebCore/page/PageConsoleClient.cpp:127:25: error: invalid use of incomplete type 'class Inspector::ConsoleMessage' In file included from ../../Source/WebCore/inspector/InspectorConsoleInstrumentation.h:34:0, from ../../Source/WebCore/page/PageConsoleClient.cpp:36: ../../Source/WebCore/inspector/InspectorInstrumentation.h:60:7: error: forward declaration of 'class Inspector::ConsoleMessage' ../../Source/WebCore/page/PageConsoleClient.cpp:128:34: error: invalid use of incomplete type 'class Inspector::ConsoleMessage' In file included from ../../Source/WebCore/inspector/InspectorConsoleInstrumentation.h:34:0, from ../../Source/WebCore/page/PageConsoleClient.cpp:36: ../../Source/WebCore/inspector/InspectorInstrumentation.h:60:7: error: forward declaration of 'class Inspector::ConsoleMessage' ../../Source/WebCore/page/PageConsoleClient.cpp:129:36: error: invalid use of incomplete type 'class Inspector::ConsoleMessage' In file included from ../../Source/WebCore/inspector/InspectorConsoleInstrumentation.h:34:0, from ../../Source/WebCore/page/PageConsoleClient.cpp:36: ../../Source/WebCore/inspector/InspectorInstrumentation.h:60:7: error: forward declaration of 'class Inspector::ConsoleMessage' ... The whole ConsoleMessage.h is guarded with ENABLE(INSPECTOR), but it is ununsed without guard in PageConsoleClient.cpp. 1062311 1 ossy 2015-01-19 02:18:47 -0800 Does r178527 mean that inspector is mandatory feature since now? If yes, let's remove the compile time guards and make it really mandatory. If no, could you possibly give me any hint how to fix the build properly? 1062371 2 ap 2015-01-19 09:22:05 -0800 Given that we use the logic in Inspector::ConsoleMessage even when inspector is not enabled, yes, this logic needs to be always compiled in. I haven't looked into how much of other Inspector code it uses. Perhaps it can be factored out of Inspector::ConsoleMessage. What is the rationale for disabling Inspector? The !ENABLE(INSPECTOR) build appears to be very high maintenance. 1062373 3 burg 2015-01-19 09:30:25 -0800 (In reply to comment #1) > Does r178527 mean that inspector is mandatory feature since now? > > If yes, let's remove the compile time guards and make it really mandatory. > If no, could you possibly give me any hint how to fix the build properly? It seems that these are the only methods that actually require ENABLE(INSPECTOR), and are entry points from InspectorConsoleAgent: void addToFrontend(InspectorConsoleFrontendDispatcher*, InjectedScriptManager*, bool generatePreview); void updateRepeatCountInConsole(InspectorConsoleFrontendDispatcher*); Plus IdentifiersFactory.h which looks to have no other Inspector dependencies. So you would need to remove the guard of the entire file and only surround those methods (and their static helper functions in the .cpp file). Honestly, someone should request to webkit-dev to remove ENABLE(INSPECTOR) guards. There are no bots running this configuration, and I would be surprised if anyone contributing to trunk depends on it. If such a person exists, I would have expected to hear about these build breaks before Ossy found it via --minimal. 1062374 4 ossy 2015-01-19 09:31:36 -0800 (In reply to comment #2) > Given that we use the logic in Inspector::ConsoleMessage even when inspector > is not enabled, yes, this logic needs to be always compiled in. I haven't > looked into how much of other Inspector code it uses. Perhaps it can be > factored out of Inspector::ConsoleMessage. > > What is the rationale for disabling Inspector? The !ENABLE(INSPECTOR) build > appears to be very high maintenance. It can be useful to reduce the binary codesize on embedded systems where you don't need inspector at all. ( https://bugs.webkit.org/show_bug.cgi?id=140098#c5 ) I don't have strong opinion about it, but I don't like broken and unmaintained ifdef guards. If we want to have a guard, we should maintain it, if we don't want to maintain it, let's remove the guard. 1062375 5 ossy 2015-01-19 09:32:26 -0800 (In reply to comment #3) > Honestly, someone should request to webkit-dev to remove ENABLE(INSPECTOR) > guards. There are no bots running this configuration, and I would be > surprised if anyone contributing to trunk depends on it. If such a person > exists, I would have expected to hear about these build breaks before Ossy > found it via --minimal. Will send soon. 1062377 6 ossy 2015-01-19 09:45:01 -0800 webkit-dev mail sent: https://lists.webkit.org/pipermail/webkit-dev/2015-January/027153.html 1063080 7 ossy 2015-01-21 09:39:02 -0800 There is no !INSPECTOR build since http://trac.webkit.org/changeset/178820 .