Bug 131095 - [GTK][CMake] Look for glx.h in OPENGL_INCLUDE_DIR.
Summary: [GTK][CMake] Look for glx.h in OPENGL_INCLUDE_DIR.
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: Raphael Kubo da Costa (:rakuco)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-02 01:22 PDT by Raphael Kubo da Costa (:rakuco)
Modified: 2014-04-03 00:28 PDT (History)
6 users (show)

See Also:

Attachments
Patch (1.97 KB, patch)
2014-04-02 01:24 PDT, Raphael Kubo da Costa (:rakuco) 		no flags Details | Formatted Diff | Diff
Patch for landing (1.97 KB, patch)
2014-04-03 00:27 PDT, Raphael Kubo da Costa (:rakuco) 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Raphael Kubo da Costa (:rakuco) 2014-04-02 01:22:37 PDT 
[GTK][CMake] Look for glx.h in OPENGL_INCLUDE_DIR.
Comment 1 Raphael Kubo da Costa (:rakuco) 2014-04-02 01:24:09 PDT 
Created attachment 228372 [details]
Patch
Comment 2 Martin Robinson 2014-04-02 07:48:42 PDT 
Comment on attachment 228372 [details]
Patch

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

> Source/cmake/OptionsGTK.cmake:231
> +set(_required_includes_old ${CMAKE_REQUIRED_INCLUDES})

Why do you use the macro naming scheme here? I think this can be old_required_includes or OLD_REQUIRED_INCLUDES.
Comment 3 Raphael Kubo da Costa (:rakuco) 2014-04-02 07:55:07 PDT 
Comment on attachment 228372 [details]
Patch

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

>> Source/cmake/OptionsGTK.cmake:231
>> +set(_required_includes_old ${CMAKE_REQUIRED_INCLUDES})
> 
> Why do you use the macro naming scheme here? I think this can be old_required_includes or OLD_REQUIRED_INCLUDES.

No particular reason, I just thought it would make sense to make the name look internal. I can change it to OLD_REQUIRED_INCLUDES before landing.
Comment 4 Martin Robinson 2014-04-02 08:00:40 PDT 
Thanks for fixing!
Comment 5 Raphael Kubo da Costa (:rakuco) 2014-04-03 00:27:17 PDT 
Created attachment 228479 [details]
Patch for landing
Comment 6 Raphael Kubo da Costa (:rakuco) 2014-04-03 00:28:37 PDT 
Committed r166702: <http://trac.webkit.org/changeset/166702>