Skip to content

Commit

Permalink
src: make --icu-data-dir= switch testable
Browse files Browse the repository at this point in the history
Move some code around so we can properly test whether the switch
actually does anything.

PR-URL: #11255
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
  • Loading branch information
bnoordhuis committed Feb 11, 2017
1 parent 75019df commit 46345b9
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 21 deletions.
4 changes: 1 addition & 3 deletions lib/internal/process.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ function setupConfig(_source) {
const oldV8BreakIterator = Intl.v8BreakIterator;
const des = Object.getOwnPropertyDescriptor(Intl, 'v8BreakIterator');
des.value = require('internal/util').deprecate(function v8BreakIterator() {
if (processConfig.hasSmallICU && !process.icu_data_dir) {
if (processConfig.hasSmallICU && !processConfig.icuDataDir) {
// Intl.v8BreakIterator() would crash w/ fatal error, so throw instead.
throw new Error('v8BreakIterator: full ICU data not installed. ' +
'See https://github.com/nodejs/node/wiki/Intl');
Expand All @@ -134,8 +134,6 @@ function setupConfig(_source) {
'DEP0017');
Object.defineProperty(Intl, 'v8BreakIterator', des);
}
// Don’t let icu_data_dir leak through.
delete process.icu_data_dir;
}


Expand Down
13 changes: 1 addition & 12 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ static const char* trace_enabled_categories = nullptr;

#if defined(NODE_HAVE_I18N_SUPPORT)
// Path to ICU data (for i18n / Intl)
static std::string icu_data_dir; // NOLINT(runtime/string)
std::string icu_data_dir; // NOLINT(runtime/string)
#endif

// used by C++ modules as well
Expand Down Expand Up @@ -3095,17 +3095,6 @@ void SetupProcessObject(Environment* env,
"ares",
FIXED_ONE_BYTE_STRING(env->isolate(), ARES_VERSION_STR));

#if defined(NODE_HAVE_I18N_SUPPORT) && defined(U_ICU_VERSION)
// ICU-related versions are now handled on the js side, see bootstrap_node.js

if (!icu_data_dir.empty()) {
// Did the user attempt (via env var or parameter) to set an ICU path?
READONLY_PROPERTY(process,
"icu_data_dir",
OneByteString(env->isolate(), icu_data_dir.c_str()));
}
#endif

const char node_modules_version[] = NODE_STRINGIFY(NODE_MODULE_VERSION);
READONLY_PROPERTY(
versions,
Expand Down
7 changes: 5 additions & 2 deletions src/node_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,11 @@ void InitConfig(Local<Object> target,
READONLY_BOOLEAN_PROPERTY("hasSmallICU");
#endif // NODE_HAVE_SMALL_ICU

if (flag_icu_data_dir)
READONLY_BOOLEAN_PROPERTY("usingICUDataDir");
target->DefineOwnProperty(env->context(),
OneByteString(env->isolate(), "icuDataDir"),
OneByteString(env->isolate(), icu_data_dir.data()))
.FromJust();

#endif // NODE_HAVE_I18N_SUPPORT

if (config_preserve_symlinks)
Expand Down
3 changes: 0 additions & 3 deletions src/node_i18n.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ using v8::Object;
using v8::String;
using v8::Value;

bool flag_icu_data_dir = false;

namespace i18n {

const size_t kStorageSize = 1024;
Expand Down Expand Up @@ -415,7 +413,6 @@ bool InitializeICUDirectory(const std::string& path) {
#endif // !NODE_HAVE_SMALL_ICU
return (status == U_ZERO_ERROR);
} else {
flag_icu_data_dir = true;
u_setDataDirectory(path.c_str());
return true; // No error.
}
Expand Down
2 changes: 1 addition & 1 deletion src/node_i18n.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

namespace node {

extern bool flag_icu_data_dir;
extern std::string icu_data_dir; // NOLINT(runtime/string)

namespace i18n {

Expand Down
7 changes: 7 additions & 0 deletions test/parallel/test-intl-no-icu-data.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Flags: --icu-data-dir=test/fixtures/empty/
'use strict';
require('../common');
const assert = require('assert');

// No-op when ICU case mappings are unavailable.
assert.strictEqual('ç'.toLocaleUpperCase('el'), 'ç');

0 comments on commit 46345b9

Please sign in to comment.