WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
108761
[CMake] Don't warn unused cmake variables which aren't used by cmake ports
https://bugs.webkit.org/show_bug.cgi?id=108761
Summary
[CMake] Don't warn unused cmake variables which aren't used by cmake ports
Gyuyoung Kim
Reported
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
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
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Gyuyoung Kim
Comment 1
2013-02-02 20:50:42 PST
Created
attachment 186247
[details]
Patch
Build Bot
Comment 2
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
Laszlo Gombos
Comment 3
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
Gyuyoung Kim
Comment 4
2013-02-03 21:48:15 PST
Created
attachment 186301
[details]
Patch
Gyuyoung Kim
Comment 5
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.
Build Bot
Comment 6
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
Laszlo Gombos
Comment 7
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 ?
Gyuyoung Kim
Comment 8
2013-02-04 19:33:03 PST
Created
attachment 186527
[details]
Patch
Gyuyoung Kim
Comment 9
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.
Laszlo Gombos
Comment 10
2013-02-05 08:06:58 PST
Comment on
attachment 186527
[details]
Patch lgtm.
WebKit Review Bot
Comment 11
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
>
WebKit Review Bot
Comment 12
2013-02-05 15:42:12 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug