When options are presented to the user they are presented in the form USE_FOO, but the WTF USE(...) macro expects WTF_USE_FOO. We can automatically expose WTF_USE_FOO to make some things simpler.
Created attachment 251964 [details] Patch
Nits: You should mention in the changelog that this fixes USE_REDIRECTED_XCOMPOSITE_WINDOW after yesterday's commit, since that's a significant consequence of the change. Also, the comment "when we forget" implies that exposing WTF_USE_WHATEVER is best practice, but after this change I don't see any reason to do that anymore.
Created attachment 251966 [details] Patch
(In reply to comment #2) > Nits: > > You should mention in the changelog that this fixes > USE_REDIRECTED_XCOMPOSITE_WINDOW after yesterday's commit, since that's a > significant consequence of the change. > > Also, the comment "when we forget" implies that exposing WTF_USE_WHATEVER is > best practice, but after this change I don't see any reason to do that > anymore. Thanks for the comments. I have updated the patch with your suggestions.
Comment on attachment 251966 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251966&action=review > Source/cmake/OptionsGTK.cmake:378 > set(DERIVED_SOURCES_GOBJECT_DOM_BINDINGS_DIR ${DERIVED_SOURCES_DIR}/webkitdom) I guess you can remove SET_AND_EXPOSE_TO_BUILD(WTF_USE_LIBHYPHEN 1) > Source/cmake/WebKitFeatures.cmake:314 > + _ADD_CONFIGURATION_LINE_TO_HEADER_STRING(_file_contents ${_variable_name} ${_variable_name}) > + > + # WebKit looks for WTF_USE_FOO when calling the USE(FOO) macro. Automatically exposing > + # this definition when USE_FOO is exposed, prevents us from having to expose both (or > + # forgetting to do so). > + string(FIND ${_variable_name} "USE_" _use_string_location) > + if (${_use_string_location} EQUAL 0) > + _ADD_CONFIGURATION_LINE_TO_HEADER_STRING(_file_contents ${_variable_name} "WTF_${_variable_name}") This duplicates the USE macros in the cmakeconfig.h, with and without the WTF prefix, but I guess only the prefixed one is useful. I guess we could move the previous _ADD_CONFIGURATION_LINE_TO_HEADER_STRING to and else() here.
(In reply to comment #5) > Comment on attachment 251966 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=251966&action=review > > > Source/cmake/OptionsGTK.cmake:378 > > set(DERIVED_SOURCES_GOBJECT_DOM_BINDINGS_DIR ${DERIVED_SOURCES_DIR}/webkitdom) > > I guess you can remove SET_AND_EXPOSE_TO_BUILD(WTF_USE_LIBHYPHEN 1) > Yes, this patch was written before libhyphen integration was complete. :) > > Source/cmake/WebKitFeatures.cmake:314 > > + _ADD_CONFIGURATION_LINE_TO_HEADER_STRING(_file_contents ${_variable_name} ${_variable_name}) > > + > > + # WebKit looks for WTF_USE_FOO when calling the USE(FOO) macro. Automatically exposing > > + # this definition when USE_FOO is exposed, prevents us from having to expose both (or > > + # forgetting to do so). > > + string(FIND ${_variable_name} "USE_" _use_string_location) > > + if (${_use_string_location} EQUAL 0) > > + _ADD_CONFIGURATION_LINE_TO_HEADER_STRING(_file_contents ${_variable_name} "WTF_${_variable_name}") > > This duplicates the USE macros in the cmakeconfig.h, with and without the > WTF prefix, but I guess only the prefixed one is useful. I guess we could > move the previous _ADD_CONFIGURATION_LINE_TO_HEADER_STRING to and else() > here. I waffled a bit on this, so your approach is probably fine too.
(In reply to comment #6) > > This duplicates the USE macros in the cmakeconfig.h, with and without the > > WTF prefix, but I guess only the prefixed one is useful. I guess we could > > move the previous _ADD_CONFIGURATION_LINE_TO_HEADER_STRING to and else() > > here. > > I waffled a bit on this, so your approach is probably fine too. Actually, this isn't possible because there is at least one exception. USE_SYSTEM_MALLOC shouldn't have the prefix. I think it's safer to expose both for now.
Created attachment 252060 [details] Patch
Comment on attachment 252060 [details] Patch Hopefully USE won't require the WTF prefix eventually.
Comment on attachment 252060 [details] Patch Clearing flags on attachment: 252060 Committed r183626: <http://trac.webkit.org/changeset/183626>
All reviewed patches have been landed. Closing bug.