WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
163774
[CMake][Win] Visual Studio invokes make_settings.pl twice
https://bugs.webkit.org/show_bug.cgi?id=163774
Summary
[CMake][Win] Visual Studio invokes make_settings.pl twice
Fujii Hironori
Reported
2016-10-20 22:57:40 PDT
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)
Attachments
screenshot of Visual Studio
(68.11 KB, image/png)
2016-10-20 22:57 PDT
,
Fujii Hironori
no flags
Details
Patch
(3.45 KB, patch)
2016-10-21 04:23 PDT
,
Fujii Hironori
no flags
Details
Formatted Diff
Diff
Patch
(3.60 KB, patch)
2016-10-24 02:03 PDT
,
Fujii Hironori
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Fujii Hironori
Comment 1
2016-10-21 02:35:39 PDT
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
Comment 2
2016-10-21 03:54:50 PDT
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
Comment 3
2016-10-21 04:23:12 PDT
Created
attachment 292342
[details]
Patch
Alex Christensen
Comment 4
2016-10-21 09:53:20 PDT
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
Comment 5
2016-10-24 01:09:50 PDT
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
Comment 6
2016-10-24 02:03:12 PDT
Created
attachment 292596
[details]
Patch
Michael Catanzaro
Comment 7
2016-10-29 08:42:38 PDT
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 8
2016-10-29 09:06:18 PDT
Comment on
attachment 292596
[details]
Patch Clearing flags on attachment: 292596 Committed
r208104
: <
http://trac.webkit.org/changeset/208104
>
WebKit Commit Bot
Comment 9
2016-10-29 09:06:23 PDT
All reviewed patches have been landed. Closing bug.
Fujii Hironori
Comment 10
2016-10-30 19:10:19 PDT
***
Bug 155872
has been marked as a duplicate of this bug. ***
Fujii Hironori
Comment 11
2016-10-30 19:10:35 PDT
***
Bug 154938
has been marked as a duplicate of this bug. ***
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug