Bug 174986

Summary: [WTF] Use pragma once
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: Web Template FrameworkAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: achristensen, annulen, ap, beidson, don.olmstead, fujii.hironori, joepeck, mark.lam, mcatanzaro
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Yusuke Suzuki
Reported 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.
Attachments
Patch (103.84 KB, patch)
2018-02-07 13:01 PST, Don Olmstead 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 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.
Yusuke Suzuki
Comment 2 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>.
Don Olmstead
Comment 3 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?
Konstantin Tokarev
Comment 4 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
Konstantin Tokarev
Comment 5 2017-09-01 11:56:05 PDT
(And in WTF we already use private includes)
Don Olmstead
Comment 6 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.
Don Olmstead
Comment 7 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.
Alex Christensen
Comment 8 2018-10-12 13:08:16 PDT
I'm fixing this in https://bugs.webkit.org/show_bug.cgi?id=190527
Fujii Hironori
Comment 9 2018-10-15 18:41:17 PDT
*** This bug has been marked as a duplicate of bug 190527 ***
Note You need to log in before you can comment on or make changes to this bug.