Bug 164054

Summary: generate-bindings-all.pl should recompile supplemented IDL if its supplemental IDL are added or removed
Product: WebKit Reporter: Fujii Hironori <fujii.hironori>
Component: Tools / TestsAssignee: Fujii Hironori <fujii.hironori>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, cdumez, commit-queue, gyuyoung.kim, lforschler, mcatanzaro
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 161433    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Fujii Hironori
Reported 2016-10-27 02:46:47 PDT
generate-bindings-all.pl was introduced in <http://trac.webkit.org/changeset/207617>. Before this change, updating supplemental_dependency.tmp triggered regenerating all bindings. I tried to solve this problem by checking timestamp of supplemental dependencies. But, its logic has bugs in following cases: * Adding a new supplemental IDL which has older timestamp than supplemented IDL's generated files. ⇒ Supplemented IDL should be processed * Removing a supplemental IDL ⇒ Supplemented IDL should be processed * Changing enabled features ⇒ All IDL should be processed
Attachments
Patch (6.30 KB, patch)
2016-10-27 03:24 PDT, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Patch (6.63 KB, patch)
2016-10-27 21:34 PDT, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Patch (4.51 KB, patch)
2016-10-28 04:10 PDT, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Fujii Hironori
Comment 1 2016-10-27 02:51:50 PDT
*** Bug 163883 has been marked as a duplicate of this bug. ***
Fujii Hironori
Comment 2 2016-10-27 03:24:26 PDT
Created attachment 293008 [details] Patch
Fujii Hironori
Comment 3 2016-10-27 03:53:30 PDT
Manual test cases of incremental build: * touch Source/WebCore/dom/Comment.idl ⇒ Recompile Comment.idl * touch Source/WebCore/html/HTMLMediaElement.idl ⇒ Recompile HTMLMediaElement.idl, HTMLAudioElement.idl and HTMLVideoElement.idl * touch Source/WebCore/dom/Node.idl ⇒ Recompile a lot of IDL including HTML Elements and SVG Elements. * touch Source/WebCore/Modules/webdatabase/DOMWindowWebDatabase.idl ⇒ Recompile DOMWindowWebDatabase.idl and DOMWindow.idl * Comment out Modules/webdatabase/DOMWindowWebDatabase.idl in Source/WebCore/CMakeLists.txt ⇒ Recompile DOMWindow.idl * Remove the comment of Modules/webdatabase/DOMWindowWebDatabase.idl ⇒ Recompile DOMWindow.idl * Add "[NoInterfaceObject]" to Source/WebCore/Modules/webdatabase/Database.idl ⇒ Recompile Database.idl and DOMWindow.idl (DOMWindowConstructors.idl is updated) * Removed the "[NoInterfaceObject]" of Source/WebCore/Modules/webdatabase/Database.idl ⇒ Recompile Database.idl and DOMWindow.idl (DOMWindowConstructors.idl is updated) * Enable ro disable features ⇒ Recompile all IDL
Michael Catanzaro
Comment 4 2016-10-27 06:41:02 PDT
Did you mean to set r?
Fujii Hironori
Comment 5 2016-10-27 07:04:42 PDT
I just wanted to try EWS. I need more testing before requesting review. Thank you.
Fujii Hironori
Comment 6 2016-10-27 21:31:43 PDT
I've found one more problem that preprocess-idls.pl is not invoked when a IDL is added or removed. I didn't notice this bug because Bug163882 causes infinite preprocessing. To test this case, I need to stop infinite preprocessing first. > $ touch WebKitBuild/Debug/DerivedSources/WebCore/{supplemental_dependency.tmp,*Constructors.idl} > $ ./Tools/Scripts/build-webkit --gtk --debug --64-bit --makeargs="WebCoreBindings" And, do following test cases: > * Comment out Modules/webdatabase/DOMWindowWebDatabase.idl in Source/WebCore/CMakeLists.txt > ⇒ Recompile DOMWindow.idl > * Remove the comment of Modules/webdatabase/DOMWindowWebDatabase.idl > ⇒ Recompile DOMWindow.idl
Fujii Hironori
Comment 7 2016-10-27 21:34:35 PDT
Created attachment 293114 [details] Patch
Michael Catanzaro
Comment 8 2016-10-28 03:45:13 PDT
Comment on attachment 293114 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=293114&action=review > Source/WebCore/ChangeLog:13 > + generetated files. generetated -> generated > Source/cmake/WebKitMacros.cmake:110 > + # because some of them use #if yet. Just say: "because some of them use #if" I would prefer to split this change into a separate patch.
Fujii Hironori
Comment 9 2016-10-28 04:10:03 PDT
Created attachment 293141 [details] Patch Thank you for reviewing my patch. I revised the patch. * Fixed the typo * Removed the change of WebKitMacros.cmake
WebKit Commit Bot
Comment 10 2016-10-28 05:39:25 PDT
Comment on attachment 293141 [details] Patch Clearing flags on attachment: 293141 Committed r208045: <http://trac.webkit.org/changeset/208045>
WebKit Commit Bot
Comment 11 2016-10-28 05:39:30 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.