bitwiseworks / qtwebengine-chromium-os2

Port of Chromium and related tools to OS/2
9 stars 2 forks source link

Align to ICU 68 #29

Closed dmik closed 3 years ago

dmik commented 3 years ago

Turns out that Chromium's V8 (JS engine implementation) uses this draft function from ICU (from here https://github.com/bitwiseworks/icu-os2/blob/3e4343c5f41e6d2faa2a8874ca124afd5913a5e4/source/i18n/unicode/listformatter.h#L223):

#ifndef U_HIDE_DRAFT_API
    /**
     * Format a list of strings.
     *
     * @param items     An array of strings to be combined and formatted.
     * @param n_items   Length of the array items.
     * @param appendTo  The string to which the formatted result will be
     *                  appended.
     * @param posIter   On return, can be used to iterate over positions of
     *                  fields generated by this format call. Field values are
     *                  defined in UListFormatterField. Can be NULL.
     * @param errorCode ICU error code returned here.
     * @return          Formatted string combining the elements of items,
     *                  appended to appendTo.
     * @draft ICU 63
     */
    UnicodeString& format(const UnicodeString items[], int32_t n_items,
        UnicodeString & appendTo, FieldPositionIterator* posIter,
        UErrorCode& errorCode) const;
#endif // U_HIDE_DRAFT_API

in these 3 sources:

v8\src\objects\js-date-time-format.cc
v8\src\objects\js-list-format.cc
v8\src\objects\js-number-format.cc

In ICU 67.1 this function was deprecated and completely removed in favor of another function, see https://unicode-org.atlassian.net/browse/ICU-20809. So, when using the latest ICU 68.1 RPMs from netlabs-exp, with the current test Chromium build, it ends up in a POPUPLOG.OS2 entry claiming that the respective export is missing (currently Chromium is built against our last stable ICU 65.1 from netlabs-rel).

Note that we can't just rebuild Chromium against ICU 68.1 since the function is completely missing there. We need to switch V8 to the new API formatStringsToValue as upstream did long ago (check https://chromium.googlesource.com/v8/v8/+/refs/heads/master/src/objects/js-list-format.cc).

Our Chromium build in Qt is quite old already (late 2019) so no surprise ICU 68 is much newer.

dmik commented 3 years ago

As a temporary solution, one may downgrade ICU 68.1 RPMs to ICU 65.1.

dmik commented 3 years ago

This is the diff in question, I hope it will apply cleanly: https://chromium.googlesource.com/v8/v8/+/49569e5aed4894f18b701ffe3fa335daac6202d7.

dmik commented 3 years ago

Works like a charm with the latest ICU from exp now. Note that newer V8 also uses ICU draft API so we can't disable it in our ICU RPM builds. Will have to deal with that just like above if it breaks again.

dmik commented 3 years ago

There is actually more stuff to fix, the current tree can't be clearly built, reopening this.

E.g.

g++ -MMD -MF obj/base/i18n/string_compare.o.d -DBASE_I18N_IMPLEMENTATION -DUSE_AURA=1 -DUSE_NSS_CERTS=1 -DNO_TCMALLOC -DCHROMIUM_BUILD -DTOOLKIT_QT -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DUSING_SYSTEM_ICU=1 -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC -Igen -I../../../../../qt5/qtwebengine/src/3rdparty/chromium -Igen -Igen -Igen -I../../../../../qt5/qtwebengine/src/3rdparty/chromium/third_party/ced/src -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -funwind-tables -Zomf -m32 -msse2 -mfpmath=sse -mmmx -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wno-unused-local-typedefs -Wno-maybe-uninitialized -Wno-deprecated-declarations -Wno-comments -Wno-dangling-else -Wno-packed-not-aligned -Wno-missing-field-initializers -Wno-unused-parameter -fomit-frame-pointer -s -O2 -std=gnu++14 -Wno-narrowing -Wno-attributes -Wno-class-memaccess -Wno-subobject-linkage -Wno-invalid-offsetof -fno-exceptions -fno-rtti -c ../../../../../qt5/qtwebengine/src/3rdparty/chromium/base/i18n/string_compare.cc -o obj/base/i18n/string_compare.o
../../../../../qt5/qtwebengine/src/3rdparty/chromium/base/i18n/string_compare.cc: In function 'UCollationResult base::i18n::CompareString16WithCollator(const icu::Collator&, const string16&, const string16&)':
../../../../../qt5/qtwebengine/src/3rdparty/chromium/base/i18n/string_compare.cc:21:26: error: 'FALSE' was not declared in this scope
   21 |       icu::UnicodeString(FALSE, lhs.c_str(), static_cast<int>(lhs.length())),
      |                          ^~~~~

We fixed some of these places in 3368fd62577558e29b7ba3c97e28e7f57ce2601e but apparently there are more files to fix. This is the original Google commit that fixes it all, I will try to cherry pick it: https://chromium.googlesource.com/chromium/src.git/+/4d16e52a5e6771c4aa5f892e14486bf0e87027d1