Bug 175057 - [PAL] Move spi/cf directory into PAL
Summary: [PAL] Move spi/cf directory into PAL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-01 16:10 PDT by Yoshiaki Jitsukawa
Modified: 2017-08-03 17:21 PDT (History)
8 users (show)

See Also:


Attachments
Patch (105.49 KB, patch)
2017-08-01 16:38 PDT, Yoshiaki Jitsukawa
no flags Details | Formatted Diff | Diff
Patch (106.64 KB, patch)
2017-08-01 17:23 PDT, Yoshiaki Jitsukawa
no flags Details | Formatted Diff | Diff
Patch (108.05 KB, patch)
2017-08-02 00:14 PDT, Yoshiaki Jitsukawa
no flags Details | Formatted Diff | Diff
Patch (109.76 KB, patch)
2017-08-02 01:36 PDT, Yoshiaki Jitsukawa
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews113 for mac-elcapitan (1.99 MB, application/zip)
2017-08-02 05:10 PDT, Build Bot
no flags Details
Patch (109.76 KB, patch)
2017-08-02 13:53 PDT, Yoshiaki Jitsukawa
no flags Details | Formatted Diff | Diff
Patch (68.98 KB, patch)
2017-08-03 14:48 PDT, Yoshiaki Jitsukawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yoshiaki Jitsukawa 2017-08-01 16:10:45 PDT
[PAL] Move spi/cf directory into PAL
Comment 1 Yoshiaki Jitsukawa 2017-08-01 16:18:48 PDT
Moving all the SPI headers at once tends to cause merge conflict so I'm trying one directory by one.
Comment 2 Yoshiaki Jitsukawa 2017-08-01 16:38:58 PDT
Created attachment 316910 [details]
Patch
Comment 3 Build Bot 2017-08-01 16:41:58 PDT
Attachment 316910 [details] did not pass style-queue:


ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:42:  _CFNetworkHTTPConnectionCacheGetLimit is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:43:  _CFNetworkHTTPConnectionCacheSetLimit is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFLocaleSPI.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WebCore/PAL/pal/spi/cf/CoreMediaSPI.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
Total errors found: 5 in 72 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Yoshiaki Jitsukawa 2017-08-01 17:09:56 PDT
Do we need to fix style of SPI headers?
Comment 5 Yoshiaki Jitsukawa 2017-08-01 17:23:55 PDT
Created attachment 316917 [details]
Patch

I forgot to fix sources under the "Tools" directory...
Comment 6 Build Bot 2017-08-01 17:25:33 PDT
Attachment 316917 [details] did not pass style-queue:


ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:42:  _CFNetworkHTTPConnectionCacheGetLimit is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:43:  _CFNetworkHTTPConnectionCacheSetLimit is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFLocaleSPI.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WebCore/PAL/pal/spi/cf/CoreMediaSPI.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
Total errors found: 5 in 74 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Myles C. Maxfield 2017-08-01 22:48:28 PDT
Looks good once you fix the style check.
Comment 8 Don Olmstead 2017-08-01 23:27:44 PDT
Just the pragma once correct?
Comment 9 Yoshiaki Jitsukawa 2017-08-02 00:14:26 PDT
Created attachment 316942 [details]
Patch

Fixed the pragma once. I'm not sure whether the function names should be fixed or not...
Comment 10 Build Bot 2017-08-02 00:17:15 PDT
Attachment 316942 [details] did not pass style-queue:


ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:41:  _CFNetworkHTTPConnectionCacheGetLimit is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:42:  _CFNetworkHTTPConnectionCacheSetLimit is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 74 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Yoshiaki Jitsukawa 2017-08-02 01:36:52 PDT
Created attachment 316948 [details]
Patch

Add a path-specific(WebCore/PAL/pal/spi) style rules to ignore underscores in identifier names.
Comment 12 Build Bot 2017-08-02 05:10:12 PDT
Comment on attachment 316948 [details]
Patch

Attachment 316948 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4239366

