[CMake] Decouple generating bindings of WebCore and WebCoreTestSupport Generating bindings of WebCore and WebCoreTestSupport shares a single supplementalDependencyFile. But, nothing supplements any IDL of WebCoreTestSupport. This introduces unnecessary dependencies. Generating bindings of WebCoreTestSupport does not need a supplementalDependencyFile at the moment.
Created attachment 287601 [details] Patch
win-ews failed: > C:\cygwin\home\buildbot\WebKit\Source\WebCore\page\Settings.h(33): fatal error C1083: Cannot open include file: 'SettingsMacros.h': No such file or directory (compiling source file C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\WebCore\StyleBuilder.cpp) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCoreDerivedSources.vcxproj] > C:\cygwin\home\buildbot\WebKit\Source\WebCore\page\Settings.h(33): fatal error C1083: Cannot open include file: 'SettingsMacros.h': No such file or directory (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\DerivedSources.cpp) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCoreDerivedSources.vcxproj] WebCoreDerivedSources did not have a dependency to SettingsMacros.h. According to the CMake document, I need to add SettingsMacros.h as the source file. https://cmake.org/cmake/help/v3.0/prop_sf/OBJECT_DEPENDS.html > This property need not be used to specify the dependency of a source > file on a generated header file that it includes. Although the > property was originally introduced for this purpose, it is no longer > necessary. If the generated header file is created by a custom command > in the same target as the source file, the automatic dependency > scanning process will recognize the dependency. If the generated > header file is created by another target, an inter-target dependency > should be created with the add_dependencies command (if one does not > already exist due to linking relationships).
Created attachment 287609 [details] Patch
What is the motivation for this change? What does it fix?
There are three problems: 1) This unnecessary dependency causes unnecessary recompilation Updating supplementalDependencyFile triggers unnecessarily regenerating bindings of WebCoreTestSupport. This is actually a tiny problem because WebCoreTestSupport doesn't have many IDL files. 2) VisualStudio projects generated by CMake triggers preprocess-idls.pl twice Both WebCoreDerivedSources and WebCoreTestSupport projects trigger preprocess-idls.pl because they depend on it. This seems to cause following problems: Bug 155872 – [Win] CMake seems to build all generated files every time Bug 154938 – Generated sources triggers when not required Unfortunately, This this patch solve this problem because both WebCoreDerivedSources and WebCoreTestSupport projects still trigger make-settings.pl. But, I think this is the right direction to go. 3) Unnecessarily complicated This change can simplify the build rule. I have a plan to generate bindings outside of CMake. This change can help so much. Bug 161433 – [CMake] CMake does not support the dep file of preprocess-idls.pl --supplementalMakefileDeps
Comment on attachment 287609 [details] Patch Wish I was more of a CMake expert. This patch looks great, but I don’t think I am qualified to say review+
Comment on attachment 287609 [details] Patch Clearing flags on attachment: 287609 Committed r205512: <http://trac.webkit.org/changeset/205512>
All reviewed patches have been landed. Closing bug.