Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2738453005:
Try ICU-59-to be with UChar set to {wchart_t, uint16_t} for C API users
|
Created:
3 years, 9 months ago by jungshik at Google Modified:
3 years, 9 months ago Reviewers:
CC:
v8-reviews_googlegroups.com, mscherer, aheninger Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionTry ICU-59-to be with UChar set to {wchart_t, uint16_t} for C API users
ICU's UChar was uint16_t (non-Win) or wchar_t (Windows). It's switching
to char16_t in both C/C++ API. It needs some changes. Fortunately,
v8 needs only a couple of changes because v8 has been using
reinterpret_cast in many places calling ICU API.
This CL rolls ICU to ICU-59-to-be so that it can be tested in trybots.
BUG=v8:6062
TEST=trybot
Patch Set 1 #Patch Set 2 : roll ICU with toUCharPtr and use it #Patch Set 3 : update gyp files and use toUCharPtr #Patch Set 4 : Roll ICU to bb7765c (fix studata.cpp issue in gyp) #Patch Set 5 : only include ICU roll because other files were landed #Patch Set 6 : Roll ICU to 2bccace with /utf-8 set on Windows builds #Patch Set 7 : Roll ICU to 97b9daa (goma work-around/PNaCl fix) #Messages
Total messages: 42 (32 generated)
The CQ bit was checked by jshin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_verify_csa_rel_n...) v8_linux64_verify_csa_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_verify_csa_rel_n...) v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/23870)
The CQ bit was checked by jshin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_android_arm_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...) v8_linux64_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_verify_csa_rel_n...) v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...) v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/23876)
The CQ bit was checked by jshin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Some bot failures are due to crbug.com/606987 (a goma issue).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_verify_csa_rel_n...) v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...) v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...) v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...) v8_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...)
The CQ bit was checked by jshin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_android_arm_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...) v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...) v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/22198) v8_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...)
Description was changed from ========== Prepare for ICU's switch to char16_t ICU's UChar was uint16_t (non-Win) or wchar_t (Windows). It's switching to char16_t in both C/C++ API. It needs some changes. Fortunately, v8 needs only a couple of changes because v8 has been using reinterpret_cast in many places calling ICU API. This change was confirmed to work fine with ICU-59-to-be on Linux. This CL is the same as http://crrev.com/2738503008#ps1 except that it rolls ICU to ICU-59-to-be so that it can be tested in trybots. patch from issue 2738503008 at patchset 1 (http://crrev.com/2738503008#ps1) BUG=v8:6062 TEST=trybot ========== to ========== Prepare for ICU's switch to char16_t ICU's UChar was uint16_t (non-Win) or wchar_t (Windows). It's switching to char16_t in both C/C++ API. It needs some changes. Fortunately, v8 needs only a couple of changes because v8 has been using reinterpret_cast in many places calling ICU API. This change was confirmed to work fine with ICU-59-to-be on Linux. This CL is the same as http://crrev.com/2738503008#ps1 except that it rolls ICU to ICU-59-to-be so that it can be tested in trybots. patch from issue 2738503008 at patchset 1 (http://crrev.com/2738503008#ps1) BUG=v8:6062 TEST=trybot ==========
On 2017/03/09 00:57:35, jungshik at Google wrote: > Some bot failures are due to crbug.com/606987 (a goma issue). Hmm... goma is fine when UCHAR_TYPE is always char16_t (inside or outside ICU).
On 2017/03/09 20:27:57, jungshik at Google wrote: > On 2017/03/09 00:57:35, jungshik at Google wrote: > > Some bot failures are due to crbug.com/606987 (a goma issue). > > Hmm... goma is fine when UCHAR_TYPE is always char16_t (inside or outside ICU). See https://bugs.chromium.org/p/chromium/issues/detail?id=683012#c50
Description was changed from ========== Prepare for ICU's switch to char16_t ICU's UChar was uint16_t (non-Win) or wchar_t (Windows). It's switching to char16_t in both C/C++ API. It needs some changes. Fortunately, v8 needs only a couple of changes because v8 has been using reinterpret_cast in many places calling ICU API. This change was confirmed to work fine with ICU-59-to-be on Linux. This CL is the same as http://crrev.com/2738503008#ps1 except that it rolls ICU to ICU-59-to-be so that it can be tested in trybots. patch from issue 2738503008 at patchset 1 (http://crrev.com/2738503008#ps1) BUG=v8:6062 TEST=trybot ========== to ========== Try ICU-59-to be with UChar set to {wchart_t, uint16_t} for C API users ICU's UChar was uint16_t (non-Win) or wchar_t (Windows). It's switching to char16_t in both C/C++ API. It needs some changes. Fortunately, v8 needs only a couple of changes because v8 has been using reinterpret_cast in many places calling ICU API. This CL rolls ICU to ICU-59-to-be so that it can be tested in trybots. BUG=v8:6062 TEST=trybot ==========
On 2017/03/09 21:20:06, jungshik at Google wrote: > On 2017/03/09 20:27:57, jungshik at Google wrote: > > On 2017/03/09 00:57:35, jungshik at Google wrote: > > > Some bot failures are due to crbug.com/606987 (a goma issue). > > > > Hmm... goma is fine when UCHAR_TYPE is always char16_t (inside or outside > ICU). > > See > https://bugs.chromium.org/p/chromium/issues/detail?id=683012#c50 This is not true. UChar being char16_t everywhere does not make goma happy, either. https://codereview.chromium.org/2740293004 has the same failure as this one. Somehow goma hates ICU 59 ;-).
The CQ bit was checked by jshin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...) v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
On 2017/03/10 20:28:06, jungshik at Google wrote: > On 2017/03/09 21:20:06, jungshik at Google wrote: > > On 2017/03/09 20:27:57, jungshik at Google wrote: > > > On 2017/03/09 00:57:35, jungshik at Google wrote: > > > > Some bot failures are due to crbug.com/606987 (a goma issue). > > > > > > Hmm... goma is fine when UCHAR_TYPE is always char16_t (inside or outside > > ICU). > > > > See > > https://bugs.chromium.org/p/chromium/issues/detail?id=683012#c50 > > This is not true. UChar being char16_t everywhere does not make goma happy, > either. > https://codereview.chromium.org/2740293004 has the same failure as this one. > > Somehow goma hates ICU 59 ;-). Perhaps, it's due to this ICU issue (recursive include) http://bugs.icu-project.org/trac/ticket/13025
On 2017/03/11 00:37:49, jungshik at Google wrote: > On 2017/03/10 20:28:06, jungshik at Google wrote: > > On 2017/03/09 21:20:06, jungshik at Google wrote: > > > On 2017/03/09 20:27:57, jungshik at Google wrote: > > > > On 2017/03/09 00:57:35, jungshik at Google wrote: > > > > > Some bot failures are due to crbug.com/606987 (a goma issue). > > > > > > > > Hmm... goma is fine when UCHAR_TYPE is always char16_t (inside or outside > > > ICU). > > > > > > See > > > https://bugs.chromium.org/p/chromium/issues/detail?id=683012#c50 > > > > This is not true. UChar being char16_t everywhere does not make goma happy, > > either. > > https://codereview.chromium.org/2740293004 has the same failure as this one. > > > > Somehow goma hates ICU 59 ;-). > > Perhaps, it's due to this ICU issue (recursive include) > > http://bugs.icu-project.org/trac/ticket/13025 It cannot be. I misunderstood the ICU bug above.
The CQ bit was checked by jshin@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. Committers are members of the group "project-v8-committers". Note that this has nothing to do with OWNERS files.
The CQ bit was checked by jshin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/03/13 20:26:45, commit-bot: I haz the power wrote: > No L-G-T-M from a valid reviewer yet. > CQ run can only be started by full committers or once the patch has > received an L-G-T-M from a full committer. > Even if an L-G-T-M may have been provided, it was from a non-committer, > _not_ a full super star committer. > Committers are members of the group "project-v8-committers". > Note that this has nothing to do with OWNERS files. ooops. meant to run a dryrun.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/24022)
The CQ bit was checked by jshin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/22517) v8_linux64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng_triggered...)
v8 builds went well everywhere except that Etc/GMT, Etc/UTC timezone name canonicalization behavior changes lead Ecma tests to fail. |
