Skip to content

Commit

Permalink
intl: Add more versions from ICU
Browse files Browse the repository at this point in the history
* Adds process.versions.cldr, .tz, and .unicode
* Changes how process.versions.icu is loaded
* Lazy loads the process.versions.* values for these
* add an exception to util.js
   to cause 'node -p process.versions' to still work
* update process.version docs

Fixes: #9237
  • Loading branch information
srl295 committed Oct 27, 2016
1 parent 97dfced commit 4fb27d4
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 6 deletions.
6 changes: 4 additions & 2 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -1643,8 +1643,10 @@ Will generate output similar to:
ares: '1.10.0-DEV',
modules: '43',
icu: '55.1',
openssl: '1.0.1k'
}
openssl: '1.0.1k',
unicode: '8.0',
cldr: '29.0',
tz: '2016b' }
```

## Exit Codes
Expand Down
32 changes: 32 additions & 0 deletions lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
// do this good and early, since it handles errors.
setupProcessFatal();

setupProcessICUVersions();

setupGlobalVariables();
if (!process._noBrowserGlobals) {
setupGlobalTimeouts();
Expand Down Expand Up @@ -304,6 +306,36 @@
};
}

function setupProcessICUVersions() {
const icu = process.binding('config').hasIntl ?
process.binding('icu') : undefined;
if (!icu) return; // no Intl/ICU: nothing to add here.
// With no argument, getVersion() returns a comma separated list
// of possible types.
const versionTypes = icu.getVersion().split(',');
versionTypes.forEach((name) => {
// Copied from module.js:addBuiltinLibsToObject
Object.defineProperty(process.versions, name, {
configurable: true,
enumerable: true,
get: () => {
// With an argument, getVersion(type) returns
// the actual version string.
const version = icu.getVersion(name);

This comment has been minimized.

Copy link
@refack

refack May 27, 2017

Contributor

@srl295 is there a scenario that makes this change in runtime?

// Replace the current getter with a new
// property.
delete process.versions[name];
Object.defineProperty(process.versions, name, {
value: version,
writable: false,
enumerable: true
});
return version;
}
});
});
}

function tryGetCwd(path) {
var threw = true;
var cwd;
Expand Down
4 changes: 4 additions & 0 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -1049,3 +1049,7 @@ exports._exceptionWithHostPort = function(err,
}
return ex;
};

// process.versions needs a custom function as some values are lazy-evaluated.
process.versions[exports.inspect.custom] =

This comment has been minimized.

Copy link
@thefourtheye

thefourtheye Nov 17, 2016

Contributor

Is this function really necessary? I don't see this being used anywhere.

This comment has been minimized.

Copy link
@addaleax

addaleax Nov 17, 2016

Member

@thefourtheye It’s so that printing process.versions doesn’t just show [Getter] for the added values, in particular for commands like node -p process.versions that are useful for remote debugging.

Maybe we should add a test for that?

This comment has been minimized.

Copy link
@thefourtheye

thefourtheye Nov 17, 2016

Contributor

@addaleax Hmmm, can you give an example? Right now I get this

➜  node git:(master) node -p process.versions
{ http_parser: '2.7.0',
  node: '6.9.1',
  v8: '5.1.281.84',
  uv: '1.9.1',
  zlib: '1.2.8',
  ares: '1.10.1-DEV',
  icu: '57.1',
  modules: '48',
  openssl: '1.0.2j' }

This comment has been minimized.

Copy link
@srl295

srl295 Nov 17, 2016

Author Member

@addaleax there's a test, it doesn't check for a specific value though. Can improve that, good idea.

@thefourtheye well, this change didn't land in node 6.9.1 but in 4fb27d4

Showing [getter] isn't a bug I fixed, it would have been a bug without this line. This is a feature enhancement.
This function is using a Symbol and so gets automatically invoked by the inspector.

This comment has been minimized.

Copy link
@thefourtheye

thefourtheye Nov 17, 2016

Contributor

@srl295 I'll get a better idea if I look at the test. Can you point me to the actual test? So, the inspector uses this function to actually get the versions?

This comment has been minimized.

Copy link
@srl295

srl295 Nov 18, 2016

Author Member

@thefourtheye yes, it does. See 4fb27d4#diff-d2c5d55862b1a92c267ddbe972ac5392 or test/parallel/test-process-versions.js in this commit.

(depth) => exports.format(JSON.parse(JSON.stringify(process.versions)));
4 changes: 1 addition & 3 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3032,9 +3032,7 @@ void SetupProcessObject(Environment* env,
FIXED_ONE_BYTE_STRING(env->isolate(), ARES_VERSION_STR));

#if defined(NODE_HAVE_I18N_SUPPORT) && defined(U_ICU_VERSION)
READONLY_PROPERTY(versions,
"icu",
OneByteString(env->isolate(), U_ICU_VERSION));
// ICU-related versions are now handled on the js side, see bootstrap_node.js

if (icu_data_dir != nullptr) {
// Did the user attempt (via env var or parameter) to set an ICU path?
Expand Down
68 changes: 67 additions & 1 deletion src/node_i18n.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,18 @@
#include "util-inl.h"
#include "v8.h"

#include <unicode/utypes.h>
#include <unicode/putil.h>
#include <unicode/uchar.h>
#include <unicode/udata.h>
#include <unicode/uidna.h>
#include <unicode/utypes.h>
#include <unicode/ucnv.h>
#include <unicode/utf8.h>
#include <unicode/utf16.h>
#include <unicode/timezone.h>
#include <unicode/ulocdata.h>
#include <unicode/uvernum.h>
#include <unicode/uversion.h>

#ifdef NODE_HAVE_SMALL_ICU
/* if this is defined, we have a 'secondary' entry point.
Expand Down Expand Up @@ -339,6 +343,67 @@ static void ICUErrorName(const FunctionCallbackInfo<Value>& args) {
v8::NewStringType::kNormal).ToLocalChecked());
}

#define TYPE_ICU "icu"
#define TYPE_UNICODE "unicode"
#define TYPE_CLDR "cldr"
#define TYPE_TZ "tz"

/**
* This is the workhorse function that deals with the actual version info.
* Get an ICU version.
* @param type the type of version to get. One of VERSION_TYPES
* @param buf optional buffer for result
* @param status ICU error status. If failure, assume result is undefined.
* @return version number, or NULL. May or may not be buf.
*/
static const char* GetVersion(const char* type,
char buf[U_MAX_VERSION_STRING_LENGTH],
UErrorCode* status) {
if (!strcmp(type, TYPE_ICU)) {
return U_ICU_VERSION;
} else if (!strcmp(type, TYPE_UNICODE)) {
return U_UNICODE_VERSION;
} else if (!strcmp(type, TYPE_TZ)) {
return TimeZone::getTZDataVersion(*status);
} else if (!strcmp(type, TYPE_CLDR)) {
UVersionInfo versionArray;
ulocdata_getCLDRVersion(versionArray, status);
if (U_SUCCESS(*status)) {
u_versionToString(versionArray, buf);
return buf;
}
}
// Fall through - unknown type or error case
return nullptr;
}

static void GetVersion(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
if ( args.Length() == 0 ) {
// With no args - return a comma-separated list of allowed values
args.GetReturnValue().Set(
String::NewFromUtf8(env->isolate(),
TYPE_ICU ","
TYPE_UNICODE ","
TYPE_CLDR ","
TYPE_TZ));
} else {
CHECK_GE(args.Length(), 1);
CHECK(args[0]->IsString());
Utf8Value val(env->isolate(), args[0]);
UErrorCode status = U_ZERO_ERROR;
char buf[U_MAX_VERSION_STRING_LENGTH] = ""; // Possible output buffer.
const char* versionString = GetVersion(*val, buf, &status);

if (U_SUCCESS(status) && versionString) {
// Success.
args.GetReturnValue().Set(
String::NewFromUtf8(env->isolate(),
versionString));
}
}
}

bool InitializeICUDirectory(const char* icu_data_path) {
if (icu_data_path != nullptr) {
flag_icu_data_dir = true;
Expand Down Expand Up @@ -558,6 +623,7 @@ void Init(Local<Object> target,
env->SetMethod(target, "toUnicode", ToUnicode);
env->SetMethod(target, "toASCII", ToASCII);
env->SetMethod(target, "getStringWidth", GetStringWidth);
env->SetMethod(target, "getVersion", GetVersion);

// One-shot converters
env->SetMethod(target, "icuErrName", ICUErrorName);
Expand Down
3 changes: 3 additions & 0 deletions test/parallel/test-process-versions.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ if (common.hasCrypto) {

if (common.hasIntl) {
expected_keys.push('icu');
expected_keys.push('cldr');
expected_keys.push('tz');
expected_keys.push('unicode');
}

expected_keys.sort();
Expand Down

1 comment on commit 4fb27d4

@addaleax
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR-URL: #9266
Reviewed-By: @jasnell
Reviewed-By: @addaleax

Please sign in to comment.