detect "full-icu" module #3460

srl295 · 2015-10-20T22:54:04Z

Followup to nodejs/node-v0.x-archive#8996

The npm module full-icu will at postinstall time make sure node_modules/full-icu contains the appropriate icudt*.dat file needed for full ICU support.
- it also prints out instructions on how to fire up Node with full ICU.
Proposal: somehow detect the special (?) module name full-icu in any of the global search paths or local module search paths. If found && the correct icudt file is present, _automatically set Node's ICU path to point to that icudt_.dat file*

This will make getting full data as easy as npm install [-g] full-icu

The text was updated successfully, but these errors were encountered:

srl295 · 2015-10-20T22:54:15Z

@jasnell

jasnell · 2015-10-20T23:00:02Z

hmmm... trying it now...

npm install icu4c-data@55l (Node 4.0.0 and small-icu 55) -> icudt55l.dat
npm WARN unmet dependency /usr/local/lib/node_modules/yeoman requires bower@'~0.3.0' but will load
npm WARN unmet dependency /usr/local/lib/node_modules/bower,
npm WARN unmet dependency which is version 1.5.3
npm WARN unmet dependency /usr/local/lib/node_modules/npm/node_modules/npm-registry-client/node_modules/npm-package-arg requires semver@'4' but will load
npm WARN unmet dependency /usr/local/lib/node_modules/npm/node_modules/semver,
npm WARN unmet dependency which is version 5.0.1
icu4c-data@0.55.2 /usr/local/lib/node_modules/icu4c-data
 •  (no icudt55l.dat)
/usr/local/lib/node_modules/full-icu/install-spawn.js:51
                        throw Error('Somehow failed to install ' + icudat);
                        ^

Error: Somehow failed to install icudt55l.dat
    at Error (native)
    at npmInstallNpm (/usr/local/lib/node_modules/full-icu/install-spawn.js:51:10)
    at Object.<anonymous> (/usr/local/lib/node_modules/full-icu/postinstall.js:62:2)
    at Module._compile (module.js:434:26)
    at Object.Module._extensions..js (module.js:452:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Function.Module.runMain (module.js:475:10)
    at startup (node.js:117:18)
    at node.js:951:3

Fishrock123 · 2015-10-20T23:02:24Z

Tagging this for tsc discussion. This is quite interesting but requires hard-coding in some this we haven't done anything like before.

Fishrock123 · 2015-10-20T23:02:45Z

Feel free to remove if you don't think it qualifies for it (yet).

jasnell · 2015-10-20T23:04:26Z

Having it autodiscover is good in theory but does introduce a startup performance concern. It would be good to have a PoC implementation that we can benchmark.

edef1c · 2015-10-20T23:16:41Z

It's worth noting that this creates a coupling between node and npm (something that hasn't really existed before — node just bundled npm thus far), or at least a coupling between node and ${npm_prefix}/lib/node_modules (which never has never been in any search path, to my knowledge).
I'm not sure that's a thing we want to do/have.

edef1c · 2015-10-20T23:18:43Z

In addition, it obscures an application's true dependencies, by violating "dependency locality".
I think that's an even more serious concern than npm coupling.

srl295 · 2015-10-20T23:33:20Z

@jasnell the issue with installing as -g has been solved, nodejs/full-icu-npm#2

This is fine to discuss at TSC. It's been rattling around as an idea for a while now.

In addition, it obscures an application's true dependencies, by violating "dependency locality".

I assume this is not just related to the auto discover but the full-icu package itself. Do you have any suggestions as to the approach? To just get the data file, the "sub-dependency" wouldn't have to be an NPM dependency- post install could just wget fetch the data from somewhere.

Anyways - the entire approach is most certainly open to discussion.

silverwind · 2015-10-22T16:05:17Z

BTW, is building with full-icu by default out of question?

Fishrock123 · 2015-10-22T16:12:11Z

@silverwind this was discussed in detail during the call: https://soundcloud.com/node-foundation/tsc-meeting-2015-10-21

Decision was to move back to GH though.

srl295 · 2015-10-22T17:49:15Z

@silverwind #3460 is about detecting the full data if you are built in small mode, which would remain relevant unless small mode was removed. I will reply to your question over on #3476 which is about how ICU is built itself.

orangemocha · 2015-10-23T17:00:37Z

Rather than having node searching through node_modules, how about having the full-icu module put the data somewhere in the user folder, and have node look for it there at startup? It's a similar approach but it decouples the mechanism used by node to find the data from the tool used to install it.

srl295 · 2015-10-24T06:55:24Z

@orangemocha that could work. Suggestions on what the 'somewhere' should look like? The only precedent for this that I know of is the modules dirs. Maybe something like:

{CWD}/icu-data/icudt56l.dat
{HOME or profile}/.icu-data/icudt56l.dat
{Node install dir}/../lib/icu-data-icudt56l.dat

I put the specific file name because node will look for that one file that it needs. Three stat calls (in this example) and then done.

orangemocha · 2015-10-26T19:06:42Z

Yes, I would follow a parallel with npm global modules. They are under %APPDATA%\npm on Windows. I believe the Linux equivalent is ~/.appname. I would use node as the app name assuming this data is only useful to node.

So:

On *nix: ~/.node/icu-data/icudt56l.dat
On Windows: %APPDATA%\node\icu-data\icudt56l.dat

srl295 · 2015-10-26T21:29:06Z

Mac would be something like ~/Library/application support/node

assume this is only applicable to node

Oooo are we going there? I guess we are going there.. I could see if there's interest in this upstream in ICU. Right now a path is allowed for data search. The node specific trick is that normally a data path can't override linked in data. So node explicitly has to decide whether it is installing the "small Icu" data or not at startup time. So this model could generalize as 1.) a convention and/or 2.) an ICU API with some sort of "Boolean isFullIcuDataFoundSomewhere()" or maybe an Icu Api to calculate the platform path.-- all without actually loading ICU.

Fishrock123 · 2015-11-03T16:03:21Z

Removing the tsc-agenda tag for now.

bnoordhuis · 2015-11-11T21:12:17Z

Resolution from today's CTC meeting:

No objections to the suggested approach.
Follow up with working code.
Mark experimental for a major cycle or two to shake out issues.

Does that sound about right?

srl295 · 2015-11-11T21:33:33Z

@bnoordhuis yes, thanks. There have been so many different opinions on this, appreciate the quick review

chrislea · 2015-11-18T01:57:38Z

FWIW, from the standpoint of making deb and rpm packages, you'd want to put the .dat file in

/usr/share/node/icudt56l.dat

We could then include basically just that file in a separate package called something like nodejs-intl-full_5.1.0.XXX.deb which would depend on the actual nodejs package, and it should all "just work" like people expect it to.

StarpTech · 2019-05-12T16:36:18Z

@bnoordhuis What do you mean with "just works"? I still need to configure the loading process either via env variable or cli flag.

bnoordhuis · 2019-05-13T06:17:09Z

@StarpTech If you scroll through the comments, you'll see people remarking they had problems installing the module. Those issues have been fixed.

If you're asking about auto-detection: unlikely to happen, or it would have happened already.

srl295 · 2019-05-16T18:15:06Z

FYI, if you use full-icu please see a proposed change here: nodejs/full-icu-npm#36

sgallagher · 2019-12-03T17:45:03Z

We (Fedora/Red Hat Enterprise Linux) have the following use-case:

We want to support developers and multiple deployment scenarios.
Not all users require full internationalization support and would prefer to avoid the disk-space usage when possible. (Example: container environments like OpenShift)
Most deployments do want full internationalization support.
We don't want to build Node.js twice (once for small-icu, once for full-icu) so as to avoid shipping different /usr/bin/node binaries.
We don't want users to be required to explicitly set NODE_ICU_DATA or pass --icu-data-dir. We want the presence or absence of the data to be based on the presence or absence of an RPM package.

