Bug 162368 - [CMake] Use CMake to determine HAVE_* defines
Summary: [CMake] Use CMake to determine HAVE_* defines
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Don Olmstead
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-21 18:47 PDT by Don Olmstead
Modified: 2016-09-28 07:04 PDT (History)
6 users (show)

See Also:


Attachments
Patch (5.14 KB, patch)
2016-09-26 16:52 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (5.14 KB, patch)
2016-09-26 16:59 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (5.27 KB, patch)
2016-09-27 10:45 PDT, Don Olmstead
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Don Olmstead 2016-09-21 18:47:34 PDT
Remove HAVE_* defines from Platform.h when its possible to determine the values that should be used through CMake.
Comment 1 Don Olmstead 2016-09-26 16:52:33 PDT
Created attachment 289894 [details]
Patch

Moves HAVE_* macros over to CMake. Some HAVE_* are moved to OS(DARWIN) from OS(UNIX) since those projects aren't using CMake. HAVE_VIRTUALALLOC was removed since I didn't see it being used.
Comment 2 WebKit Commit Bot 2016-09-26 16:54:56 PDT
Attachment 289894 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/Platform.h:608:  CPP comments are not allowed in Platform.h, please use C comments /* ... */  [build/cpp_comment] [5]
ERROR: Source/WTF/wtf/Platform.h:648:  CPP comments are not allowed in Platform.h, please use C comments /* ... */  [build/cpp_comment] [5]
ERROR: Source/cmake/OptionsCommon.cmake:215:  No space between command "endmacro" and its parentheses, should be "endmacro("  [whitespace/parentheses] [5]
ERROR: Source/cmake/OptionsCommon.cmake:220:  No space between command "endmacro" and its parentheses, should be "endmacro("  [whitespace/parentheses] [5]
ERROR: Source/cmake/OptionsCommon.cmake:225:  No space between command "endmacro" and its parentheses, should be "endmacro("  [whitespace/parentheses] [5]
ERROR: Source/cmake/OptionsCommon.cmake:230:  No space between command "endmacro" and its parentheses, should be "endmacro("  [whitespace/parentheses] [5]
Total errors found: 6 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Don Olmstead 2016-09-26 16:59:18 PDT
Created attachment 289895 [details]
Patch
Comment 4 Alex Christensen 2016-09-27 08:55:02 PDT
Comment on attachment 289895 [details]
Patch

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

> Source/WTF/wtf/Platform.h:624
> +#if !defined(HAVE_PTHREAD_NP_H)
>  #if (OS(FREEBSD) || OS(OPENBSD)) && !defined(__GLIBC__)

I think we can assume that if we're using GLIBC, then we are using CMake.  I think this whole block can be removed.

> Source/WTF/wtf/Platform.h:642
> +#if !defined(HAVE_STAT_BIRTHTIME)
>  #if (OS(DARWIN) || OS(FREEBSD) || OS(NETBSD)) && !defined(__GLIBC__)

Ditto

> Source/cmake/OptionsCommon.cmake:254
> +# Windows has signal.h but does not have all of them defined

What is "them"?
Comment 5 Don Olmstead 2016-09-27 10:45:48 PDT
Created attachment 289974 [details]
Patch

Fixing based on comments
Comment 6 WebKit Commit Bot 2016-09-27 13:32:29 PDT
Comment on attachment 289974 [details]
Patch

Clearing flags on attachment: 289974

Committed r206458: <http://trac.webkit.org/changeset/206458>
Comment 7 WebKit Commit Bot 2016-09-27 13:32:33 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Konstantin Tokarev 2016-09-28 07:03:41 PDT
Comment on attachment 289974 [details]
Patch

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

> Source/WTF/wtf/Platform.h:608
> +/* FIXME: Remove after CMake build enabled on Darwin */

This is incorrect, these definitions can only be removed if CMake is the only build system on Darwin, i.e. Xcode build is removed

> Source/cmake/OptionsCommon.cmake:212
> +macro(_HAVE_CHECK_INCLUDE _variable _header)

We don't have macros starting with underscore anywhere, either WEBKIT_ prefix is used or no prefix at all.

WEBKIT_CHECK_INCLUDE and so on may be better names
Comment 9 Konstantin Tokarev 2016-09-28 07:04:34 PDT
Underscore is used for macro parameters