Bug 174986 - [WTF] Use pragma once
Summary: [WTF] Use pragma once
Status: RESOLVED DUPLICATE of bug 190527
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-31 12:10 PDT by Yusuke Suzuki
Modified: 2018-10-15 18:41 PDT (History)
9 users (show)

See Also:


Attachments
Patch (103.84 KB, patch)
2018-02-07 13:01 PST, 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 Yusuke Suzuki 2017-07-31 12:10:05 PDT
WTF is very tricky. These headers are copied to BuildDir/usr/local/include/wtf.
And source files start using it... That causes problems.
Based on how you include headers (I believe this relies on "XXX.h" and <wtf/XXX.h> with different header search paths), the compiler includes the same files twice with difernt paths (Source/WTF/wtf/XXX.h and BuildDir/usr/local/include/wtf/XXX.h).
And it causes compile errors.

The example is wtf/text/StringView.h. When I changed this ifndef guard to pragma once, it causes compile error(https://bugs.webkit.org/show_bug.cgi?id=166676#c5). And I reverted this part.
And we can see similar problems https://bugs.webkit.org/show_bug.cgi?id=164194#c4.

But this tricky problem should be fixed. And we should change WTF headers' ifndef guard to pragma once.
Comment 1 Yusuke Suzuki 2017-07-31 12:24:28 PDT
This is my guess, but I think we should use "XXX.h", "text/XXX.h" styles in WTF instead of using <wtf/XXX.h> in WTF.
Comment 2 Yusuke Suzuki 2017-07-31 12:30:07 PDT
(In reply to Yusuke Suzuki from comment #1)
> This is my guess, but I think we should use "XXX.h", "text/XXX.h" styles in
> WTF instead of using <wtf/XXX.h> in WTF.

I think that the problem is that <wtf/XXX.h> goes to the build directory while "XXX.h" goes the source directory. We should consistently use "XXX.h" in WTF for WTF headers. Of course, outside of WTF, we should use <wtf/XXX.h>.
Comment 3 Don Olmstead 2017-08-31 19:10:18 PDT
(In reply to Yusuke Suzuki from comment #2)
> (In reply to Yusuke Suzuki from comment #1)
> > This is my guess, but I think we should use "XXX.h", "text/XXX.h" styles in
> > WTF instead of using <wtf/XXX.h> in WTF.
> 
> I think that the problem is that <wtf/XXX.h> goes to the build directory
> while "XXX.h" goes the source directory. We should consistently use "XXX.h"
> in WTF for WTF headers. Of course, outside of WTF, we should use <wtf/XXX.h>.

Might another issue be that we're using XXX_INCLUDE_DIRECTORIES when it should be XXX_PRIVATE_INCLUDE_DIRECTORIES?
Comment 4 Konstantin Tokarev 2017-09-01 11:55:21 PDT
If we use XXX_PRIVATE_INCLUDE_DIRECTORIES, we go back to situation where internal include paths of XXX are added to other targets. See https://bugs.webkit.org/show_bug.cgi?id=165686
Comment 5 Konstantin Tokarev 2017-09-01 11:56:05 PDT
(And in WTF we already use private includes)
Comment 6 Don Olmstead 2018-02-07 13:01:18 PST
Created attachment 333308 [details]
Patch

WTF headers are all copies now so this might actually work now. Lets see what the bots say.

https://pypi.python.org/pypi/guardonce was used to do the conversion.
Comment 7 Don Olmstead 2018-02-07 13:37:23 PST
WPE came back ok and I'm pretty sure the other CMake ports are going to come back green.

So the reason those are going to come back green is that the headers are installed into WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf\*. This directory is not included within the WTF include directories. Also Source\WTF is not an include path for the dependent projects.

The reason Apple Cocoa ports are coming back red is that within the xcode projects WTF is installed into /usr/local/include/wtf which for sure is on the system path.

https://trac.webkit.org/browser/webkit/trunk/Source/WTF/Configurations/CopyWTFHeaders.xcconfig#L24

So if all the headers used relative paths internally this could go forward. If we flatten the headers in WTF then this would also work.

On the CMake side its simple enough to flatten but with the Apple Cocoa side its a bit more murky.

The command within the xcode project is at https://trac.webkit.org/browser/webkit/trunk/Source/WTF/WTF.xcodeproj/project.pbxproj#L1393 and looks to be an rsync. There's a stack overflow question that looks like it might do the trick, https://stackoverflow.com/questions/14661033/rsync-recursively-sync-all-files-while-ignoring-the-directory-structure, but I'm not familiar enough with the Mac side to take a real stab at it.
Comment 8 Alex Christensen 2018-10-12 13:08:16 PDT
I'm fixing this in https://bugs.webkit.org/show_bug.cgi?id=190527
Comment 9 Fujii Hironori 2018-10-15 18:41:17 PDT

*** This bug has been marked as a duplicate of bug 190527 ***