Bug 90558 - [CMake] Fix some CMake warnings
Summary: [CMake] Fix some CMake warnings
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-04 08:36 PDT by Rob Buis
Modified: 2012-07-05 11:13 PDT (History)
4 users (show)

See Also:


Attachments
Patch (2.20 KB, patch)
2012-07-04 08:41 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (2.77 KB, patch)
2012-07-04 14:51 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (2.99 KB, patch)
2012-07-05 08:20 PDT, Rob Buis
dbates: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Buis 2012-07-04 08:36:27 PDT
CMake 2.8.5 reports warnings like this:

Manually-specified variables were not used by the project:

    ENABLE_LINK_PRERENDER
Comment 1 Rob Buis 2012-07-04 08:41:10 PDT
Created attachment 150805 [details]
Patch
Comment 2 Raphael Kubo da Costa (:rakuco) 2012-07-04 14:18:23 PDT
Comment on attachment 150805 [details]
Patch

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

> ChangeLog:3
> +        [BlackBerry] Fix some CMake warnings

Shouldn't this rather have [CMake] instead of [BlackBerry] in the title? Other ports which use CMake have the same warnings.
Comment 3 Rob Buis 2012-07-04 14:20:41 PDT
Hi Raphael,

(In reply to comment #2)
> (From update of attachment 150805 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=150805&action=review
> 
> > ChangeLog:3
> > +        [BlackBerry] Fix some CMake warnings
> 
> Shouldn't this rather have [CMake] instead of [BlackBerry] in the title? Other ports which use CMake have the same warnings.

Yes, you are right, sorry! Do you feel like reviewing?
Comment 4 Raphael Kubo da Costa (:rakuco) 2012-07-04 14:37:45 PDT
Looks fine to me(In reply to comment #3)
> Do you feel like reviewing?

You probably also need to add the new flags to Source/cmakeconfig.h.cmake. I don't have reviewer superpowers, though :-(
Comment 5 Rob Buis 2012-07-04 14:51:12 PDT
Created attachment 150845 [details]
Patch
Comment 6 Rob Buis 2012-07-04 14:52:18 PDT
Hi Raphael,

(In reply to comment #4)
> Looks fine to me(In reply to comment #3)
> > Do you feel like reviewing?
>
> You probably also need to add the new flags to Source/cmakeconfig.h.cmake. I don't have reviewer superpowers, though :-(

Thanks! I am not complete sure if our port needs the cmakeconfig.h.cmake addtion, but I did update it in my latest patch.
Comment 7 Raphael Kubo da Costa (:rakuco) 2012-07-04 16:34:47 PDT
(In reply to comment #6)
> I am not complete sure if our port needs the cmakeconfig.h.cmake addtion, but I did update it in my latest patch.

It might not need it now due to the features being disabled by default, but all CMake-based ports use that for the ENABLE() and USE() macros to work, otherwise the macro definitions will never be set.

Shouldn't you set the WTF_* entries in cmakeconfig.h.cmake as well?
Comment 8 Rob Buis 2012-07-05 07:39:50 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > I am not complete sure if our port needs the cmakeconfig.h.cmake addtion, but I did update it in my latest patch.
> 
> It might not need it now due to the features being disabled by default, but all CMake-based ports use that for the ENABLE() and USE() macros to work, otherwise the macro definitions will never be set.

Ok.

> Shouldn't you set the WTF_* entries in cmakeconfig.h.cmake as well?

You are right, however I do not see our port using these two in the near future. If you think your port will, I can add them?
Comment 9 Raphael Kubo da Costa (:rakuco) 2012-07-05 08:04:16 PDT
(In reply to comment #8)
> > Shouldn't you set the WTF_* entries in cmakeconfig.h.cmake as well?
> 
> You are right, however I do not see our port using these two in the near future. If you think your port will, I can add them?

Right now it's mostly to make the patch consistent (otherwise even though the CMake warnings are fixed the other half of adding something to WebKitFeatures.cmake will not have been done).
Comment 10 Rob Buis 2012-07-05 08:20:42 PDT
Created attachment 150943 [details]
Patch
Comment 11 Daniel Bates 2012-07-05 10:36:29 PDT
Comment on attachment 150943 [details]
Patch

r=me
Comment 12 Rob Buis 2012-07-05 11:13:25 PDT
Committed r121914: <http://trac.webkit.org/changeset/121914>