Bug 119035 - [EFL][CMAKE] Fix wrong syntax about option commands
Summary: [EFL][CMAKE] Fix wrong syntax about option commands
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryuan Choi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-23 22:41 PDT by Ryuan Choi
Modified: 2013-07-25 01:28 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.51 KB, patch)
2013-07-23 22:45 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryuan Choi 2013-07-23 22:41:48 PDT
Second value of option command should be description.

option(<option_variable> "help string describing option" [initial value])

http://www.cmake.org/cmake/help/v2.8.11/cmake.html#command:option
Comment 1 Ryuan Choi 2013-07-23 22:45:04 PDT
Created attachment 207374 [details]
Patch
Comment 2 Chris Dumez 2013-07-24 00:25:31 PDT
Comment on attachment 207374 [details]
Patch

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

> Source/cmake/OptionsEfl.cmake:211
> +    option(ENABLE_EGL "Enable EGL Support")

Shouldn't we keep ON as 3rd argument? It seems the default value is OFF otherwise.

> Source/cmake/OptionsEfl.cmake:221
> +        option(ENABLE_GLES2 "Enable GLES Support")

Ditto.
Comment 3 Ryuan Choi 2013-07-24 00:51:26 PDT
(In reply to comment #2)
> (From update of attachment 207374 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207374&action=review
> 
> > Source/cmake/OptionsEfl.cmake:211
> > +    option(ENABLE_EGL "Enable EGL Support")
> 
> Shouldn't we keep ON as 3rd argument? It seems the default value is OFF otherwise.
> 
> > Source/cmake/OptionsEfl.cmake:221
> > +        option(ENABLE_GLES2 "Enable GLES Support")
> 
> Ditto.

Yes, so these were OFF as default. (Now, ON is just description)
It is the reason why I make this patch. :)

I traced original bugs and they intended OFF.
https://bugs.webkit.org/show_bug.cgi?id=105602
https://bugs.webkit.org/show_bug.cgi?id=105816
Comment 4 Chris Dumez 2013-07-24 01:55:15 PDT
Comment on attachment 207374 [details]
Patch

Ok. r=me. No behavior change, just proper description.
Comment 5 WebKit Commit Bot 2013-07-24 02:56:43 PDT
Comment on attachment 207374 [details]
Patch

Clearing flags on attachment: 207374

Committed r153080: <http://trac.webkit.org/changeset/153080>
Comment 6 WebKit Commit Bot 2013-07-24 02:56:46 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Kalyan 2013-07-25 01:26:19 PDT
Comment on attachment 207374 [details]
Patch

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

> Source/cmake/OptionsEfl.cmake:222
>  

Didn't follow this completely. In this case GLES support should be enabled(We force GLES usage with EGL).If I understood it right this will default to off now unless we enable both EGL and GLES options
Comment 8 Kalyan 2013-07-25 01:28:30 PDT
Comment on attachment 207374 [details]
Patch

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

> Source/cmake/OptionsEfl.cmake:223
>          if (ENABLE_GLES2)

sorry, ignore my comment.