Bug 182757 – [CMake] WEBKIT_MAKE_FORWARDING_HEADERS shouldn't use POST_BUILD to copy generated headers

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Fujii Hironori]

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"
          ^~~~~~~~~~~~~~~~~~~~

[Fujii Hironori]

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.

[Fujii Hironori]

Created attachment 333764 [details]
WIP patch

This patch depends on Bug 182512.

[Don Olmstead]

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.

[Fujii Hironori]

(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.

[Fujii Hironori]

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.

[Don Olmstead]

(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?

[Don Olmstead]

Created attachment 333993 [details]
WIP Patch

Here's an idea for something that might work for you to check out Fujii.

[Fujii Hironori]

(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)

[Don Olmstead]

Created attachment 334072 [details]
WIP Patch

[Don Olmstead]

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.

[Konstantin Tokarev]

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

[Don Olmstead]

(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.

[Fujii Hironori]

I want to test your patch, but can't apply. Please remake a patch.

[Konstantin Tokarev]

(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

[Don Olmstead]

Created attachment 334430 [details]
WIP Patch

[Fujii Hironori]

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.

[Konstantin Tokarev]

(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

[Don Olmstead]

Created attachment 334487 [details]
WIP Patch

[Don Olmstead]

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.

[Don Olmstead]

Created attachment 334488 [details]
WIP Patch

[EWS Watchlist]

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.

[Don Olmstead]

Created attachment 334493 [details]
WIP Patch

[EWS Watchlist]

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.

[Don Olmstead]

Created attachment 334497 [details]
WIP Patch

[EWS Watchlist]

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.

[Don Olmstead]

Created attachment 334498 [details]
Patch

Maybe make GTK happier

[EWS Watchlist]

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.

[Don Olmstead]

Created attachment 334500 [details]
Patch

[EWS Watchlist]

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.

[Don Olmstead]

Created attachment 334501 [details]
WIP Patch

[EWS Watchlist]

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.

[Don Olmstead]

Created attachment 334502 [details]
WIP Patch

[EWS Watchlist]

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.

[Don Olmstead]

Created attachment 334540 [details]
WIP Patch

GTK built through JSC for me with this one so lets give it another go

[EWS Watchlist]

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.

[Fujii Hironori]

CMake finally has fixed the issue generated VS project invokes a custom command twice.
https://gitlab.kitware.com/cmake/cmake/issues/16767

[Fujii Hironori]

Created attachment 366469 [details]
WIP patch

* Rebased comment#2 patch on to ToT

[Fujii Hironori]

Created attachment 366471 [details]
WIP patch

[Fujii Hironori]

Created attachment 366472 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 366472 [details]
Patch

Clearing flags on attachment: 366472

Committed r243746: <https://trac.webkit.org/changeset/243746>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/49532530>