Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2174993002:
Support Big Endian in ICU: part 3 (Closed)
|
Created:
4 years, 5 months ago by jungshik at Google Modified:
4 years, 4 months ago CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/deps/icu.git@master Target Ref:
refs/heads/master Visibility:
Public. |
DescriptionSupport Big Endian in ICU: part 3
Add big endian (mips and mips64) support to BUILD.gn
- icudt[lb]_dat.S (assembly source) is generated at build-time from
icudt[lb].dat (data bundle) when icu_use_data_file is false.
- icudt[lb].dat is copied depending on the endinaness when icu_use_data_file
is true.
Part 1: https://codereview.chromium.org/2162393003
Part 2: https://codereview.chromium.org/2165403003
It works on Linux (both Chrome and v8) and Mac (v8).
Android was also tested by building base_unittests target with
icu_use_data_file=true/false.
v8 does not yet support mips/mips64 in GN so that this CL cannot be
tested with target_cpu={mips,mips64}.
With Chrome on Mac, TEST below has to be done with
https://codereview.chromium.org/2181043003 for http://crbug.com/630929.
Windows should not be affected at all.
BUG=v8:4828
TEST='gn args <builddir>' with icu_use_data_file set to true or false
TEST=build base_unittests and run with --gtest_filter=ICU*
TEST=build base_unittests and run with --gtest_filter=Message*ormat*
TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")`
R=machenbach@chromium.org
Committed: https://chromium.googlesource.com/chromium/deps/icu/+/ec9c1133693148470ffe2e5e53576998e3650c1d
Patch Set 1 #Patch Set 2 : now working #
Total comments: 6
Patch Set 3 : simplify per reviewer comment #Messages
Total messages: 30 (15 generated)
Description was changed from ========== Support Big Endian part 3 Add big endian (mips and mips64) support to BUILD.gn BUG=v8: TEST=.... ========== to ========== Support Big Endian part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* on Mac/Linux ==========
Description was changed from ========== Support Big Endian part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* on Mac/Linux ========== to ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* on Mac/Linux ==========
Description was changed from ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* on Mac/Linux ========== to ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* on Mac/Linux ==========
Description was changed from ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* on Mac/Linux ========== to ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). With Chrome on Mac, GN port fails even without this CL when icu_use_data_file is false. (see http://crbug.com/630929). That is likely to be fixed on the Chromium-side. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` ==========
Description was changed from ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). With Chrome on Mac, GN port fails even without this CL when icu_use_data_file is false. (see http://crbug.com/630929). That is likely to be fixed on the Chromium-side. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` ========== to ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). With Chrome on Mac, GN port fails even without this CL when icu_use_data_file is false. (see http://crbug.com/630929). That is likely to be fixed on the Chromium-side. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build base_unittests and run with --gtest_filter=*essage*ormat* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` ==========
Description was changed from ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). With Chrome on Mac, GN port fails even without this CL when icu_use_data_file is false. (see http://crbug.com/630929). That is likely to be fixed on the Chromium-side. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build base_unittests and run with --gtest_filter=*essage*ormat* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` ========== to ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). With Chrome on Mac, GN port fails even without this CL when icu_use_data_file is false. (see http://crbug.com/630929). That is likely to be fixed on the Chromium-side. Windows should not be affected at all. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build base_unittests and run with --gtest_filter=*essage*ormat* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` ==========
Description was changed from ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). With Chrome on Mac, GN port fails even without this CL when icu_use_data_file is false. (see http://crbug.com/630929). That is likely to be fixed on the Chromium-side. Windows should not be affected at all. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build base_unittests and run with --gtest_filter=*essage*ormat* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` ========== to ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). Android was also tested by building base_unittests target with icu_use_data_file=true/false. With Chrome on Mac, GN port fails even without this CL when icu_use_data_file is false. (see http://crbug.com/630929). That is likely to be fixed on the Chromium-side. Windows should not be affected at all. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build base_unittests and run with --gtest_filter=*essage*ormat* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` ==========
jshin@chromium.org changed reviewers: + brettw@chromium.org, hablich@chromium.org, jochen@chromium.org
Can you take a look? Thanks
jshin@chromium.org changed reviewers: + thakis@chromium.org
There's a build difference between gyp and GN when icu_use_data_file=false (GN) / icu_use_data_file_flag=0 (gyp). With gyp, the final target binary (e.g. d8, base_unittests, chrome) is statically linked to libicudata.a. OTOH, with GN, libicuuc.so is statically linked to libicudata.a and the final target binary's size didn't change. However, both d8 and base_unittests work as expected. Both results are obtained with component_build. A lot more important case (the default case of icu_use_data_file=true / icu_use_data_file_flag=1), there's no difference (well, icudtl.dat is a file to be read at start-up).
jochen@chromium.org changed reviewers: + machenbach@chromium.org
Description was changed from ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). Android was also tested by building base_unittests target with icu_use_data_file=true/false. With Chrome on Mac, GN port fails even without this CL when icu_use_data_file is false. (see http://crbug.com/630929). That is likely to be fixed on the Chromium-side. Windows should not be affected at all. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build base_unittests and run with --gtest_filter=*essage*ormat* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` ========== to ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). Android was also tested by building base_unittests target with icu_use_data_file=true/false. With Chrome on Mac, GN port fails even without this CL when icu_use_data_file is false. (see http://crbug.com/630929). That is likely to be fixed on the Chromium-side. Windows should not be affected at all. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build base_unittests and run with --gtest_filter=*essage*ormat* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` ==========
jshin@chromium.org changed reviewers: - hablich@chromium.org
Ignore the trybots. I'm experimenting with enabling icu changes on v8 trybots directly.
lgtm fyi: If you'd rebase this change to https://codereview.chromium.org/2182943002/ you would be able to run v8 trybots (hopefully). https://codereview.chromium.org/2174993002/diff/20001/BUILD.gn File BUILD.gn (right): https://codereview.chromium.org/2174993002/diff/20001/BUILD.gn#newcode584 BUILD.gn:584: ] Readability suggestion: Maybe move the case distinction to toplevel and define all required variables once (independent of data_file or not data_file). Like that there won't be more case distinctions in the code below. E.g. once: if (is_android) { data_bundle_source = ... data_bundle_out = ... data_assembly = ... } else if (current_cpu == "mips" || current_cpu == "mips64") { ... } else { ... } https://codereview.chromium.org/2174993002/diff/20001/BUILD.gn#newcode642 BUILD.gn:642: args = [ FYI: I assume the action is executed in the root_build_dir. Added: print(data_bundle) print(data_assembly) print(root_build_dir) print(rebase_path(data_bundle, root_build_dir)) print(rebase_path(data_assembly, root_build_dir)) Output for v8: common/icudtl.dat //out/exp/gen/third_party/icu/icudtl_dat.S //out/exp ../../third_party/icu/common/icudtl.dat gen/third_party/icu/icudtl_dat.S
FYI: I was playing with the v8 trybot here: https://codereview.chromium.org/2182553003/
https://codereview.chromium.org/2174993002/diff/20001/BUILD.gn File BUILD.gn (right): https://codereview.chromium.org/2174993002/diff/20001/BUILD.gn#newcode612 BUILD.gn:612: } else { Note that v8 isn't using this case anymore. I don't know who is. V8 uses data file by default.
Thank you, Michael ! https://codereview.chromium.org/2174993002/diff/20001/BUILD.gn File BUILD.gn (right): https://codereview.chromium.org/2174993002/diff/20001/BUILD.gn#newcode584 BUILD.gn:584: ] On 2016/07/26 09:18:56, Michael Achenbach (slow) wrote: > Readability suggestion: Maybe move the case distinction to toplevel and define > all required variables once (independent of data_file or not data_file). Like > that there won't be more case distinctions in the code below. E.g. once: > > if (is_android) { > data_bundle_source = ... > data_bundle_out = ... > data_assembly = ... > } else if (current_cpu == "mips" || current_cpu == "mips64") { > ... > } else { > ... > } Thank you for the suggestion. I did what you proposed except that 'data_assembly' still needs to be if-icu-use-data-file=false block. Otherwise, gn complains about an unused variable when icu_use_data_file=true. https://codereview.chromium.org/2174993002/diff/20001/BUILD.gn#newcode612 BUILD.gn:612: } else { On 2016/07/26 09:29:45, Michael Achenbach (slow) wrote: > Note that v8 isn't using this case anymore. I don't know who is. V8 uses data > file by default. Yeah. I noticed that change made in v8 a few weeks ago. There are some consumers of ICU that prefer to link icudata statically (headless blink for sure. Android webview at least used to, but i"m not sure of its current status). https://codereview.chromium.org/2174993002/diff/20001/BUILD.gn#newcode642 BUILD.gn:642: args = [ On 2016/07/26 09:18:56, Michael Achenbach (slow) wrote: > FYI: I assume the action is executed in the root_build_dir. Added: > > print(data_bundle) > print(data_assembly) > print(root_build_dir) > print(rebase_path(data_bundle, root_build_dir)) > print(rebase_path(data_assembly, root_build_dir)) > > Output for v8: > > common/icudtl.dat > //out/exp/gen/third_party/icu/icudtl_dat.S > //out/exp > ../../third_party/icu/common/icudtl.dat > gen/third_party/icu/icudtl_dat.S > Those values are as expected. Do you see any problem ?
Description was changed from ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). Android was also tested by building base_unittests target with icu_use_data_file=true/false. With Chrome on Mac, GN port fails even without this CL when icu_use_data_file is false. (see http://crbug.com/630929). That is likely to be fixed on the Chromium-side. Windows should not be affected at all. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build base_unittests and run with --gtest_filter=*essage*ormat* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` ========== to ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). Android was also tested by building base_unittests target with icu_use_data_file=true/false. With Chrome on Mac, TEST below has to be done with https://codereview.chromium.org/2181043003 for http://crbug.com/630929. Windows should not be affected at all. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build base_unittests and run with --gtest_filter=Message*ormat* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` ==========
Description was changed from ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). Android was also tested by building base_unittests target with icu_use_data_file=true/false. With Chrome on Mac, TEST below has to be done with https://codereview.chromium.org/2181043003 for http://crbug.com/630929. Windows should not be affected at all. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build base_unittests and run with --gtest_filter=Message*ormat* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` ========== to ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). Android was also tested by building base_unittests target with icu_use_data_file=true/false. v8 does not yet support mips/mips64 in GN so that this CL cannot be tested with target_cpu={mips,mips64}. With Chrome on Mac, TEST below has to be done with https://codereview.chromium.org/2181043003 for http://crbug.com/630929. Windows should not be affected at all. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build base_unittests and run with --gtest_filter=Message*ormat* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` ==========
On 2016/07/26 08:43:53, Michael Achenbach (slow) wrote: > Ignore the trybots. I'm experimenting with enabling icu changes on v8 trybots > directly. Thank you for the attempt. I meant to send an email to the infra-team about enabling ICU changes on both chromium and v8 trybots, but haven't managed to, yet. Well, I've just filed https://bugs.chromium.org/p/chromium/issues/detail?id=631575 and cc'd you there. Thanks again.
On 2016/07/26 09:18:56, Michael Achenbach (slow) wrote: > lgtm > > fyi: If you'd rebase this change to https://codereview.chromium.org/2182943002/ > you would be able to run v8 trybots (hopefully). Thank you for the CL. I missed this. I rebased and try to run a tryjob, but I don't know how v8 or chromium pull a version of ICU with this CL. https://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn... does not seem to do what I expected it to do.
Description was changed from ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). Android was also tested by building base_unittests target with icu_use_data_file=true/false. v8 does not yet support mips/mips64 in GN so that this CL cannot be tested with target_cpu={mips,mips64}. With Chrome on Mac, TEST below has to be done with https://codereview.chromium.org/2181043003 for http://crbug.com/630929. Windows should not be affected at all. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build base_unittests and run with --gtest_filter=Message*ormat* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` ========== to ========== Support Big Endian in ICU: part 3 Add big endian (mips and mips64) support to BUILD.gn - icudt[lb]_dat.S (assembly source) is generated at build-time from icudt[lb].dat (data bundle) when icu_use_data_file is false. - icudt[lb].dat is copied depending on the endinaness when icu_use_data_file is true. Part 1: https://codereview.chromium.org/2162393003 Part 2: https://codereview.chromium.org/2165403003 It works on Linux (both Chrome and v8) and Mac (v8). Android was also tested by building base_unittests target with icu_use_data_file=true/false. v8 does not yet support mips/mips64 in GN so that this CL cannot be tested with target_cpu={mips,mips64}. With Chrome on Mac, TEST below has to be done with https://codereview.chromium.org/2181043003 for http://crbug.com/630929. Windows should not be affected at all. BUG=v8:4828 TEST='gn args <builddir>' with icu_use_data_file set to true or false TEST=build base_unittests and run with --gtest_filter=ICU* TEST=build base_unittests and run with --gtest_filter=Message*ormat* TEST=build 'd8' (v8) and try `(new Date()).toLocaleString("de")` R=machenbach@chromium.org Committed: https://chromium.googlesource.com/chromium/deps/icu/+/ec9c1133693148470ffe2e5... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) manually as ec9c1133693148470ffe2e5e53576998e3650c1d (presubmit successful).
Message was sent while issue was closed.
I thought rebase and reupload would be enough. It should work for newly uploaded CLs. You'll see a "Project: icu" field on the left in Rietveld (at least in the old UI). Like in this CL: https://codereview.chromium.org/2182553003/ V8 trybots work now. Chromium not yet.
Message was sent while issue was closed.
On 2016/07/27 06:08:08, Michael Achenbach (slow) wrote: > I thought rebase and reupload would be enough. It should work for newly uploaded > CLs. You'll see a "Project: icu" field on the left in Rietveld (at least in the > old UI). Like in this CL: > https://codereview.chromium.org/2182553003/ > > V8 trybots work now. Chromium not yet. Thanks. It works ! ! https://codereview.chromium.org/2186133002 (I broke it on purpose :-)).
Message was sent while issue was closed.
Out of interest, why do we care about big endian support? Lots of code in chrome assumes little endian (imo, rightfully so). What's the motivation here? |
