Repository navigation
feat: build V8 with ICU so Intl is available - #9
Draft
aleclarson wants to merge 1 commit into
Draft
aleclarson wants to merge 1 commit into
aleclarson wants to merge 1 commit into
Conversation
Both scripts carried v8_enable_i18n_support=false over from the 10.3
build. That removes Intl entirely and leaves localeCompare,
toLocale*String, locale case mapping, String#normalize and RegExp \p{...}
missing or locale-blind on Android and iOS.
Turn i18n on and keep icu_use_data_file=false, so the ICU data is linked
into the libraries: V8::InitializeICU is then a no-op and neither runtime
needs code changes or an icudtl.dat to load. On iOS the ICU objects,
data included, land in libv8_third_party through the existing
obj/third_party sweep.
Refs NativeScript/NativeScript#11563
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs NativeScript/NativeScript#11563
Summary
Both matrix scripts set
v8_enable_i18n_support=false, carried over from the 10.3 build. That removesIntlentirely. It also leaveslocaleCompare,toLocale*String, locale case mapping,String#normalizeand RegExp\p{…}missing or locale-blind on Android and iOS.This PR turns i18n on and keeps
icu_use_data_file=false, so the ICU data is linked into the libraries:third_party/icu/BUILD.gn)icudtl.datsizeandroidiosuse_blink=true)commonWhy embedded data
V8::InitializeICUreturnstruewithout doing anything (src/init/icu_util.cc), so neither runtime has to find, extract or map anicudtl.dat.icudataobject land underobj/third_party/icu, which the existinglibv8_third_partysweep already picks up. On Android,v8_monolithis a complete static lib and includes them too.ucol_open_<ver>), so it can't collide with Apple'slibicucore.This is meant to settle the open questions in the issue (on by default or a separate flavor, embedded data or one shared data file) with real numbers. A shared data file would store the data once per APK instead of once per ABI. That's a follow-up for both runtimes if the size here is too high.
Size
Baseline is release
v8-14.9.207.39-8. I'll fill in the new column from this PR's CI artifacts. Archive size is an upper bound. The real app-size delta needs a runtime linked against these libraries, since the linker drops unused ICU code.Known follow-ups in the runtimes (not part of this PR)
LANG/LC_*), which Android and iOS apps don't set, so V8 falls back toen-US. Explicit locales (new Intl.NumberFormat('de')) work. Following the device locale needs the runtimes to set ICU's default locale beforeV8::Initialize.persist.sys.timezone, so that should just work. iOS should be checked on device.libNativeScript.so/NativeScript.framework. Worth checking once linked.Test plan
python3 tests/apple-args.py: newtest_every_variant_embeds_icuchecks all seven Apple variants getv8_enable_i18n_support=trueandicu_use_data_file=falsetypeof Intl === 'object',new Intl.PluralRules('en').select(1),/\p{L}/u.test('é')and'a'.localeCompare('B', 'en')