Bug 167343 – webkit-gtk-2.15.3 fails to build on macOS due to missing declaration of U8_MAX_LENGTH

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Jeremy Huddleston Sequoia]

webkit-gtk-2.15.3 fails to build on macOS (darwin/gtk/x11) with the following error.  This is a regression from 2.15.2:

/opt/local/var/macports/build/_Users_jeremy_src_macports_macports-ports_www_webkit2-gtk-devel/webkit2-gtk-devel/work/webkitgtk-2.15.3/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:146:32: error: use of undeclared identifier 'U8_MAX_LENGTH'; did you mean 'UITER_LENGTH'?
        LChar utf8OctetsBuffer[U8_MAX_LENGTH];
                               ^~~~~~~~~~~~~
                               UITER_LENGTH
/opt/local/include/unicode/uiter.h:50:58: note: 'UITER_LENGTH' declared here
    UITER_START, UITER_CURRENT, UITER_LIMIT, UITER_ZERO, UITER_LENGTH
                                                         ^
/opt/local/var/macports/build/_Users_jeremy_src_macports_macports-ports_www_webkit2-gtk-devel/webkit2-gtk-devel/work/webkitgtk-2.15.3/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:151:9: error: use of undeclared identifier 'U8_APPEND_UNSAFE'
        U8_APPEND_UNSAFE(utf8OctetsBuffer, utf8Length, codePoint);
        ^

[John Ralls]

Created attachment 307443 [details]
Rename Source/WTF/wtf/unicode/UTF8.h to .../wtf_utf8.h

This problem has a long history: Bugs 63948, 65811, 70913, 72152, and 88307. The "fix" implemented in a patch at bug 70913 worked until CMake came along, but now CMake's dependency-discovery mechanism appends Source/WTF/wtf to the include list and ICU's #include "unicode/utf8.h" (in unicode/utf.h) finds it again.

This patch renames the offending UTF8.h and UTF8.cpp to wtf_utf8.h and wtf_utf8.cpp, which is what should have been done in the first place.

[John Ralls]

Created attachment 307444 [details]
Rename Source/WTF/wtf/unicode/UTF8.h to .../wtf_utf8.h, this time without the transposition.

[Build Bot]

Attachment 307444 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:65:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:122:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:221:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:234:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:234:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:235:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:236:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:247:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:254:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:256:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:256:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:257:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:257:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:258:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:262:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:262:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:263:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:264:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:265:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:266:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:270:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:288:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:288:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:289:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:290:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:291:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:292:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:330:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.cpp:414:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.h:38:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.h:46:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.h:46:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.h:47:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.h:48:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.h:49:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.h:54:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.h:67:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.h:71:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/unicode/wtf_utf8.h:75:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 40 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Sam Weinig]

