Bug 143572 – [CMake] Options should be marked as advanced by default

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

Options defined with WEBKIT_OPTION_DEFINE should be marked as advanced so that they don't show up in CMake GUIs or with cmake -L. We also shouldn't print them when running cmake. This prevents ports from unexpectedly getting new public options. The idea is that you only make an option public if you're really willing to support building both with and without it.

If WEBKIT_OPTION_DEFINE_PORT_VALUE is called, then the option should become public. WEBKIT_OPTION_PRIVATE_PORT_VALUE is provided to change the default value for a port without making it public.

WEBKIT_OPTION_DEFINE_PUBLIC is provided to create a new port-specific option and mark it public without needing to use both WEBKIT_OPTION_DEFINE and WEBKIT_OPTION_DEFINE_PORT_VALUE.

Note that there is no enforcement: users can still set all of these options if they really want to, as before, e.g. 'cmake -DSOME_UNSUPPORTED_OPTION=ON', but probably only developers will do this.

[Michael Catanzaro]

Warning: if your port doesn't use WEBKIT_OPTION_DEFINE_PORT_VALUE for an option, then this patch will cause that option to disappear!

This seemed like the best way to make it easy to hide options, and make sure new options don't unexpectedly appear for particular ports, without making all the Mac and EFL options disappear.

[Alex Christensen]

That should not be a problem for the Mac and Windows ports' CMake files right now.  They are experimental and under construction right now anyways.

[Michael Catanzaro]

Created attachment 250464 [details]
Patch

[Michael Catanzaro]

Comment on attachment 250464 [details]
Patch

I'll need to try again. WEBKIT_OPTION_DEFINE_PUBLIC doesn't work.

[Michael Catanzaro]

Created attachment 250496 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 250496 [details]
Patch

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

I think _PUBLIC and _PRIVATE macros look like better method to define options. Basically I agree to use it. However someone else might want to take a look this patch before landing.

> Source/cmake/WebKitFeatures.cmake:11
> +    set(_WEBKIT_AVAILABLE_OPTIONS_ISPUBLIC_${_name} TRUE)

Isn't "_IS_PUBLIC" looks more clear than "_ISPUBLIC" ?

[Michael Catanzaro]

(In reply to comment #6)
> Isn't "_IS_PUBLIC" looks more clear than "_ISPUBLIC" ?

I almost did that, but went with ISPUBLIC to match _WEBKIT_AVAILABLE_OPTIONS_INITIALVALUE. Probably IS_PUBLIC would be better, indeed.

[WebKit Commit Bot]

Comment on attachment 250496 [details]
Patch

Clearing flags on attachment: 250496

Committed r182658: <http://trac.webkit.org/changeset/182658>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.