As a first step of ConsoleMessage refactoring (https://bugs.webkit.org/show_bug.cgi?id=142599), rename ConsoleMessage and ConsoleMessageImpl to LegacyConsoleMessage and LegacyConsoleMessageImpl respectively. Rename WebInspector.ConsoleMessage class to WebInspector.LegacyConsoleMessage. Rename WebInspector.ConsoleMessageImpl class to WebInspector.LegacyConsoleMessageImpl. Rename ConsoleMessage.js file to LegacyConsoleMessage.js. Rename ConsoleMessageImpl file to LegacyConsoleMessageImpl.js.
Created attachment 248685 [details] Patch
Comment on attachment 248685 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=248685&action=review > Source/WebInspectorUI/UserInterface/Main.html:309 > - <script src="Views/ConsoleMessageImpl.js"></script> > + <script src="Views/LegacyConsoleMessageImpl.js"></script> Technically this should be sorted, but it doesn't matter since it will be removed later. > Source/WebInspectorUI/WebInspectorUI.vcxproj/WebInspectorUI.vcxproj:319 > + <None Include="..\UserInterface\LegacyConsoleMessage.js" /> > + <None Include="..\UserInterface\LegacyConsoleMessageImpl.js" /> Same here.
Comment on attachment 248685 [details] Patch Clearing flags on attachment: 248685 Committed r181517: <http://trac.webkit.org/changeset/181517>
All reviewed patches have been landed. Closing bug.
Looks like you broke GTK+! https://webkit-queues.appspot.com/results/5613271193223168
I searched via: ack -Q 'ConsoleMessageImpl.js' and I couldn’t find any references to the old files inside the WebKit repository. I’m not familiar with GTK build. Any idea what am I supposed to do fix this?
Re-opened since this is blocked by bug 142718
(In reply to comment #7) > Re-opened since this is blocked by bug 142718 Actually, I think the GTK bot should just need a clean build. It is picking up a file in the derived sources that, after this patch, will no longer exist. Unfortunately this did just get rolled out, so we can just roll it back in.
build error: ninja: error: '../../Source/WebInspectorUI/UserInterface/Views/ConsoleMessageImpl.js', needed by 'DerivedSources/webkit2gtk/InspectorGResourceBundle.xml', missing and no known rule to make it Ouch, you shouldn't have rolled out the patch, I already triggered clean build on the GTK bots long time ago and they were happy. The problem is that the mechanism in GTK build system to determine when should it rerun cmake to regenarate make/ninja files is buggy. I noticed the same InspectorUI related incremental build issue 2-3 times in the near past. We should add make GTK build to run cmake always if this part changes.
> Ouch, you shouldn't have rolled out the patch, I already triggered > clean build on the GTK bots long time ago and they were happy. Yeah, sorry. NVI came to me, I set the flag. We figured out what happened, I unset the flag, and moments later the commit-queue landed the patch. =(
(In reply to comment #9) > build error: > ninja: error: > '../../Source/WebInspectorUI/UserInterface/Views/ConsoleMessageImpl.js', > needed by 'DerivedSources/webkit2gtk/InspectorGResourceBundle.xml', missing > and no known rule to make it > > Ouch, you shouldn't have rolled out the patch, I already triggered > clean build on the GTK bots long time ago and they were happy. > > The problem is that the mechanism in GTK build system to determine > when should it rerun cmake to regenarate make/ninja files is buggy. > I noticed the same InspectorUI related incremental build issue 2-3 > times in the near past. We should add make GTK build to run cmake > always if this part changes. I checked the bug in details. The problem is that the generated InspectorGResourceBundle.xml depends on a non existent file (after rename). In this case cmake should rerun to regenerate the makefile. Maybe an additional check in webkitdirs.pm-shouldRemoveCMakeCache() could help. http://trac.webkit.org/browser/trunk/Source/WebKit2/PlatformGTK.cmake#L559 http://trac.webkit.org/browser/trunk/Source/WebKit2/PlatformGTK.cmake#L406
(In reply to comment #9) > build error: > ninja: error: > '../../Source/WebInspectorUI/UserInterface/Views/ConsoleMessageImpl.js', > needed by 'DerivedSources/webkit2gtk/InspectorGResourceBundle.xml', missing > and no known rule to make it > > Ouch, you shouldn't have rolled out the patch, I already triggered > clean build on the GTK bots long time ago and they were happy. > > The problem is that the mechanism in GTK build system to determine > when should it rerun cmake to regenarate make/ninja files is buggy. > I noticed the same InspectorUI related incremental build issue 2-3 > times in the near past. We should add make GTK build to run cmake > always if this part changes. Yes, the problem is the use of wildcard characters in cmake that I never liked. We could just list all the files in the mkefile, but then everytime a new file is added to the inspector and not explicitly added to the cmake files, we will miss it.
(In reply to comment #12) > Yes, the problem is the use of wildcard characters in cmake that I never > liked. We could just list all the files in the mkefile, but then everytime a > new file is added to the inspector and not explicitly added to the cmake > files, we will miss it. Or we could extract files from Source/WebInspectorUI/UserInterface/Main.html with a simple cmake script (with an additional sanity check if the file really exist and we didn't parsed a wrong thing).
Comment on attachment 248685 [details] Patch Rejecting attachment 248685 [details] from commit-queue. nvasilyev@apple.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
Comment on attachment 248685 [details] Patch Rejecting attachment 248685 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 248685, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in Source/WebInspectorUI/ChangeLog contains OOPS!. Full output: http://webkit-queues.appspot.com/results/5388747482857472
Created attachment 248788 [details] Patch
Comment on attachment 248788 [details] Patch Clearing flags on attachment: 248788 Committed r181610: <http://trac.webkit.org/changeset/181610>
(In reply to comment #9) > build error: > ninja: error: > '../../Source/WebInspectorUI/UserInterface/Views/ConsoleMessageImpl.js', > needed by 'DerivedSources/webkit2gtk/InspectorGResourceBundle.xml', missing > and no known rule to make it > > Ouch, you shouldn't have rolled out the patch, I already triggered > clean build on the GTK bots long time ago and they were happy. This just landed again, and GTK bots need the clean build. Could you trigger it? (Am I able to trigger it?)
> This just landed again, and GTK bots need the clean build. Could you trigger > it? (Am I able to trigger it?) I think I just triggered it via the build pages: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Build%29 I did a "Force Clean Build" on the Debug and Release bots.