[webkit-reviews] review denied: [Bug 103162] [cmake] Unify the way default features are enabled/disable on CMake based ports. : [Attachment 176028] Patch

[bugzilla-daemon at webkit.org]

Laszlo Gombos <laszlo.gombos at webkit.org> has denied Hugo Parente Lima
<hugo.lima at openbossa.org>'s request for review:
Bug 103162: [cmake] Unify the way default features are enabled/disable on CMake
based ports.
https://bugs.webkit.org/show_bug.cgi?id=103162

Attachment 176028: Patch
https://bugs.webkit.org/attachment.cgi?id=176028&action=review

------- Additional Comments from Laszlo Gombos <laszlo.gombos at webkit.org>
Thanks for your patch. This is hard problem to tackle that requires by in from
many. Overall I think the patch contains same great ideas, but I expect a few
more rounds of reviews.

r- for making the Source directory dependent on the Tools directory.

This patch moves the defaults from the Source directory into the Tools
directory. One of the project goals is that the Source directory should not be
dependent on Tools and the Source directory should produce a functional WebKit
library. It seems to me that this patch violates this goal. I think the we
should keep the defaults under the Source directory. I am not sure what is the
best directory under Source to store the defaults as that depends on which part
of the build system would actually read the defaults. Source/cmake is still
probably the safest bet.

I think we should explore the possibility of considering a different file
format for "FeatureDefaults" to enable the file not only processed by scripts
but also to be a proper .h file that could be included in the project source
directly. So instead of

"ENABLE_XXX = 1"

the file would esentially do the following (like in Platform.h)

#if !defined(ENABLE_XXX)
#define ENABLE_XXX 1
#endif

As a first step it make sense to only do this change for CMake based ports, but
I think we should choose a file format that enables the possibility for other
ports to use it in C/C++ code directly.