-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Publish tarball that contains only full ICU data #8996
Comments
You can pull the full data file out of |
Note: this is blocked by #9046 - we can't generate the big endian tarball for the same reason (conversion). Probably the solution proposed there needs to go in first, IF we want to publish BE data from node's |
ICU can't "swap endianness" if its conversion, etc code is removed. We don't want this code in node, but it's needed for the tools. May affect 'target' inadvertently - to be analyzed. Basically this is nodejs#9046 but also blocks nodejs#8996
…CU data see nodejs#8996 Have not tested the resulting tarballs yet, posting for comment. Generates: - 9.8M node-v0.11.16-locales-be.tar.gz - 9.6M node-v0.11.16-locales-le.tar.gz
@misterdjules ^^ thoughts on #9091 ? Is it the right shape? |
FYI @sxa555 - how does this look? |
|
|
@misterdjules where should the extended tests ^^ live? |
Seems good - subject to an assumption that Windows users will be ok with a .tgz instead of a ZIP. |
@sxa555 yeah wasn't sure about that. Make a .zip also? make them all zips? |
@srl295 Thank you very much for your work, and sorry for taking so long to do a first review. It's not clear to me yet how to add tests for that, since I believe we don't currently have any similar "integration" test. I would put them in a separate directory in The test doesn't need to upload the tarball, it could just build node with We would also want to have that work on all supported platforms (OS X, Linux, SmartOS and Windows). What do you think? |
Tricky to say on tgz vs zip since in the general case you cant' assume that UNIX users will have an unzip/jar command either ... but I guess that might be more likely than a windows user have gzip+tar ... Another thought though - how practical would it be to ship the ICU data as an NPM module which users could install globally - if you did that it might be possible to also have node default to searching in the global location for the ICU data file if no alternative was explcitly specified? |
@sxa555 actually, the npm module idea makes some sense. This could morph into a 'generator' that auto publishes into npm the appropriate versions. And actually, we could install with node (part of the normal build) a text file containing: |
While I look into the
Alternate options:
Any thoughts about the npm package naming and use of the tag? |
OK, NPM package published here - https://www.npmjs.com/package/icu4c-data snippets:
any thoughts? |
I'd probably install the latest LE version by default if you don't specify , but otherwise LGTM I guess we need to encourage people to specify the version properly anyway to ensure the ICU version patches what's built into their node anyway. |
@sxa555 yeah, I think the default has to do with the 'real' npm package version being the highest. Maybe I can retag 'latest' somehow. |
Why is endianness part of the package name? Doesn't NPM have a means of selecting platform-specific binaries? Also, what's wrong with using semver for the package version, rather than putting the version in the name as well? |
It's part of the tag, not the name. I'm not aware of that means but I will look, do you have a link? edit sorry, please see https://www.npmjs.com/package/icu4c-data - I didn't update the proposal at the top of this ticket. 0.54.0 is tagged as 54l, 0.54.1 is tagged as 54b so you can |
I guess npm's means of selecting platform-specific binaries is to include a binding.gyp in the package, which is then used on install. So, you'd have a binding.gyp that builds/downloads the appropriate blob at install time, depending on the platform it's been installed to. (gyp documentation: https://code.google.com/p/gyp/wiki/GypUserDocumentation) I'm not sure how gyp would select the endianness of the platform, but that essentially just stems from me not knowing what big-endian platforms there are with Node. Other approaches that seem simple enough to my untrained eye:
Specifying the endianness as part of the package spec is no good, as it unnecessarily limits your package to a specific platform. |
@stuartpb I was hoping to keep the dependencies more lightweight than gyp. Also, you have to select the right version number of the ICU data anyways, so if v8 was linked against icu 54 you already need to select 54 and not 53 or 55 - so selecting 54l versus 54b doesn't seem to add that much of a burden. What you could have is a node module that on 'install', turns around and like this executes the command output by this.
|
@srl295 Wasn't sure where you were storing the source for the NPM, but the usage example in the MD has ")))" at the end which is one too many - should just be "))" |
@sxa555 thanks, it's actually on icu-project at the moment. I'll fix that typo. |
The ICU ticket has been accepted by the ICU PMC. For now, this will be a task done and updated by the ICU release process (me). If ICU fails to update it, the updating could be taken over by node.js or some other party. |
@srl295 ... what's the status on this one? |
@srl295 ... going to assume we can close this. Feel free to reopen if that assumption is incorrect. |
Eventually, binary installers will ship with full ICU data (see #8995). However, other binaries (binary tarballs, custom builds from source) will still not ship and load the full ICU data by default.
To allow users of these other binaries to use the full ICU data, we should generate and publish a separate tarball that contains only the full ICU data as a separate
.dat
file.To use the full ICU data, users could put this file in
$PREFIX/share/node/icu/
or could pass its path on the command line using--icu-data-dir=
.The tarball would be automatically generated and published by a Jenkins job.
This depends on #8983.
cc @srl295 @tjfontaine
The text was updated successfully, but these errors were encountered: