Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node 5.0.0 wasn't compiled with Intl for armhf *.deb #168

Closed
shouze opened this issue Nov 6, 2015 · 19 comments
Closed

Node 5.0.0 wasn't compiled with Intl for armhf *.deb #168

shouze opened this issue Nov 6, 2015 · 19 comments

Comments

@shouze
Copy link

shouze commented Nov 6, 2015

When I run this command:

$ node -e 'var res = typeof Intl; console.log(res);'
undefined

Should be object of course.

@shouze
Copy link
Author

shouze commented Nov 6, 2015

It was for ubuntu vivid if this info is relevant.

@shouze
Copy link
Author

shouze commented Nov 6, 2015

Probably that libicu52 and libicu-dev weren't installed on the ubtuntu machine used to compile node?

Or --with-intl not passed to configure as specified here?

@chrislea
Copy link
Contributor

chrislea commented Nov 6, 2015

It hasn't been compiled with --with-intl anywhere. We'll look into that for the next release.

@shouze
Copy link
Author

shouze commented Nov 6, 2015

Ok guys intl support is not included:

node -p process.config
{ target_defaults:
   { cflags: [],
     default_configuration: 'Release',
     defines: [],
     include_dirs: [],
     libraries: [] },
  variables:
   { arm_float_abi: 'hard',
     arm_fpu: 'vfpv3',
     arm_thumb: 0,
     arm_version: '7',
     asan: 0,
     gas_version: '2.25',
     host_arch: 'arm',
     icu_small: false,
     node_byteorder: 'little',
     node_install_npm: true,
     node_prefix: '/usr',
     node_release_urlbase: '',
     node_shared_http_parser: false,
     node_shared_libuv: false,
     node_shared_openssl: false,
     node_shared_zlib: false,
     node_tag: '',
     node_use_dtrace: false,
     node_use_etw: false,
     node_use_lttng: false,
     node_use_openssl: true,
     node_use_perfctr: false,
     openssl_fips: '',
     openssl_no_asm: 0,
     python: '/usr/bin/python',
     target_arch: 'arm',
     uv_parent_path: '/deps/uv/',
     uv_use_dtrace: false,
     v8_enable_gdbjit: 0,
     v8_enable_i18n_support: 0,
     v8_no_strict_aliasing: 1,
     v8_optimized_debug: 0,
     v8_random_seed: 0,
     v8_use_snapshot: 0,
     want_separate_host_toolset: 0 } }

@shouze
Copy link
Author

shouze commented Nov 7, 2015

@chrislea yes, and we were used to use an intl polyfill that was endorsing the native Intl responsibility in one of our apps till now and... we didn't detect that till now. At the moment this same app don't build anymore because of missing Intl and polyfill problem so I detected that.

@shouze
Copy link
Author

shouze commented Nov 7, 2015

On wich repo can I make a PR to fix that? Also about the deb/rpm packages libicu maybe should become a required dependency.

@chrislea
Copy link
Contributor

chrislea commented Nov 9, 2015

Well, if we decide to 'officially' support this compile option, then yes, the libicu libraries would become dependencies. Which is part of the equation regarding if we do it or not, as we'll need to see if that's even feasible on some older distros that we want to keep supporting.

I've asked other people at NodeSource about this who know more about the issues and am waiting to hear back from them.

I'd just leave this issue open to track it. Whatever we decide to do I will update here.

@shouze
Copy link
Author

shouze commented Nov 10, 2015

Ok no pb.

As nodejs official binary distributions includes intl/icu should be a great compatibility lineup BTW.

@chrislea
Copy link
Contributor

Okay. The 4.2.2 builds and the 5.0.0 builds are now compiled with --with-intl=system-icu. So they are using the system's ICU libs. This should resolve the above issue. I'll leave this open for a bit in case any issues pop up.

@shouze
Copy link
Author

shouze commented Nov 12, 2015

@chrislea thx I will give it a try today! 😻

@shouze
Copy link
Author

shouze commented Nov 18, 2015

@chrislea looks ok but I was running it in a ubuntu vivid container without any locale generated (POSIX was the only one). It's important to generate the right locales.

BTW I've seen that node 5.1 has been release today and that the deb don't include any dependency on libicu. Haven't installed it yet but I guess that this release wasn't compiled with system intl support?

@shouze
Copy link
Author

shouze commented Nov 18, 2015

Also, does it exist a way to pin to a specific release?

If in my Dockerfile I pu the following lines:

RUN curl -sL https://deb.nodesource.com/setup_5.x?version=5.0.0-3 | bash - \
    && apt-get -q update \
    && apt-get -y -qq upgrade \
    && apt-get -y -qq install git-core nodejs \
    && apt-get clean

Il will install 5.1.0 release for example.... but if i put

RUN curl -sL https://deb.nodesource.com/setup_5.x?version=5.0.0-3 | bash - \
    && apt-get -q update \
    && apt-get -y -qq upgrade \
    && apt-get -y -qq install git-core nodejs=5.0.0-3nodesource1~vivid1 \
    && apt-get clean

It will fail as the Packages file of your debs repo only expose the last release even if the previous one is still available in the pool.

@chrislea
Copy link
Contributor

@shouze after some annoying issues with using system-icu, we built 5.1 and statically linked in the ICU stuff. So it's a) consistent across all the packages we make and b) also consistent with the official binaries you can download from https://nodejs.org. So it's in there, but ldd won't show the shared library being linked in.

The pinning thing is an issue with the reprepro tool we use to manage the repositories, which doesn't support anything other than promoting the newest version I'm afraid. It's on my list of things to look into but it's at a low priority for now, sorry.

@shouze
Copy link
Author

shouze commented Nov 18, 2015

Ok I understand about system-icu and I've seen that yes it's effectively statically linked.... but... as it's the case with official node binary distributions for x86 arch, icu only bring the en locale as illustrated when I run

$ node -p process.config | grep locales                                                                                                                        icu_locales: 'en,root',

So let's say that if I need to support other locales in my app... ok no... I will always have to use the intl polyfill it's a shame.
I will lookup how to use node Intl in any circumstances as it's a 5% gain on my app (not the biggest one nor biggest priority however).

@chrislea
Copy link
Contributor

@shouze: The problem there is that to statically compile in everything using full-icu you increase the binary size of node by ~ 25M, which nobody really wants. There is talk here of separating out the additional data into an individually installable package that could get picked up at runtime or something like that. So if you want to add any commentary or encouragement, that's the place to do it.

@auvipy
Copy link

auvipy commented Nov 19, 2015

node 5.1 released

@shouze
Copy link
Author

shouze commented Nov 19, 2015

@chrislea ok with npm install full-icu ATM it solves the issue.

@shouze
Copy link
Author

shouze commented Nov 19, 2015

@auvipy yup and the debs are available.

@shouze
Copy link
Author

shouze commented Dec 14, 2015

I close this one as all is ok right now.

@shouze shouze closed this as completed Dec 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants