Bug 182757 - [CMake] WEBKIT_MAKE_FORWARDING_HEADERS shouldn't use POST_BUILD to copy generated headers
Summary: [CMake] WEBKIT_MAKE_FORWARDING_HEADERS shouldn't use POST_BUILD to copy gener...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks: 182512
  Show dependency treegraph
 
Reported: 2018-02-13 17:57 PST by Fujii Hironori
Modified: 2019-04-02 13:45 PDT (History)
9 users (show)

See Also:


Attachments
WIP patch (7.26 KB, patch)
2018-02-13 20:49 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff
WIP Patch (53.74 KB, patch)
2018-02-15 20:20 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
WIP Patch (5.04 KB, patch)
2018-02-16 14:33 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
WIP Patch (6.17 KB, patch)
2018-02-21 17:37 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
WIP Patch (107.99 KB, patch)
2018-02-22 17:20 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
WIP Patch (113.48 KB, patch)
2018-02-22 18:09 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
WIP Patch (114.23 KB, patch)
2018-02-22 18:32 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
WIP Patch (114.68 KB, patch)
2018-02-22 19:01 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (115.02 KB, patch)
2018-02-22 19:08 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (115.02 KB, patch)
2018-02-22 19:15 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
WIP Patch (115.01 KB, patch)
2018-02-22 19:26 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
WIP Patch (115.03 KB, patch)
2018-02-22 19:34 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
WIP Patch (115.02 KB, patch)
2018-02-23 11:29 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
WIP patch (8.11 KB, patch)
2019-04-01 21:03 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
WIP patch (9.93 KB, patch)
2019-04-01 22:30 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (14.77 KB, patch)
2019-04-02 00:23 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2018-02-13 17:57:37 PST
REGRESSION(r228431) [CMake][Ninja] Fails to compile TestWebCore due to missing WebCore's derived headers

> [2079/3246] Building CXX object Tools/TestWebKitA...les/TestWebCore.dir/Tests/WebCore/CSSParser.cpp.o
> FAILED: Tools/TestWebKitAPI/CMakeFiles/TestWebCore.dir/Tests/WebCore/CSSParser.cpp.o 
> /usr/lib/ccache/c++  -DBUILDING_GTK__=1 -DBUILDING_WEBKIT2__ -DBUILDING_WITH_CMAKE=1 -DDATA_DIR=\"share\" -DGETTEXT_PACKAGE=\"WebKit2GTK-4.0\" -DGTEST_HAS_RTTI=0 -DGTEST_LINKED_AS_SHARED_LIBRARY=1 -DHAVE_CONFIG_H=1 -DTEST_WEBKIT2_RESOURCES_DIR=\"/home/fujii/work/webkit/ga/Tools/TestWebKitAPI/Tests/WebKit\" -DWEBKITGTK_API_VERSION_STRING=\"4.0\" -IDerivedSources/ForwardingHeaders -IDerivedSources/ForwardingHeaders/JavaScriptCore -I../../Source/WebKit/UIProcess/API/C/soup -I../../Source/WebKit/UIProcess/API/C/gtk -I../../Source/WebKit/UIProcess/API/gtk -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include/gtk-3.0 -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include/gio-unix-2.0 -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include/cairo -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include/pango-1.0 -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include/harfbuzz -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include/gdk-pixbuf-2.0 -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include/pixman-1 -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include/freetype2 -isystem /usr/include/libpng16 -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include/libxml2 -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include/libdrm -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include/glib-2.0 -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/lib/glib-2.0/include -isystem ../DependenciesGTK/Root/include/glib-2.0 -isystem ../DependenciesGTK/Root/lib/glib-2.0/include -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include/at-spi2-atk/2.0 -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include/at-spi-2.0 -isystem /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Root/include/atk-1.0 -isystem /usr/include/dbus-1.0 -isystem /usr/lib/x86_64-linux-gnu/dbus-1.0/include -isystem ../DependenciesGTK/Root/include/libsoup-2.4 -I../../Tools/TestWebKitAPI -I. -I../../Source -I../../Source/JavaScriptCore -I../../Source/ThirdParty/gtest/include -I../../Source/WebKit/Platform/IPC -I../../Source/WebKit/Shared -I../../Source/WebKit/Shared/API -I../../Source/WebKit/Shared/API/c -I../../Source/WebKit/Shared/Plugins -I../../Source/WebKit/UIProcess -I../../Source/WebKit/UIProcess/API -I../../Source/WebKit/WebProcess/InjectedBundle -I../../Source/WebKit/WebProcess/InjectedBundle/API/c -I../../Source/bmalloc -IDerivedSources -I../../Source/ThirdParty -fdiagnostics-color=always -Wno-expansion-to-defined -Wno-attributes -Wno-noexcept-type -Wno-maybe-uninitialized -Wwrite-strings -Wundef -Wpointer-arith -Wmissing-format-attribute -Wformat-security -Wcast-align -Wextra -Wall  -fno-strict-aliasing -fno-exceptions -std=c++14 -fno-rtti -O3 -DNDEBUG -fPIE   -Wno-sign-compare -Wno-undef -Wno-unused-parameter -MD -MT Tools/TestWebKitAPI/CMakeFiles/TestWebCore.dir/Tests/WebCore/CSSParser.cpp.o -MF Tools/TestWebKitAPI/CMakeFiles/TestWebCore.dir/Tests/WebCore/CSSParser.cpp.o.d -o Tools/TestWebKitAPI/CMakeFiles/TestWebCore.dir/Tests/WebCore/CSSParser.cpp.o -c ../../Tools/TestWebKitAPI/Tests/WebCore/CSSParser.cpp
> In file included from DerivedSources/ForwardingHeaders/WebCore/CSSParserToken.h:32:0,
>                  from DerivedSources/ForwardingHeaders/WebCore/CSSParserTokenRange.h:32,
>                  from DerivedSources/ForwardingHeaders/WebCore/StyleProperties.h:25,
>                  from ../../Tools/TestWebKitAPI/Tests/WebCore/CSSParser.cpp:30:
> DerivedSources/ForwardingHeaders/WebCore/CSSPrimitiveValue.h:24:10: fatal error: CSSPropertyNames.h: No such file or directory
>  #include "CSSPropertyNames.h"
          ^~~~~~~~~~~~~~~~~~~~