(In reply to John Ralls from comment #1)
> Created attachment 307443 [details]
> Rename Source/WTF/wtf/unicode/UTF8.h to .../wtf_utf8.h
> 
> This problem has a long history: Bugs 63948, 65811, 70913, 72152, and 88307.
> The "fix" implemented in a patch at bug 70913 worked until CMake came along,
> but now CMake's dependency-discovery mechanism appends Source/WTF/wtf to the
> include list and ICU's #include "unicode/utf8.h" (in unicode/utf.h) finds it
> again.
> 
> This patch renames the offending UTF8.h and UTF8.cpp to wtf_utf8.h and
> wtf_utf8.cpp, which is what should have been done in the first place.

wtf_utf8.h/cpp doesn't really match out naming at all.  If this rename does need to happen, though I admit, I don't quite understand why that is, it should probably follow the String classes lead, and be called WTFUTF8.h, hideous though that be.

[Michael Catanzaro]

(In reply to Sam Weinig from comment #4)
> wtf_utf8.h/cpp doesn't really match out naming at all.  If this rename does
> need to happen, though I admit, I don't quite understand why that is, it
> should probably follow the String classes lead, and be called WTFUTF8.h,
> hideous though that be.

Yeah, that's the name I would choose. It also matches WTFThreadData.h.

The problem is that both ICU and WTF provide unicode/UTF8.h, they're both in the include path, and the wrong one is being chosen in this case.

[John Ralls]

> Yeah, that's the name I would choose. It also matches WTFThreadData.h.

OK, I'll redo the patch. It'll have to wait till Thursday.

> The problem is that both ICU and WTF provide unicode/UTF8.h, they're both in the include path, and the wrong one is being chosen in this case.

That's half the problem. The other half is that MacOS's default fils system is case-preserving but not case-sensitive. ICU actually provides unicode/utf8.h which is a different filename on most Linux file systems but the same on MacOS.

[Michael Catanzaro]

Comment on attachment 307444 [details]
Rename Source/WTF/wtf/unicode/UTF8.h to .../wtf_utf8.h, this time without the transposition.

Thanks for your patch. This does need some work. First off, and most importantly, the patch should not touch the hand-written Makefiles, since those are part of the XCode build. It should certainly not replace them with CMake-generated files, which are not source and should not be under version control. I haven't reviewed the rest of the changes since the vast majority of the diff is changes to the Makefiles, so I'll look over the rest of the patch once those changes are removed. Note: you should use a separate build directory in order to ensure that generated files like these Makefiles aren't created in the source directory; I'm kind of surprised that an in-tree build worked at all, since we've never tested or supported that.

You can ignore all the complaints from style checker, since this is a straight rename, but you can't ignore the red Early Warning System (EWS) bots. This broke the iOS Simulator, JSC, and Mac EWS bots. The problem is that the XCode build needs to be updated to account for the rename in addition to the CMake build; those bots don't use CMake at all. Normally I would ask someone from Apple to do that, but since you use macOS I guess you can probably handle it yourself? If not, perhaps Sam would be willing to help with that.

Please use Tools/Scripts/prepare-ChangeLog to prepare your changelog entries. In particular, note that you won't actually modify the toplevel ChangeLog file, but the ChangeLog files in the JavaScriptCore, WTF, WebCore, and WebKit2 directories. It will say "Reviewed by NOBODY (OOPS!)" and it's best to leave that in the patch you upload, since our tools will handle replacing it automatically.

[Konstantin Tokarev]

I think we should not rename UTF8.h, but rather wtf/unicode, to prevent possibility of future file name conflicts.

[John Ralls]

> I think we should not rename UTF8.h, but rather wtf/unicode, to prevent possibility of future file name conflicts.

OK, I suppose. A somewhat more extensive change (72 files vs. 14 when changing only UTF8.h & cpp). Would the new directory name be Source/WTF/wtf/WTFunicode?

[John Ralls]

Created attachment 307612 [details]
Rename Source/WTF/wtf/unicode/UTF8.h to .../wtf_utf8.h with corrections as requested.

Sorry about all of the noise in the previous iteration. It didn't occur to me that there'd be hand-built Makefiles when there's a CMake implementation too.

I didn't get any messages from EWS. Should I have?

I did a test-build in Xcode which completed successfully, but that would be of Safari's backend rather than WebKitGtk.

[Build Bot]

Attachment 307612 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/unicode/WTFUTF8.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.h:38:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.h:46:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.h:46:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.h:47:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.h:48:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.h:49:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.h:54:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.h:67:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.h:71:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.h:75:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WebCore/ChangeLog:10:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:65:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:122:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:221:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:234:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:234:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:235:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:236:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:247:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:254:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:256:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:256:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:257:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:257:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:258:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:262:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:262:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:263:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:264:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:265:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:266:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:270:  More than one command on the same line in if  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:288:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:288:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:289:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:290:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:291:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:292:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:330:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
ERROR: Source/WTF/wtf/unicode/WTFUTF8.cpp:414:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 41 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Konstantin Tokarev]

>Would the new directory name be Source/WTF/wtf/WTFunicode?

I'm afraid that would result in too much WTF per minute.

Could you chewck if the following patch fixes the build for you?

diff --git a/Source/WTF/wtf/CMakeLists.txt b/Source/WTF/wtf/CMakeLists.txt
index 1bae3bb90b3..7d902484fcd 100644
--- a/Source/WTF/wtf/CMakeLists.txt
+++ b/Source/WTF/wtf/CMakeLists.txt
@@ -286,6 +286,12 @@ set(WTF_SOURCES
 set(WTF_INCLUDE_DIRECTORIES
     "${BMALLOC_DIR}"
     "${WTF_DIR}"
+    "${THIRDPARTY_DIR}"
+    "${CMAKE_BINARY_DIR}"
+    "${DERIVED_SOURCES_DIR}"
+)
+
+set(WTF_PRIVATE_INCLUDE_DIRECTORIES
     "${WTF_DIR}/wtf"
     "${WTF_DIR}/wtf/dtoa"
     "${WTF_DIR}/wtf/persistence"
@@ -293,9 +299,6 @@ set(WTF_INCLUDE_DIRECTORIES
     "${WTF_DIR}/wtf/text/icu"
     "${WTF_DIR}/wtf/threads"
     "${WTF_DIR}/wtf/unicode"
-    "${THIRDPARTY_DIR}"
-    "${CMAKE_BINARY_DIR}"
-    "${DERIVED_SOURCES_DIR}"
 )
 
 set(WTF_LIBRARIES

[Konstantin Tokarev]

I'm really sorry, here is a complete patch:

diff --git a/Source/WTF/wtf/CMakeLists.txt b/Source/WTF/wtf/CMakeLists.txt
index 1bae3bb90b3..7d902484fcd 100644
--- a/Source/WTF/wtf/CMakeLists.txt
+++ b/Source/WTF/wtf/CMakeLists.txt
@@ -286,6 +286,12 @@ set(WTF_SOURCES
 set(WTF_INCLUDE_DIRECTORIES
     "${BMALLOC_DIR}"
     "${WTF_DIR}"
+    "${THIRDPARTY_DIR}"
+    "${CMAKE_BINARY_DIR}"
+    "${DERIVED_SOURCES_DIR}"
+)
+
+set(WTF_PRIVATE_INCLUDE_DIRECTORIES
     "${WTF_DIR}/wtf"
     "${WTF_DIR}/wtf/dtoa"
     "${WTF_DIR}/wtf/persistence"
@@ -293,9 +299,6 @@ set(WTF_INCLUDE_DIRECTORIES
     "${WTF_DIR}/wtf/text/icu"
     "${WTF_DIR}/wtf/threads"
     "${WTF_DIR}/wtf/unicode"
-    "${THIRDPARTY_DIR}"
-    "${CMAKE_BINARY_DIR}"
-    "${DERIVED_SOURCES_DIR}"
 )
 
 set(WTF_LIBRARIES
diff --git a/Source/cmake/WebKitMacros.cmake b/Source/cmake/WebKitMacros.cmake
index a793ce40683..a0e81ee50dd 100644
--- a/Source/cmake/WebKitMacros.cmake
+++ b/Source/cmake/WebKitMacros.cmake
@@ -283,6 +283,7 @@ macro(WEBKIT_FRAMEWORK _target)
         ${${_target}_SOURCES}
     )
     target_include_directories(${_target} PUBLIC "$<BUILD_INTERFACE:${${_target}_INCLUDE_DIRECTORIES}>")
+    target_include_directories(${_target} PRIVATE "$<BUILD_INTERFACE:${${_target}_PRIVATE_INCLUDE_DIRECTORIES}>")
     target_link_libraries(${_target} ${${_target}_LIBRARIES})
     set_target_properties(${_target} PROPERTIES COMPILE_DEFINITIONS "BUILDING_${_target}")
 
@@ -332,7 +333,7 @@ macro(WEBKIT_CREATE_FORWARDING_HEADERS _framework)
     if (NOT WIN32)
         set(_processing_directories 0)
         set(_processing_files 0)
-        set(_target_directory "${DERIVED_SOURCES_DIR}/ForwardingHeaders/${_framework}")
+        set(_target_directory "${FORWARDING_HEADERS_DIR}/${_framework}")
 
         file(GLOB _files "${_target_directory}/*.h")
         foreach (_file ${_files})

[Konstantin Tokarev]

Created attachment 307617 [details]
Correct patch

Please try attchaed patch

[Konstantin Tokarev]

Created attachment 307619 [details]
patch

[Konstantin Tokarev]

Created attachment 307733 [details]
Patch

[Michael Catanzaro]

Comment on attachment 307733 [details]
Patch

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

Wow.

Please rename the bug to match the changelog title.

> Source/WTF/ChangeLog:10
> +        implicitly without copying directory lists around. This matches exisiting

exisiting -> existing

> Source/WTF/ChangeLog:11
> +        practise for all targets except WTF, headers from which are always included

practise -> practice, unless you want to go full British

[Konstantin Tokarev]

Thanks for proofreading!

>Please rename the bug to match the changelog title.

I think I'll open different bug then in case original issue remains, as I didn't check it (though I believe it should be fixed, also r209665 was right between 2.15.2 and 2.15.3)

[Konstantin Tokarev]

Comment on attachment 307733 [details]
Patch

Moved patch to https://bugs.webkit.org/show_bug.cgi?id=171115

[John Ralls]

Sorry for the delay. Yes, fixing the WTF includes does allow the compile to continue to the next problem (a missing #include "VisibilityChangeClient.h" in WebCore/dom/Document.cpp).

I suggest that you should still change the name of UTF8.h or wtf/unicode as insurance against yet another regression.

Incidentally, another similar problem looms: ICU has added sometime between 1.55 and 1.58 some macros to unicode/platform.h for adding C++11 keywords if the compiler can use them (e.g. U_NOEXCEPT). WebKit has older copies of platform.h in JavaScriptCore/icu/unicode, WebCore/icu/unicode/platform.h, and WTF/icu/unicode/platform.h that are inserted into the include chain for ICU, and that breaks the declarations in the ICU headers because the new macros aren't defined.

[Michael Catanzaro]

We need to make sure these third-party ICU directories are never added to the include path for CMake builds.

[Konstantin Tokarev]

(In reply to Michael Catanzaro from comment #21)
> We need to make sure these third-party ICU directories are never added to
> the include path for CMake builds.

AFAIU, GTK port does not have any of these directories in include path

[Konstantin Tokarev]

(In reply to John Ralls from comment #20)
> I suggest that you should still change the name of UTF8.h or wtf/unicode as
> insurance against yet another regression.

You mean, to hide another regression?

[Michael Catanzaro]

(In reply to Konstantin Tokarev from comment #22)
> (In reply to Michael Catanzaro from comment #21)
> > We need to make sure these third-party ICU directories are never added to
> > the include path for CMake builds.
> 
> AFAIU, GTK port does not have any of these directories in include path

It shouldn't, but then comment #20 argues otherwise....

[John Ralls]

> It shouldn't, but then comment #20 argues otherwise....

Maybe. I should have included that I encountered the error while building an old version of webkit that still used autotools. I haven't yet tried it on master.

[Michael Catanzaro]

OK, we deleted the Autotools build years ago so there's no way your experience there is relevant to current problems. If you have problems with 2.16.1 or master, you can file a new bug. The original problem here is fixed by bug #171115.

[John Ralls]

>> I suggest that you should still change the name of UTF8.h or wtf/unicode as
>> insurance against yet another regression.


> You mean, to hide another regression?

No, to make a regression impossible. Your patch and its predecessor that changed the intentional include path to wtf/unicode/UTF8.h are what keeps masking the actual bug, which is the duplicate filename and copying of ICU's "unicode" namespace.

[Konstantin Tokarev]

I agree that having includes with "unicode/" prefix is asking for trouble, but if we ensure that <wtf/unicode/...> cannot be used as <unicode/...>, I think that's OK.

[John Ralls]

> I agree that having includes with "unicode/" prefix is asking for trouble, 
> but if we ensure that <wtf/unicode/...> cannot be used as <unicode/...>, I 
> think that's OK.

But that's exactly the failure to ensure that that's caused this problem at least twice.

Regardless, don't forget to back-port 215614 to 2.16.

[Carlos Garcia Campos]

(In reply to John Ralls from comment #29)
> > I agree that having includes with "unicode/" prefix is asking for trouble, 
> > but if we ensure that <wtf/unicode/...> cannot be used as <unicode/...>, I 
> > think that's OK.
> 
> But that's exactly the failure to ensure that that's caused this problem at
> least twice.
> 
> Regardless, don't forget to back-port 215614 to 2.16.

Please, when you want something to get backported to 2.16 add it to the wiki, https://trac.webkit.org/wiki/WebKitGTK/2.16.x It's the only way I won't forget. I'm not even CC-ed to this bug. Anyway, I tried to backport it and it failed to build, I've lost the build error, though.

[Carlos Garcia Campos]

(In reply to Carlos Garcia Campos from comment #30)
> (In reply to John Ralls from comment #29)
> > > I agree that having includes with "unicode/" prefix is asking for trouble, 
> > > but if we ensure that <wtf/unicode/...> cannot be used as <unicode/...>, I 
> > > think that's OK.
> > 
> > But that's exactly the failure to ensure that that's caused this problem at
> > least twice.
> > 
> > Regardless, don't forget to back-port 215614 to 2.16.
> 
> Please, when you want something to get backported to 2.16 add it to the
> wiki, https://trac.webkit.org/wiki/WebKitGTK/2.16.x It's the only way I
> won't forget. I'm not even CC-ed to this bug. Anyway, I tried to backport it
> and it failed to build, I've lost the build error, though.

/home/cgarcia/src/git/gnome/WebKit-2.16/Source/WebCore/platform/graphics/texmap/coordinated/TiledBackingStore.cpp:27:39: fatal error: wtf/MemoryPressureHandler.h: No existe el fichero o el directorio
 #include <wtf/MemoryPressureHandler.h>
                                       ^
compilation terminated.

Ok, this is because in 2.16 MemoryPressureHandler.h is still in WebCore/platform, so I'll backport the commit without the TiledBackingStore.cpp change. I hope that's enough to fix the build for you.