build: support --icu-data-dir configure switch

[misterdjules]

A few comments about this PR:

• We may want to remove the usage of --icu-data-dir in the Makefile's target that builds the OS X package, since we haven't solved the cross-compilation issue (see build: fix icu support when target arch != host #12) yet and we need it for OS X packages.

EDIT: the cross-compilation issue has been fixed by a recent change in the srl-v0.12-autoicu branch.

• It seems that our current V8 doesn't handle the situation when ICU is linked with only the stubbed data and the .dat file cannot be found. It segfaults with the following call stack:

* thread #1: tid = 0x266c56, 0x000000010035a5b3 node`v8::internal::(anonymous namespace)::SetResolvedDateSettings(isolate=0x0000000101804c00, icu_locale=0x00007fff5fbfea80, date_format=0x0000000000000000, resolved=<unavailable>) + 99 at i18n.cc:132, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x000000010035a5b3 node`v8::internal::(anonymous namespace)::SetResolvedDateSettings(isolate=0x0000000101804c00, icu_locale=0x00007fff5fbfea80, date_format=0x0000000000000000, resolved=<unavailable>) + 99 at i18n.cc:132
   129    Factory* factory = isolate->factory();
   130    UErrorCode status = U_ZERO_ERROR;
   131    icu::UnicodeString pattern;
-> 132    date_format->toPattern(pattern);
   133    JSObject::SetProperty(
   134        resolved,
   135        factory->NewStringFromStaticAscii("pattern"),
(lldb) bt
* thread #1: tid = 0x266c56, 0x000000010035a5b3 node`v8::internal::(anonymous namespace)::SetResolvedDateSettings(isolate=0x0000000101804c00, icu_locale=0x00007fff5fbfea80, date_format=0x0000000000000000, resolved=<unavailable>) + 99 at i18n.cc:132, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010035a5b3 node`v8::internal::(anonymous namespace)::SetResolvedDateSettings(isolate=0x0000000101804c00, icu_locale=0x00007fff5fbfea80, date_format=0x0000000000000000, resolved=<unavailable>) + 99 at i18n.cc:132
    frame #1: 0x000000010035a351 node`v8::internal::DateFormat::InitializeDateTimeFormat(isolate=0x0000000101804c00, locale=<unavailable>, options=<unavailable>, resolved=<unavailable>) + 353 at i18n.cc:775
    frame #2: 0x000000010045bdaa node`v8::internal::Runtime_CreateDateTimeFormat(int, v8::internal::Object**, v8::internal::Isolate*) [inlined] v8::internal::__RT_impl_Runtime_CreateDateTimeFormat(isolate=0x0000000101804c00, arguments=0x00007fff5fbff048) + 185 at runtime.cc:14055
    frame #3: 0x000000010045bcf1 node`v8::internal::Runtime_CreateDateTimeFormat(args_length=<unavailable>, args_object=0x00007fff5fbff048, isolate=0x0000000101804c00) + 17 at runtime.cc:14036
    frame #4: 0x000027480ab0740e
    frame #5: 0x000027480aba9d00
    frame #6: 0x000027480aba9688
    frame #7: 0x000027480ab07b75
    frame #8: 0x000027480ab32de2
    frame #9: 0x000027480aba94bf
    frame #10: 0x000027480aba7d03
    frame #11: 0x000027480aba7ac9
    frame #12: 0x000027480ab07b75
    frame #13: 0x000027480aba55e7
    frame #14: 0x000027480ab5e0a6
    frame #15: 0x000027480aba43a6
    frame #16: 0x000027480ab9e9ac
    frame #17: 0x000027480ab9b1e0
    frame #18: 0x000027480ab91d45
    frame #19: 0x000027480ab91704
    frame #20: 0x000027480ab6451f
    frame #21: 0x000027480ab62f10
    frame #22: 0x000027480ab5b1c0
    frame #23: 0x000027480ab28d31
    frame #24: 0x0000000100210603 node`v8::internal::Invoke(is_construct=<unavailable>, argc=1, args=0x00007fff5fbff7f8, function=<unavailable>, receiver=<unavailable>) + 419 at execution.cc:91
    frame #25: 0x0000000100123d6f node`v8::Function::Call(this=0x0000000101858418, argc=1, argv=0x00007fff5fbff7f8, recv=<unavailable>) + 207 at api.cc:4019
    frame #26: 0x0000000100541ce9 node`node::LoadEnvironment(env=0x0000000101803000) + 517 at node.cc:2868
    frame #27: 0x0000000100542f0a node`node::Start(argc=2, argv=<unavailable>) + 305 at node.cc:3696
    frame #28: 0x0000000100000b74 node`start + 52
(lldb)

It seems that we should at least make V8 exits more gracefully with an error message that indicates the cause of the error.

• Defining the ICU_DATA_DIR macro with a relative path cannot work, as ICU would try to load the .dat file relative to the current working directory. Thus, the configure script either needs to always get an absolute path for --icu-data-dir, or it needs to know about the installation prefix. This is why in this PR the prefix definition has been moved from Makefile to configure.

Fixes #13.

[srl295]

re crash: I go into this in #6. Upstream patches in v8 pending to make it a softer err. Node could try loading ICU on its own and do something- gracefully exit from main() or such. But

1. that would require loading ICU data- something we usually want to avoid and delay to the last second ( load time, disk hit, memory hit etc)
2. I'm not sure there's a way to keep v8 from using Intl

[misterdjules]

@srl295 Thanks! Just adding the link to the V8 issue here, so that it's easier to find: https://code.google.com/p/v8/issues/detail?id=3348.

[srl295]

@misterdjules v8 has fixed the crashing issue upstream. It now exits with a fatal err instead. Progress.

[misterdjules]

@srl295 That's great, thank you!

Two comments:

1. This could be less critical if we decided to check the ICU data directory before calling u_setDataDirectory in all cases (small-icu, full-icu, etc.). However, if I understand correctly, we decided to check if the ICU data path is a directory only in the small-icu case.

2. Since this V8 change will land in a version of V8 we can't upgrade to for 0.12, we may need to float this patch, as I think it would be quite helpful to diagnose future issues with ICU data.

@srl295 @tjfontaine What do you think?

[srl295]

1. But in the "full icu" case we won't crash as the full icu data is all
   there. Icu will only use the directory as fallback (like if you ask for
   Klingon). On disk can't override linked in data.

2. up to you. Sorry I didn't press more on this case, if it would have made
   a difference in April.

[misterdjules]

1. Sorry I wasn't clear, by "full-icu" I meant full ICU data in a separate file and stub data linked in the binary, the way we plan to build node with ICU support for binary installers (OS X, Windows and binary tarballs).

2. No worries, it's already great that they landed it!

[srl295]

Ok yes. The "stub+full" case will crash if it can't find the data.