Bug 161474 – [CMake] Decouple generating bindings of WebCore and WebCoreTestSupport

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Fujii Hironori]

[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.

[Fujii Hironori]

Created attachment 287601 [details]
Patch

[Fujii Hironori]

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).

[Fujii Hironori]

Created attachment 287609 [details]
Patch

[Alex Christensen]

What is the motivation for this change?  What does it fix?

[Fujii Hironori]

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

[Darin Adler]

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+

[WebKit Commit Bot]

Comment on attachment 287609 [details]
Patch

Clearing flags on attachment: 287609

Committed r205512: <http://trac.webkit.org/changeset/205512>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.