New failing tests:
fast/dom/HTMLTemplateElement/ownerDocumentXHTML.xhtml
Comment 13 Build Bot 2017-08-02 05:10:14 PDT
Created attachment 316955 [details]
Archive of layout-test-results from ews113 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 14 Yoshiaki Jitsukawa 2017-08-02 13:53:20 PDT
Created attachment 316990 [details]
Patch
Comment 15 Yoshiaki Jitsukawa 2017-08-02 18:24:30 PDT
(In reply to Myles C. Maxfield from comment #7)
> Looks good once you fix the style check.

I've fixed the style check
 - Fix the include guards
 - Add a path-specific style rule for WebCore/PAL/pal/spi to ignore [readability/naming/underscores]

Would you review my patch?

The test failure on mac-debug EWS seems to be a false positive.
Comment 16 Myles C. Maxfield 2017-08-02 22:05:01 PDT
Comment on attachment 316990 [details]
Patch

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

It looks like there are a lot of unrelated changes in the Xcode project files. Are these intentional?

Also, because this patch touches the WebKit sub-project, you’ll need an OWNER to review those pieces.

Other than that, this looks good.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:-21137
> -				CDA29A2E1CBF73FC00901CCF /* PlaybackSessionInterfaceAVKit.h */,

Can you make sure you run the sort-Xcode script in Tools/Scripts on this project?
Comment 17 Yoshiaki Jitsukawa 2017-08-02 22:27:54 PDT
(In reply to Myles C. Maxfield from comment #16)
> Comment on attachment 316990 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=316990&action=review
> 
> It looks like there are a lot of unrelated changes in the Xcode project
> files. Are these intentional?
> 
> Also, because this patch touches the WebKit sub-project, you’ll need an
> OWNER to review those pieces.
> 
> Other than that, this looks good.
> 
> > Source/WebCore/WebCore.xcodeproj/project.pbxproj:-21137
> > -				CDA29A2E1CBF73FC00901CCF /* PlaybackSessionInterfaceAVKit.h */,
> 
> Can you make sure you run the sort-Xcode script in Tools/Scripts on this
> project?

Thank you. I CC Alex from the Owners list. 

I ran the sort script and unfortunately those unrelated diff were made by it.
Comment 18 Myles C. Maxfield 2017-08-03 01:07:45 PDT
(In reply to Yoshiaki Jitsukawa from comment #17)
> (In reply to Myles C. Maxfield from comment #16)
> > Comment on attachment 316990 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=316990&action=review
> > 
> > It looks like there are a lot of unrelated changes in the Xcode project
> > files. Are these intentional?
> > 
> > Also, because this patch touches the WebKit sub-project, you’ll need an
> > OWNER to review those pieces.
> > 
> > Other than that, this looks good.
> > 
> > > Source/WebCore/WebCore.xcodeproj/project.pbxproj:-21137
> > > -				CDA29A2E1CBF73FC00901CCF /* PlaybackSessionInterfaceAVKit.h */,
> > 
> > Can you make sure you run the sort-Xcode script in Tools/Scripts on this
> > project?
> 
> Thank you. I CC Alex from the Owners list. 
> 
> I ran the sort script and unfortunately those unrelated diff were made by it.

Can we sort the file in a preliminary separate patch?
Comment 19 Yoshiaki Jitsukawa 2017-08-03 01:45:29 PDT
(In reply to Myles C. Maxfield from comment #18)
> Can we sort the file in a preliminary separate patch?

Sure. Let me try.
Comment 20 Yoshiaki Jitsukawa 2017-08-03 02:07:47 PDT
(In reply to Yoshiaki Jitsukawa from comment #19)
> (In reply to Myles C. Maxfield from comment #18)
> > Can we sort the file in a preliminary separate patch?
> 
> Sure. Let me try.

I've filed these bugs and sent patches.
Bug 175121 - [WebCore] Sort Xcode project files
Bug 175122 - [WebKit] Sort Xcode project file (just in case)
Comment 21 Yoshiaki Jitsukawa 2017-08-03 14:48:10 PDT
Created attachment 317160 [details]
Patch
Comment 22 Don Olmstead 2017-08-03 16:49:10 PDT
Comment on attachment 317160 [details]
Patch

Not sure whats up with ios-sim but it seems like its flaky in other patches we've submitted today.
Comment 23 WebKit Commit Bot 2017-08-03 17:20:44 PDT
Comment on attachment 317160 [details]
Patch

Clearing flags on attachment: 317160

Committed r220243: <http://trac.webkit.org/changeset/220243>
Comment 24 WebKit Commit Bot 2017-08-03 17:20:46 PDT
All reviewed patches have been landed.  Closing bug.
Comment 25 Radar WebKit Bug Importer 2017-08-03 17:21:57 PDT
<rdar://problem/33713217>