What we would like to see is a mode to build Node.js with small-icu bundled but also build the icudt6X[bl].dat from the full-icu. We would then like for there to be a well-known location under $PREFIX that will always be checked as if the NODE_ICU_DATA was set to it (if it's not manually set).

We would then ship two subpackages for Node.js:

nodejs-i18n-data which contains only the data files to put in the well-known location
nodejs which provides the interpreter and Recommends: nodejs-i18n-data. (Recommends: is an RPM dependency specification which means that the listed package will be installed by default unless --no-recommends are passed, such as in a container environment. And also that removing it later does not result in removing the dependent package.)

srl295 · 2019-12-03T22:26:04Z

@sgallagher Hello. The full-icu npm module will download the icudt*.dat file for you. Right now it downloads from NPM, but I want to change it to download from ICU's GitHub release directly. Node.js doesn't need to build icudt*.dat file separately. Starting with Node.js 13, --with-intl=full-icu is the default. But you can still build with --with-intl=small-icu explicitly, and I recommend doing so starting now if that is your preference as a packager.

We would then like for there to be a well-known location under $PREFIX that will always be checked as if the NODE_ICU_DATA was set to it (if it's not manually set).

Yes, that was the premise of this issue when I opened it four years ago. However, it was never implemented beyond a proof of concept phase. It didn't work on all platforms, etc, etc. Someone could still implement this, but I don't plan to work on it.

When compiled with `--with-intl=small` and `--with-icu-default-data-dir=PATH`, Node.js will use PATH as a fallback location for the ICU data. We will first perform an access check using fopen(PATH, 'r') to ensure that the file is readable. If it is, we'll set the icu_data_directory and proceed. There's a slight overhead for the fopen() check, but it should be barely measurable. This will be useful for Linux distribution packagers who want to be able to ship a minimal node binary in a container image but also be able to add on the full i18n support where needed. With this patch, it becomes possible to ship the interpreter as /usr/bin/node in one package for the distribution and to ship the data files in another package (without a strict dependency between the two). This means that users of the distribution will not need to explicitly direct Node.js to locate the ICU data. It also means that in environments where full internationalization is not required, they do not need to carry the extra content (with the associated storage costs). Refs: nodejs#3460 Signed-off-by: Stephen Gallagher <sgallagh@redhat.com>

When compiled with `--with-intl=small` and `--with-icu-default-data-dir=PATH`, Node.js will use PATH as a fallback location for the ICU data. We will first perform an access check using fopen(PATH, 'r') to ensure that the file is readable. If it is, we'll set the icu_data_directory and proceed. There's a slight overhead for the fopen() check, but it should be barely measurable. This will be useful for Linux distribution packagers who want to be able to ship a minimal node binary in a container image but also be able to add on the full i18n support where needed. With this patch, it becomes possible to ship the interpreter as /usr/bin/node in one package for the distribution and to ship the data files in another package (without a strict dependency between the two). This means that users of the distribution will not need to explicitly direct Node.js to locate the ICU data. It also means that in environments where full internationalization is not required, they do not need to carry the extra content (with the associated storage costs). Refs: #3460 Signed-off-by: Stephen Gallagher <sgallagh@redhat.com> PR-URL: #30825 Reviewed-By: Steven R Loomis <srloomis@us.ibm.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

srl295 added the i18n-api Issues and PRs related to the i18n implementation. label Oct 20, 2015

Fishrock123 added the tsc-agenda label Oct 20, 2015

rvagg mentioned this issue Oct 21, 2015

Node Foundation TSC Meeting 2015-10-21 #3464

Closed

srl295 mentioned this issue Oct 21, 2015

Revisit checking in enough source to build "small-icu" into the node repo #3476

Closed

This was referenced Oct 24, 2015

Discoverability/Modularity of ICU Data File nodejs/Intl#11

Closed

Calling Intl.v8BreakIterator makes the process crash #3111

Closed

rvagg mentioned this issue Oct 28, 2015

Node Foundation CTC Meeting 2015-10-28 #3561

Closed

Fishrock123 removed the tsc-agenda label Nov 3, 2015

srl295 mentioned this issue Nov 4, 2015

Node Foundation CTC Meeting 2015-11-04 #3660

Closed

srl295 mentioned this issue Nov 17, 2015

nodejs: Users are not aware of the "full-icu" option Homebrew/legacy-homebrew#46084

Closed

chrislea mentioned this issue Nov 18, 2015

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

Closed

srl295 mentioned this issue May 30, 2019

OpenJS Summit i18n WG Session Agenda nodejs/i18n#197

Closed

sgallagher mentioned this issue Dec 6, 2019

build: Auto-load ICU data from --with-icu-default-data-dir #30825

Closed

4 tasks

SCastaneda mentioned this issue Feb 18, 2020

date formatting with ICU i18next/i18next-icu#14

Closed

srl295 mentioned this issue Mar 24, 2020

Retry on failed ICU data load, ignoring NODE_ICU_DATA or --icu-data-dir #32466

Open

leonardoadame mentioned this issue Nov 28, 2023

[Snyk] Fix for 6 vulnerabilities leonardoadame/node#185

Open

aliscco mentioned this issue Nov 28, 2023

[Snyk] Fix for 5 vulnerabilities aliscco/alisco-node#694

Open

leonardoadame mentioned this issue Nov 29, 2023

[Snyk] Fix for 6 vulnerabilities leonardoadame/node#210

Open

This was referenced Nov 29, 2023

[Snyk] Fix for 4 vulnerabilities aliscco/alisco-node#714

Open

[Snyk] Fix for 5 vulnerabilities aliscco/alisco-node#732

Open

leonardoadame mentioned this issue Nov 29, 2023

[Snyk] Fix for 5 vulnerabilities leonardoadame/node#263

Open

This was referenced Nov 29, 2023

[Snyk] Fix for 14 vulnerabilities aliscco/alisco-node#776

Open

[Snyk] Fix for 6 vulnerabilities aliscco/alisco-node#1023

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

detect "full-icu" module #3460

detect "full-icu" module #3460

srl295 commented Oct 20, 2015

srl295 commented Oct 20, 2015

jasnell commented Oct 20, 2015

Fishrock123 commented Oct 20, 2015

Fishrock123 commented Oct 20, 2015

jasnell commented Oct 20, 2015

edef1c commented Oct 20, 2015

edef1c commented Oct 20, 2015

srl295 commented Oct 20, 2015

silverwind commented Oct 22, 2015

Fishrock123 commented Oct 22, 2015

srl295 commented Oct 22, 2015

orangemocha commented Oct 23, 2015

srl295 commented Oct 24, 2015

orangemocha commented Oct 26, 2015

srl295 commented Oct 26, 2015 via email

Fishrock123 commented Nov 3, 2015

bnoordhuis commented Nov 11, 2015

srl295 commented Nov 11, 2015

chrislea commented Nov 18, 2015

StarpTech commented May 12, 2019

bnoordhuis commented May 13, 2019

srl295 commented May 16, 2019

sgallagher commented Dec 3, 2019

srl295 commented Dec 3, 2019 •

edited

Loading

detect "full-icu" module #3460

detect "full-icu" module #3460

Comments

srl295 commented Oct 20, 2015

srl295 commented Oct 20, 2015

jasnell commented Oct 20, 2015

Fishrock123 commented Oct 20, 2015

Fishrock123 commented Oct 20, 2015

jasnell commented Oct 20, 2015

edef1c commented Oct 20, 2015

edef1c commented Oct 20, 2015

srl295 commented Oct 20, 2015

silverwind commented Oct 22, 2015

Fishrock123 commented Oct 22, 2015

srl295 commented Oct 22, 2015

orangemocha commented Oct 23, 2015

srl295 commented Oct 24, 2015

orangemocha commented Oct 26, 2015

srl295 commented Oct 26, 2015 via email

Fishrock123 commented Nov 3, 2015

bnoordhuis commented Nov 11, 2015

srl295 commented Nov 11, 2015

chrislea commented Nov 18, 2015

StarpTech commented May 12, 2019

bnoordhuis commented May 13, 2019

srl295 commented May 16, 2019

sgallagher commented Dec 3, 2019

srl295 commented Dec 3, 2019 • edited Loading

srl295 commented Dec 3, 2019 •

edited

Loading