Comment 1 Fujii Hironori 2018-02-13 18:06:50 PST
WEBKIT_MAKE_FORWARDING_HEADERS is using POST_BUILD event to copy derived headers.
TestWebCore has a dependency to WebCore. But this is link-time dependency, it should be compile-time dependency.
Comment 2 Fujii Hironori 2018-02-13 20:49:40 PST
Created attachment 333764 [details]
WIP patch

This patch depends on Bug 182512.
Comment 3 Don Olmstead 2018-02-14 11:10:59 PST
What if instead of ${FRAMEWORK} being dependent on ${FRAMEWORK}ForwardingHeaders it was flipped so that it was ${FRAMEWORK}ForwardingHeaders was reliant on ${FRAMEWORK}. That way you could target the derived sources in the list of headers.

It seems like this problem exists currently just the fact that WebCore has so many headers that this sort of race condition can occur.
Comment 4 Fujii Hironori 2018-02-14 15:45:37 PST
(In reply to Don Olmstead from comment #3)
> What if instead of ${FRAMEWORK} being dependent on
> ${FRAMEWORK}ForwardingHeaders it was flipped so that it was
> ${FRAMEWORK}ForwardingHeaders was reliant on ${FRAMEWORK}. 

For example, WebKit links WebCore, and WebCore depends on WebCoreForwardingHeaders.
Then, building WebKit triggers WebCoreForwardingHeaders.

> That way you
> could target the derived sources in the list of headers.

I don't understand this sentence. What do you mean?

> It seems like this problem exists currently just the fact that WebCore has
> so many headers that this sort of race condition can occur.

You are right.
Comment 5 Fujii Hironori 2018-02-14 15:48:40 PST
Comment on attachment 333764 [details]
WIP patch

This patch has the similar issue with Bug 181117, Bug 177286 and Bug 163774. Generating headers are triggered twice in CMake Visual Studio builds.
Comment 6 Don Olmstead 2018-02-15 12:36:58 PST
(In reply to Fujii Hironori from comment #4)
> (In reply to Don Olmstead from comment #3)
> > What if instead of ${FRAMEWORK} being dependent on
> > ${FRAMEWORK}ForwardingHeaders it was flipped so that it was
> > ${FRAMEWORK}ForwardingHeaders was reliant on ${FRAMEWORK}. 
> 
> For example, WebKit links WebCore, and WebCore depends on
> WebCoreForwardingHeaders.
> Then, building WebKit triggers WebCoreForwardingHeaders.
> 
> > That way you
> > could target the derived sources in the list of headers.
> 
> I don't understand this sentence. What do you mean?
> 
> > It seems like this problem exists currently just the fact that WebCore has
> > so many headers that this sort of race condition can occur.
> 
> You are right.

This would be the dependencies.

WTF depends on nothing
WTFForwardingHeaders depends on WTF

JavaScriptCore depends on WTF and WTFForwardingHeaders
JavaScriptCoreForwardingHeaders depends on JavaScriptCore

WebCore depends on WTF, WTFForwardingHeaders, JavaScriptCore, and JavaScriptCoreForwardingHeaders
WebCoreForwardingHeaders depends on WebCore

If its a dependency on WebCore and WebCore produces Settings.h then within the CMake we would append the derived sources headers.

list(APPEND WebCore_PRIVATE_FRAMEWORK_HEADERS
    ${DERIVED_SOURCES_WEBCORE_DIR}/Settings.h
    ....
)

Correct me if I'm wrong but wouldn't ninja be happy at that point?
Comment 7 Don Olmstead 2018-02-15 20:20:41 PST
Created attachment 333993 [details]
WIP Patch

Here's an idea for something that might work for you to check out Fujii.
Comment 8 Fujii Hironori 2018-02-15 21:33:09 PST
(In reply to Fujii Hironori from comment #1)
> WEBKIT_MAKE_FORWARDING_HEADERS is using POST_BUILD event to copy derived
> headers.
> TestWebCore has a dependency to WebCore. But this is link-time dependency,
> it should be compile-time dependency.

I think the simplest solution is introducing an intermediate target between TestWebCore and WebCore.

diff --git a/Tools/TestWebKitAPI/PlatformGTK.cmake b/Tools/TestWebKitAPI/PlatformGTK.cmake
index 4aef6953cd1..013fa9301e9 100644
--- a/Tools/TestWebKitAPI/PlatformGTK.cmake
+++ b/Tools/TestWebKitAPI/PlatformGTK.cmake
@@ -105,6 +105,9 @@ add_executable(TestWebCore
 
 target_link_libraries(TestWebCore ${test_webcore_LIBRARIES})
 add_dependencies(TestWebCore ${ForwardingHeadersForTestWebKitAPI_NAME})
+add_custom_target(pre-TestWebCore)
+add_dependencies(TestWebCore pre-TestWebCore)
+add_dependencies(pre-TestWebCore WebCore)
 
 add_test(TestWebCore ${TESTWEBKITAPI_RUNTIME_OUTPUT_DIRECTORY}/WebCore/TestWebCore)
 set_tests_properties(TestWebCore PROPERTIES TIMEOUT 60)
Comment 9 Don Olmstead 2018-02-16 14:33:37 PST
Created attachment 334072 [details]
WIP Patch
Comment 10 Don Olmstead 2018-02-16 14:42:49 PST
Comment on attachment 334072 [details]
WIP Patch

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

Ok I think this is better than that last patch. I don't see rebuilds in Visual Studio.

Fujii and Konstantin could you take a look to see if what I'm doing is sane within CMake land?

> Source/JavaScriptCore/CMakeLists.txt:15
> +    "${CMAKE_BINARY_DIR}/JavaScriptCore.framework/Headers"
> +    "${CMAKE_BINARY_DIR}/JavaScriptCore.framework/PrivateHeaders"

This follows along with how Apple builds operate.

Stuff in PrivateHeaders does a #include <JavaScriptCore/Foo.h> so this would likely catch any compile errors. Also LLInt needs these headers installed.

> Source/JavaScriptCore/CMakeLists.txt:1187
> +    PRIVATE_HEADERS ${JavaScriptCore_PRIVATE_FRAMEWORK_HEADERS} ${JavaScriptCore_HEADERS}

All the DerivedSources seem to be in ${JavaScriptCore_HEADERS}. It feels like the JSC CMake stuff is in better shape than WebCore. I feel like we might want to do something similar in WebCore with Derived Sources.

> Source/JavaScriptCore/CMakeLists.txt:1195
> +add_dependencies(LLIntOffsetsExtractor JavaScriptCoreFrameworkHeaders)

By splitting into public and private we can do this. Actually I think the only reason we weren't hitting a compile error here is that the copying just happens to complete before this is hit.

> Source/cmake/WebKitMacros.cmake:265
> +            add_custom_command(TARGET ${framework} PRE_LINK

So this moves the DerivedSources to being copied within the original ${framework} as a PRE_LINK step which happens before a POST_BUILD. Seems like a safe bet that things are going to be ok at this point.

I need to see how things work with WebCore to really determine if this will work across the board.

It seems to not cause rebuilds within Visual Studio so I think it solves things. Placing them within PrivateFrameworkHeaders causes the rebuilds in visual studio.

> Source/cmake/WebKitMacros.cmake:283
> +    #add_dependencies(${framework}PrivateFrameworkHeaders ${framework})

I'm not listing any dependencies for either of the *FrameworkHeaders projects. This should all be listed in required projects. Hopefully it will mean that we don't end up with a really serialized clean build.
Comment 11 Konstantin Tokarev 2018-02-17 07:51:18 PST
Comment on attachment 334072 [details]
WIP Patch

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

>> Source/JavaScriptCore/CMakeLists.txt:15
>> +    "${CMAKE_BINARY_DIR}/JavaScriptCore.framework/PrivateHeaders"
> 
> This follows along with how Apple builds operate.
> 
> Stuff in PrivateHeaders does a #include <JavaScriptCore/Foo.h> so this would likely catch any compile errors. Also LLInt needs these headers installed.

I don't think it makes any sense to copy layout of macOS framework when there is no actual framework built.

Unless we are 100% sure that we don't want to use built-in cmake's support for frameworks on macOS, and even then having .framework on other platforms may be confusing

>> Source/cmake/WebKitMacros.cmake:265
>> +            add_custom_command(TARGET ${framework} PRE_LINK
> 
> So this moves the DerivedSources to being copied within the original ${framework} as a PRE_LINK step which happens before a POST_BUILD. Seems like a safe bet that things are going to be ok at this point.
> 
> I need to see how things work with WebCore to really determine if this will work across the board.
> 
> It seems to not cause rebuilds within Visual Studio so I think it solves things. Placing them within PrivateFrameworkHeaders causes the rebuilds in visual studio.

Did you check with Ninja generator? In general it's better to stay away from PRE_LINK, POST_BUILD etc., these things don't have well-defined behavior outside of Visual Studio generator
Comment 12 Don Olmstead 2018-02-17 14:13:33 PST
(In reply to Konstantin Tokarev from comment #11)
> Comment on attachment 334072 [details]
> WIP Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=334072&action=review
> 
> >> Source/JavaScriptCore/CMakeLists.txt:15
> >> +    "${CMAKE_BINARY_DIR}/JavaScriptCore.framework/PrivateHeaders"
> > 
> > This follows along with how Apple builds operate.
> > 
> > Stuff in PrivateHeaders does a #include <JavaScriptCore/Foo.h> so this would likely catch any compile errors. Also LLInt needs these headers installed.
> 
> I don't think it makes any sense to copy layout of macOS framework when
> there is no actual framework built.
> 
> Unless we are 100% sure that we don't want to use built-in cmake's support
> for frameworks on macOS, and even then having .framework on other platforms
> may be confusing

Eventually I would move this particular function into WEBKIT_FRAMEWORK so then it would enumerate the headers the CMake way for Cocoa frameworks. For the other ones it would just emulate the behavior.

> 
> >> Source/cmake/WebKitMacros.cmake:265
> >> +            add_custom_command(TARGET ${framework} PRE_LINK
> > 
> > So this moves the DerivedSources to being copied within the original ${framework} as a PRE_LINK step which happens before a POST_BUILD. Seems like a safe bet that things are going to be ok at this point.
> > 
> > I need to see how things work with WebCore to really determine if this will work across the board.
> > 
> > It seems to not cause rebuilds within Visual Studio so I think it solves things. Placing them within PrivateFrameworkHeaders causes the rebuilds in visual studio.
> 
> Did you check with Ninja generator? In general it's better to stay away from
> PRE_LINK, POST_BUILD etc., these things don't have well-defined behavior
> outside of Visual Studio generator

According to the docs https://cmake.org/cmake/help/v3.11/command/add_custom_command.html#build-events its PRE_BUILD that is Visual Studio only. It makes it sound like PRE_LINK and POST_BUILD are ok to use on any generator.
Comment 13 Fujii Hironori 2018-02-18 22:53:49 PST
I want to test your patch, but can't apply. Please remake a patch.
Comment 14 Konstantin Tokarev 2018-02-19 02:04:50 PST
(In reply to Don Olmstead from comment #12)
> Eventually I would move this particular function into WEBKIT_FRAMEWORK so
> then it would enumerate the headers the CMake way for Cocoa frameworks. For
> the other ones it would just emulate the behavior.

In CMake, .framework directory is the target, it's created by CMake and its contents are managed by CMake
Comment 15 Don Olmstead 2018-02-21 17:37:10 PST
Created attachment 334430 [details]
WIP Patch
Comment 16 Fujii Hironori 2018-02-21 19:55:32 PST
I created a example demonstrating Ninja dependency issue (Comment 1).
https://gist.github.com/fujii/1746bb1ea190944d0817c2f5c9aabdd1

> [2/4] /usr/lib/ccache/cc    -MD -MT CMakeFiles/TestWebCore.dir/main.c.o -MF CMakeFiles/TestWebCore.dir/main.c.o.d -o CMakeFiles/TestWebCore.dir/main.c.o   -c ../main.c
> [3/4] cd /home/fujii/tmp/cmake-ninja/build && /usr/bin/cmake -E echo hello && cd /home/fujii/tmp/cmake-ninja/build && /usr/bin/cmake -E remove libWebCore.a && /usr/bin/ar qc libWebCore.a  CMakeFiles/WebCore.dir/foo.c.o && /usr/bin/ranlib libWebCore.a && :

main.c was compiled before PRE_LINK command was executed.

> build CMakeFiles/TestWebCore.dir/main.c.o: C_COMPILER__TestWebCore ../main.c || cmake_object_order_depends_target_TestWebCore

That's because main.c.o doesn't have a dependency to the PRE_LINK command.
Comment 17 Konstantin Tokarev 2018-02-22 11:36:17 PST
(In reply to Don Olmstead from comment #12)
> According to the docs
> https://cmake.org/cmake/help/v3.11/command/add_custom_command.html#build-
> events its PRE_BUILD that is Visual Studio only. It makes it sound like
> PRE_LINK and POST_BUILD are ok to use on any generator.

I didn't say that they won't work, but behavior (i.e. resulting build order) may differ. It's better to use explicit dependencies on other files or targets everywhere
Comment 18 Don Olmstead 2018-02-22 17:20:39 PST
Created attachment 334487 [details]
WIP Patch
Comment 19 Don Olmstead 2018-02-22 17:33:52 PST
Comment on attachment 334487 [details]
WIP Patch

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

This adds the headers from the original WebCore patch as well to see if we get through the regression.

Still work in progress but feels like its getting closer to shippable.

Can you all take a look? I'm wondering if I can split up the WEBKIT_MAKE_FRAMEWORK_HEADERS to call a different function to avoid duplication.

> Source/cmake/WebKitMacros.cmake:297
> +    if (_derived_sources)
> +        if (ON)
> +            set(_build_file ${DERIVED_SOURCES_DIR}/${framework}/prelink.cmd)
> +            file(WRITE ${_build_file})
> +        else ()
> +            set(_build_file ${DERIVED_SOURCES_DIR}/${framework}/prelink.sh)
> +            file(WRITE ${_build_file} "set -ex")
> +        endif ()
> +
> +        foreach (_header IN LISTS _derived_sources)
> +            get_filename_component(_header_filename ${_header} NAME)
> +            set(_framework_header ${_private_destination}/${_header_filename})
> +        
> +            file(APPEND ${_build_file} "${CMAKE_COMMAND} -E copy_if_different ${_header} ${_framework_header}\n")
> +        endforeach ()
> +
> +        add_custom_command(TARGET ${framework} PRE_LINK
> +            COMMAND ${_build_file}
> +            DEPENDS ${_derived_sources}
> +            VERBATIM
> +        )
> +    endif ()

On Ninja the PRE_LINK command is too long to run. To work around this I'm making a file that is then invoked.

This was actually fixed recently in CMake https://gitlab.kitware.com/cmake/cmake/merge_requests/1604 but it'll be forever before everyone is on a latest release.
Comment 20 Don Olmstead 2018-02-22 18:09:01 PST
Created attachment 334488 [details]
WIP Patch
Comment 21 EWS Watchlist 2018-02-22 18:10:14 PST
Attachment 334488 [details] did not pass style-queue:


ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npruntime.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npfunctions.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginView.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/Plugins/PluginPackage.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:47:  "CoreFoundation/CoreFoundation.h" already included at Source/WebKitLegacy/win/WebKitPrefix.h:38  [build/include] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:57:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/TestWebKitAPI/PlatformGTK.cmake:19:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/cmake/WebKitMacros.cmake:288:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/WebCore/CMakeLists.txt:3298:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npapi.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginStream.h:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/CMakeLists.txt:1179:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/TestWebKitAPI/PlatformWin.cmake:123:  No trailing spaces  [whitespace/trailing] [5]
Total errors found: 14 in 46 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 22 Don Olmstead 2018-02-22 18:32:45 PST
Created attachment 334493 [details]
WIP Patch
Comment 23 EWS Watchlist 2018-02-22 18:35:32 PST
Attachment 334493 [details] did not pass style-queue:


ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npruntime.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npfunctions.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginView.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/Plugins/PluginPackage.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:47:  "CoreFoundation/CoreFoundation.h" already included at Source/WebKitLegacy/win/WebKitPrefix.h:38  [build/include] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:57:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/TestWebKitAPI/PlatformGTK.cmake:19:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/cmake/WebKitMacros.cmake:288:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/WebCore/CMakeLists.txt:3296:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npapi.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginStream.h:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/CMakeLists.txt:1179:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/TestWebKitAPI/PlatformWin.cmake:123:  No trailing spaces  [whitespace/trailing] [5]
Total errors found: 14 in 47 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 24 Don Olmstead 2018-02-22 19:01:30 PST
Created attachment 334497 [details]
WIP Patch
Comment 25 EWS Watchlist 2018-02-22 19:04:13 PST
Attachment 334497 [details] did not pass style-queue:


ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npruntime.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npfunctions.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginView.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/Plugins/PluginPackage.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:47:  "CoreFoundation/CoreFoundation.h" already included at Source/WebKitLegacy/win/WebKitPrefix.h:38  [build/include] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:57:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/TestWebKitAPI/PlatformGTK.cmake:19:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/cmake/WebKitMacros.cmake:290:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/WebCore/CMakeLists.txt:3296:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npapi.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginStream.h:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/CMakeLists.txt:1179:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/TestWebKitAPI/PlatformWin.cmake:123:  No trailing spaces  [whitespace/trailing] [5]
Total errors found: 14 in 47 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 26 Don Olmstead 2018-02-22 19:08:19 PST
Created attachment 334498 [details]
Patch

Maybe make GTK happier
Comment 27 EWS Watchlist 2018-02-22 19:11:22 PST
Attachment 334498 [details] did not pass style-queue:


ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npruntime.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npfunctions.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginView.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/Plugins/PluginPackage.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:47:  "CoreFoundation/CoreFoundation.h" already included at Source/WebKitLegacy/win/WebKitPrefix.h:38  [build/include] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:57:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/TestWebKitAPI/PlatformGTK.cmake:19:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/cmake/WebKitMacros.cmake:290:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/WebCore/CMakeLists.txt:3296:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npapi.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginStream.h:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/CMakeLists.txt:1179:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/TestWebKitAPI/PlatformWin.cmake:123:  No trailing spaces  [whitespace/trailing] [5]
Total errors found: 14 in 47 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 28 Don Olmstead 2018-02-22 19:15:41 PST
Created attachment 334500 [details]
Patch
Comment 29 EWS Watchlist 2018-02-22 19:17:57 PST
Attachment 334500 [details] did not pass style-queue:


ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npruntime.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npfunctions.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginView.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/Plugins/PluginPackage.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:47:  "CoreFoundation/CoreFoundation.h" already included at Source/WebKitLegacy/win/WebKitPrefix.h:38  [build/include] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:57:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/TestWebKitAPI/PlatformGTK.cmake:19:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/cmake/WebKitMacros.cmake:290:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/WebCore/CMakeLists.txt:3296:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npapi.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginStream.h:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/CMakeLists.txt:1179:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/TestWebKitAPI/PlatformWin.cmake:123:  No trailing spaces  [whitespace/trailing] [5]
Total errors found: 14 in 47 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 30 Don Olmstead 2018-02-22 19:26:25 PST
Created attachment 334501 [details]
WIP Patch
Comment 31 EWS Watchlist 2018-02-22 19:29:05 PST
Attachment 334501 [details] did not pass style-queue:


ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npruntime.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npfunctions.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginView.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/Plugins/PluginPackage.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:47:  "CoreFoundation/CoreFoundation.h" already included at Source/WebKitLegacy/win/WebKitPrefix.h:38  [build/include] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:57:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/TestWebKitAPI/PlatformGTK.cmake:19:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/cmake/WebKitMacros.cmake:290:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/WebCore/CMakeLists.txt:3296:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npapi.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginStream.h:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/CMakeLists.txt:1179:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/TestWebKitAPI/PlatformWin.cmake:123:  No trailing spaces  [whitespace/trailing] [5]
Total errors found: 14 in 47 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 32 Don Olmstead 2018-02-22 19:34:53 PST
Created attachment 334502 [details]
WIP Patch
Comment 33 EWS Watchlist 2018-02-22 19:36:31 PST
Attachment 334502 [details] did not pass style-queue:


ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npruntime.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npfunctions.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginView.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/Plugins/PluginPackage.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:47:  "CoreFoundation/CoreFoundation.h" already included at Source/WebKitLegacy/win/WebKitPrefix.h:38  [build/include] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:57:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/TestWebKitAPI/PlatformGTK.cmake:19:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/cmake/WebKitMacros.cmake:290:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/WebCore/CMakeLists.txt:3296:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npapi.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginStream.h:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/CMakeLists.txt:1179:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/TestWebKitAPI/PlatformWin.cmake:123:  No trailing spaces  [whitespace/trailing] [5]
Total errors found: 14 in 47 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 34 Don Olmstead 2018-02-23 11:29:55 PST
Created attachment 334540 [details]
WIP Patch

GTK built through JSC for me with this one so lets give it another go
Comment 35 EWS Watchlist 2018-02-23 11:31:00 PST
Attachment 334540 [details] did not pass style-queue:


ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npruntime.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npfunctions.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginView.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/Plugins/PluginPackage.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:47:  "CoreFoundation/CoreFoundation.h" already included at Source/WebKitLegacy/win/WebKitPrefix.h:38  [build/include] [4]
ERROR: Source/WebKitLegacy/win/WebKitPrefix.h:57:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Tools/TestWebKitAPI/PlatformGTK.cmake:19:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/cmake/WebKitMacros.cmake:290:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Source/WebCore/CMakeLists.txt:3296:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/DumpRenderTree/TestNetscapePlugIn/ForwardingHeaders/WebKit/npapi.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebKitLegacy/win/Plugins/PluginStream.h:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/CMakeLists.txt:1179:  No trailing spaces  [whitespace/trailing] [5]
ERROR: Tools/TestWebKitAPI/PlatformWin.cmake:123:  No trailing spaces  [whitespace/trailing] [5]
Total errors found: 14 in 47 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 36 Fujii Hironori 2018-05-06 20:33:28 PDT
CMake finally has fixed the issue generated VS project invokes a custom command twice.
https://gitlab.kitware.com/cmake/cmake/issues/16767
Comment 37 Fujii Hironori 2019-04-01 21:03:26 PDT
Created attachment 366469 [details]
WIP patch

* Rebased comment#2 patch on to ToT
Comment 38 Fujii Hironori 2019-04-01 22:30:44 PDT
Created attachment 366471 [details]
WIP patch
Comment 39 Fujii Hironori 2019-04-02 00:23:45 PDT
Created attachment 366472 [details]
Patch
Comment 40 WebKit Commit Bot 2019-04-02 09:33:08 PDT
Comment on attachment 366472 [details]
Patch

Clearing flags on attachment: 366472

Committed r243746: <https://trac.webkit.org/changeset/243746>
Comment 41 WebKit Commit Bot 2019-04-02 09:33:10 PDT
All reviewed patches have been landed.  Closing bug.
Comment 42 Radar WebKit Bug Importer 2019-04-02 13:45:36 PDT
<rdar://problem/49532530>