Bug 108761 - [CMake] Don't warn unused cmake variables which aren't used by cmake ports
Summary: [CMake] Don't warn unused cmake variables which aren't used by cmake ports
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-02 20:37 PST by Gyuyoung Kim
Modified: 2013-02-05 15:42 PST (History)
5 users (show)

See Also:


Attachments
Patch (3.13 KB, patch)
2013-02-02 20:50 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (1.38 KB, patch)
2013-02-03 21:48 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (1.33 KB, patch)
2013-02-04 19:33 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2013-02-02 20:37:21 PST
There is cmake warning because ENABLE_DOM4_EVENTS_CONSTRUCTOR and ENABLE_SUID_SANDBOX_LINUX is not defined in cmake files.

CMake Warning:
  Manually-specified variables were not used by the project:

    ENABLE_DOM4_EVENTS_CONSTRUCTOR
    ENABLE_SUID_SANDBOX_LINUX
Comment 1 Gyuyoung Kim 2013-02-02 20:50:42 PST
Created attachment 186247 [details]
Patch
Comment 2 Build Bot 2013-02-02 22:03:14 PST
Comment on attachment 186247 [details]
Patch

Attachment 186247 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16357232
Comment 3 Laszlo Gombos 2013-02-03 05:45:25 PST
Perhaps we should disable this warning instead of keep adding CMake variables to the build system that does not seems to be used (I do not see a conditional in any CMake file using this guard).

I think if one of the CMake based ports would turn this feature on it would be ok to add this falg (for the purpose of turning it on).

We're planning to  move most of the defining of these flags in .h files and this patch seems to move in the opposite direction.

Here is how to disable the warning - http://www.cmake.org/cmake/help/v2.8.8/cmake.html#opt:--no-warn-unused-cli
Comment 4 Gyuyoung Kim 2013-02-03 21:48:15 PST
Created attachment 186301 [details]
Patch
Comment 5 Gyuyoung Kim 2013-02-03 21:49:31 PST
Thank you for your review. I agree to your suggestion. So, I use "--no-warn-unused-cli" option in new patch.
Comment 6 Build Bot 2013-02-04 00:23:28 PST
Comment on attachment 186301 [details]
Patch

Attachment 186301 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16372077
Comment 7 Laszlo Gombos 2013-02-04 17:54:08 PST
Comment on attachment 186301 [details]
Patch

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

> Tools/Scripts/webkitdirs.pm:2200
> +    $additionalCMakeArgs = "--no-warn-unused-cli";

It seems to me that (re)setting this variable - instead of appending to it - would loose the CMake arguments passed on the command line to build-webkit. As an example disabling WebKit1 using <<build-webkit --efl --cmakeargs="-DENABLE_WEBKIT=OFF">> probably no longer works with this patch. Can you double-check and perhaps append instead ?
Comment 8 Gyuyoung Kim 2013-02-04 19:33:03 PST
Created attachment 186527 [details]
Patch
Comment 9 Gyuyoung Kim 2013-02-04 19:34:48 PST
(In reply to comment #7)
> (From update of attachment 186301 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186301&action=review
> 
> > Tools/Scripts/webkitdirs.pm:2200
> > +    $additionalCMakeArgs = "--no-warn-unused-cli";
> 
> As an example disabling WebKit1 using <<build-webkit --efl --cmakeargs="-DENABLE_WEBKIT=OFF">> probably no longer works with this patch. Can you double-check and perhaps append instead ?

I update this patch. This one works with "-DENABLE_WEBKIT=ON/OFF". Thanks.
Comment 10 Laszlo Gombos 2013-02-05 08:06:58 PST
Comment on attachment 186527 [details]
Patch

lgtm.
Comment 11 WebKit Review Bot 2013-02-05 15:42:06 PST
Comment on attachment 186527 [details]
Patch

Clearing flags on attachment: 186527

Committed r141942: <http://trac.webkit.org/changeset/141942>
Comment 12 WebKit Review Bot 2013-02-05 15:42:12 PST
All reviewed patches have been landed.  Closing bug.