Added --with-intl flag on linux, android, and macOS

[jackhorton]

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

build

[jackhorton]

/cc @dilijev

[digitalinfinity]

This doesn't need the chakracore changes for Intl? What is the effect of this flag on release/1.7?

[MSLaguana]

How does this interact with a no-icu build of node? Will we fail to link in that case?

[dilijev]

@digitalinfinity The changes needed to enable the Intl object with ICU (in it's partially-functional state) are already landed in release/1.7 and are therefore already in this repo.

[dilijev]

@digitalinfinity The --with-intl flag for Linux builds of ChakraCore exposes the Intl object as described above.

@jackhorton Have you considered the Windows build of Node-ChakraCore with "/p:IntlICU=true"? Arguably not strictly necessary because the Intl object is already available in Node-ChakraCore on Windows with WinGlob, but since we're aiming for parity between Windows and Linux Node-ChakraCore as well as Windows and Linux Node, this would be a good thing for us to be able to build and test.

[jackhorton]

@dilijev Sure, I didn't think we were looking at touching windows at all for now, but it should be a fairly easy fix.

@MSLaguana I actually just opened an issue for this, because it crashes (and crashed before this change, too): #396

[dilijev]

@jackhorton You are correct, I had split up those tasks, but assuming it is this straightforward to do on Windows as well, having the ability to compare numbers between Windows and other platforms will be nice to have.

Ref: chakra-core/ChakraCore#3676
Ref: chakra-core/ChakraCore#3677

[jackhorton]

Update on this -- This pull request "works" in so far as it enables the Intl object when using ChakraCore:release/1.7 today, and it does so using node's ICU (tested with the built-in icu-small as well as a node-managed build of icu-full). There is work to be done here when the ICU work lands in ChakraCore and makes its way to Node:

• Don't assume '<(icu_path)/source/common' as the include directory, because there are important files in at least '<(icu_path)/source/i18n' as well. This path should end up being just .../source and then CMake should handle adding path/common and path/i18n to the global include paths, as done in Update build system for more generic include paths chakra-core/ChakraCore#3781
• If we want to use ICU on Windows in the node scenario, ChakraCore.dll seems to require the path to the icu*.lib files in order to link properly. On Linux, this isn't an issue because libChakraCoreStatic.a isn't anything special, so its happy to get linked with icu*.lib files when the final node executable builds. Thus, we will need to find a way to get the icu*.lib file paths to pass to /p:IcuLibDir for the windows build. That may or may not be doable easily/reproducibly, but regardless, its something that should probably wait until our intl changes flow over here so that I am not cherry picking patches from the intl-icu branch to test this build with.

[dilijev]

@jackhorton Thanks for the update! Any reason not to merge this at this point? (Or are we waiting for the ICU work to land in ChakraCore before merging this into nodejs:master?)

[dilijev]

Note: we are delaying the work on Node-ChakraCore with ICU-backed Intl on Windows it was more complex to implement and not a high enough priority at this time, since Node-ChakraCore on Windows has an Intl object backed by the WinGlob implementation.

[jackhorton]

Landed in 7ca7c6c