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

Support for node compiled with '--with-icu-default-data-dir' #63

Closed
sgallagher opened this issue Sep 24, 2021 · 3 comments
Closed

Support for node compiled with '--with-icu-default-data-dir' #63

sgallagher opened this issue Sep 24, 2021 · 3 comments

Comments

@sgallagher
Copy link

This bug has some overlap with #61 but there's more to it.

Since Node.js 12.6.0, it has supported an argument --with-icu-default-data-dir at compile-time that builds a specialized version of small-icu that also attempts to auto-load extended data from a specific path on the system. In the case of Fedora, we use /usr/share/nodejs/icudata for this purpose.

This allows us to ship Node.js either with or without the full i18n support without needing to recompile. (It also means that users can install complete i18n data by running dnf install nodejs-full-i18n.

However, this currently causes us a problem when a package includes a requirement on the full-icu NPM:

npm install full-icu
npm WARN old lockfile 
npm WARN old lockfile The package-lock.json file was created with an old version of npm,
npm WARN old lockfile so supplemental metadata must be fetched from the registry.
npm WARN old lockfile 
npm WARN old lockfile This is a one-time fix-up, please be patient...
npm WARN old lockfile 
npm notice 
npm notice New minor version of npm available! 7.21.1 -> 7.24.1
npm notice Changelog: https://github.com/npm/cli/releases/tag/v7.24.1
npm notice Run npm install -g [email protected] to update!
npm notice 
npm ERR! code 1
npm ERR! path /home/sgallagh/node_modules/full-icu
npm ERR! command failed
npm ERR! command sh -c node postinstall.js
npm ERR! npm install icu4c-data@69l (Node 16.9.1 and small-icu 69.1) -> icudt69l.dat
npm ERR! full-icu$ /usr/bin/node /usr/lib/node_modules/npm/bin/npm-cli.js install icu4c-data@69l
npm ERR! npm ERR! code ETARGET
npm ERR! npm ERR! notarget No matching version found for icu4c-data@69l.
npm ERR! npm ERR! notarget In most cases you or one of your dependencies are requesting
npm ERR! npm ERR! notarget a package version that doesn't exist.
npm ERR! 
npm ERR! npm ERR! A complete log of this run can be found in:
npm ERR! npm ERR!     /home/sgallagh/.npm/_logs/2021-09-24T13_16_23_219Z-debug.log
npm ERR! /home/sgallagh/node_modules/full-icu/install-spawn.js:62
npm ERR! 		throw(Error(cmdPath + ' ' + args.join(' ') + ' --> status ' + spawned.status));
npm ERR! 		^
npm ERR! 
npm ERR! Error: /usr/bin/node /usr/lib/node_modules/npm/bin/npm-cli.js install icu4c-data@69l --> status 1
npm ERR!     at npmInstallNpm (/home/sgallagh/node_modules/full-icu/install-spawn.js:62:9)
npm ERR!     at Object.<anonymous> (/home/sgallagh/node_modules/full-icu/postinstall.js:72:2)
npm ERR!     at Module._compile (node:internal/modules/cjs/loader:1101:14)
npm ERR!     at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
npm ERR!     at Module.load (node:internal/modules/cjs/loader:981:32)
npm ERR!     at Function.Module._load (node:internal/modules/cjs/loader:822:12)
npm ERR!     at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:79:12)
npm ERR!     at node:internal/main/run_main_module:17:47

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/sgallagh/.npm/_logs/2021-09-24T13_16_23_256Z-debug.log

What I would like to see is for the postinstall.js to check whether a data file is already present in the process.config.variables.icu_default_data location and if so, leave it alone (possibly alerting the user). Otherwise, it would be best if the full-icu NPM would download the data file to that location.

@srl295
Copy link
Member

srl295 commented Sep 24, 2021

The root issue here is #53 though this sounds like an improvement

@srl295
Copy link
Member

srl295 commented Sep 28, 2021

@sgallagher OK I think I see what you mean… if process.config.variables.icu_default_data (which I think is a search path not just a directory) contains full icu then don't bother downloading.

I think it's a good optimization, but in any event all it would do is prevent extra downloading. The above error (69l not found) is a dup of #61 , and also fixed by #53

@github-actions
Copy link

This issue seems to be stale

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants