Summary: | [Chromium] Seperate GTK specific Gyp rules from X11 Gyp rules | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Fady Samuel <fsamuel> | ||||||||
Component: | New Bugs | Assignee: | Fady Samuel <fsamuel> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | tony, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Fady Samuel
2011-09-27 14:29:47 PDT
Created attachment 108899 [details]
Patch
Comment on attachment 108899 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108899&action=review > Source/WebCore/ChangeLog:14 > + No new tests. (OOPS!) Remove this line. > Source/WebCore/WebCore.gyp/WebCore.gyp:-1373 > - ['include', 'platform/chromium/KeyCodeConversionGtk\\.cpp$'], Why can't we delete KeyCodeConversionGtk.cpp? > Source/WebCore/WebCore.gyp/WebCore.gyp:1532 > + ['use_x11 == 0', { > 'sources/': [ > ['exclude', '(Gtk|Linux)\\.cpp$'], > ['exclude', 'Harfbuzz[^/]+\\.(cpp|h)$'], > ], > }], Can we move this to an else clause of the use_x11 == 1 above? > Source/WebCore/WebCore.gyp/WebCore.gyp:1635 > + ['use_x11 == 0', { > 'sources/': [ > ['exclude', '(Gtk|Linux)\\.cpp$'], > ], This one is weird. Should we split the Gtk and Linux cases into two conditions? > Source/WebCore/WebCore.gyp/WebCore.gyp:1765 > - ['toolkit_uses_gtk == 0', { > + ['use_x11 == 0', { > 'sources/': [ > ['exclude', '(Gtk|Linux)\\.cpp$'], ditto. > Source/WebKit/chromium/ChangeLog:8 > + public/linux/WebFontInfo.h is a copy of public/gtk/WebFontInfo.h > + > + The original file will be deleted once the appropriate changes have landed in Chromium. One way to make this more clear is to have the old header be a forwarding header. It would avoid the confusion about having 2 identical headers in the tree. Created attachment 109011 [details]
Patch
Attachment 109011 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebKit/chromium/public/gtk/WebFontInfo.h:31: #ifndef header guard has wrong style, please use: WebFontInfo_h [build/header_guard] [5]
Total errors found: 1 in 6 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 109011 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109011&action=review >> Source/WebKit/chromium/public/gtk/WebFontInfo.h:31 >> +#ifndef Gtk_WebFontInfo_h > > #ifndef header guard has wrong style, please use: WebFontInfo_h [build/header_guard] [5] Hmm, if both WebFontInfo files use the same header guard, could that cause problems? Created attachment 109024 [details]
Patch
Comment on attachment 109024 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109024&action=review > Source/WebCore/WebCore.gyp/WebCore.gyp:1642 > + ['use_x11 == 0', { > + 'sources/': [ > + ['exclude', 'Linux\\.cpp$'], It might be more accurate to do 'os_posix and OS!="mac"', but this is OK too. Comment on attachment 109024 [details] Patch Clearing flags on attachment: 109024 Committed r96233: <http://trac.webkit.org/changeset/96233> All reviewed patches have been landed. Closing bug. |