Repository navigation
detect "full-icu" module #3460
Description
Activity
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:3Tagging this for tsc discussion. This is quite interesting but requires hard-coding in some this we haven't done anything like before.
Feel free to remove if you don't think it qualifies for it (yet).
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.
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.Reacted by Marvin HagemeisterIn addition, it obscures an application's true dependencies, by violating "dependency locality".
I think that's an even more serious concern than npm coupling.@jasnell the issue with installing as
-ghas been solved, nodejs/full-icu-npm#2This 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-icupackage 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 justwgetfetch the data from somewhere.Anyways - the entire approach is most certainly open to discussion.
BTW, is building with
full-icuby default out of question?@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.
@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.
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.
@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
statcalls (in this example) and then done.- {CWD}
67 remaining items
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-icubundled but also build theicudt6X[bl].datfrom the full-icu. We would then like for there to be a well-known location under$PREFIXthat 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-datawhich contains only the data files to put in the well-known locationnodejswhich provides the interpreter andRecommends: nodejs-i18n-data. (Recommends:is an RPM dependency specification which means that the listed package will be installed by default unless--no-recommendsare passed, such as in a container environment. And also that removing it later does not result in removing the dependent package.)
@sgallagher Hello. The
full-icunpm 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-icuis the default. But you can still build with--with-intl=small-icuexplicitly, 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.
- added a commit that references this issue
on Dec 14, 2019 - added a commit that references this issue
on Dec 17, 2019 - added a commit that references this issue
on Jan 14, 2020 - added a commit that references this issue
on Feb 6, 2020
Followup to nodejs/node-v0.x-archive#8996
postinstalltime make surenode_modules/full-icucontains the appropriateicudt*.datfile needed for full ICU support.full-icuin 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 thaticudt_.datfile*This will make getting full data as easy as
npm install [-g] full-icu