Bug 163774 – [CMake][Win] Visual Studio invokes make_settings.pl twice

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Fujii Hironori]

Created attachment 292322 [details]
screenshot of Visual Studio

WebCoreDerivedSources.vcxproj and WebCoreTestSupport.vcxproj triggers make_settings.pl.
This causes unnecessary recompilation in CMake VisualStudio build.

make_settings.pl generates following files:

- SettingsMacros.h
- InternalSettingsGenerated.h
- InternalSettingsGenerated.cpp
- InternalSettingsGenerated.idl

WebCoreDerivedSources depends on SettingsMacros.h.
WebCoreTestSupport depends on InternalSettingsGenerated.cpp.

This problem is described in CMake document:

https://cmake.org/cmake/help/v3.0/command/add_custom_command.html

> Do not list the output in more than one independent target that may
> build in parallel or the two instances of the rule may conflict
> (instead use add_custom_target to drive the command and make the other
> targets depend on that one).

There are some solutions:

1) Split make_settings.pl
   For example, add some command arguments like following:
    'make_settings.pl --macros-header ...' generates SettingsMacros.h.
    'make_settings.pl --internal-idl ...' generates InternalSettingsGenerated.idl.
    'make_settings.pl --internal-source ...' generates InternalSettingsGenerated.cpp and InternalSettingsGenerated.h.
2) Write output files only if changed
   Use WriteFileIfChanged of preprocess-idls.pl
   WriteFileIfChanged has a bad effect (infinite regeneration, see Bug 131756)
3) Use add_custom_target
   But, add_custom_target invokes the command every time, I need to
   implement timestamp check in CMake or Perl
4) Implement timestamp check in make_settings.pl
5) Implement timestamp check in a new script (CMake, Python or Perl)

[Fujii Hironori]

Here is my three incremental build logs of WebKit WinCairo port CMake VisualStudio build.

  https://gist.github.com/fujii/7ae2c8928768647b0f26633e7951189a

In first build, make_settings.pl was invoked twice:

>  Generating ../../DerivedSources/WebCore/SettingsMacros.h, ../../DerivedSources/WebCore/InternalSettingsGenerated.h, ../../DerivedSources/WebCore/InternalSettingsGenerated.cpp, ../../DerivedSources/WebCore/InternalSettingsGenerated.idl
(...)
>  Generating ../../DerivedSources/WebCore/SettingsMacros.h, ../../DerivedSources/WebCore/InternalSettingsGenerated.h, ../../DerivedSources/WebCore/InternalSettingsGenerated.cpp, ../../DerivedSources/WebCore/InternalSettingsGenerated.idl

In second build, recompiled some files unnecessarily
In third build, did nothing.

[Fujii Hironori]

Another idea is coming to my mind:

7) Specify only SettingsMacros.h as OUTPUT of add_custom_command
   Mark other generated files (InternalSettingsGenerated.{cpp,h,idl}) GENERATED.

This is so simple. I'll create a patch of this approach.

[Fujii Hironori]

Created attachment 292342 [details]
Patch

[Alex Christensen]

Comment on attachment 292342 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=292342&action=review

> Source/cmake/WebKitMacros.cmake:190
> +    if (${CMAKE_GENERATOR} MATCHES "Visual Studio")

I don't like this.  The solution should be the same for all generators.

[Fujii Hironori]

Thank you for reviewing my patch.

(In reply to comment #4)
> I don't like this.  The solution should be the same for all generators.

Unfortunately, I need a trick to do so for Ninja.
BYPRODUCTS option is needed to work Ninja incremental build fine.
But, BYPRODUCTS option is available since CMake 3.2.
And, we need to support less version of CMake 3.2 at the moment.

I'll create a patch.

[Fujii Hironori]

Created attachment 292596 [details]
Patch

[Michael Catanzaro]

Comment on attachment 292596 [details]
Patch

I've been bitten by basically the same problem twice recently in Automake projects. Glad this didn't require any stamp files or magic to solve.

[WebKit Commit Bot]

Comment on attachment 292596 [details]
Patch

Clearing flags on attachment: 292596

Committed r208104: <http://trac.webkit.org/changeset/208104>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Fujii Hironori]

*** Bug 155872 has been marked as a duplicate of this bug. ***

[Fujii Hironori]

*** Bug 154938 has been marked as a duplicate of this bug. ***