Skip to content

Commit

Permalink
deps: allow amaro to be externalizable
Browse files Browse the repository at this point in the history
- allow amaro to be externalized like other builtins
  containing WASM. More context is available in
  https://github.com/nodejs/node/blob/main/doc/contributing/maintaining/maintaining-dependencies.md#supporting-externalizable-dependencies-with-javascript-code

Signed-off-by: Michael Dawson <[email protected]>
PR-URL: #54646
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
mhdawson authored and aduh95 committed Sep 12, 2024
1 parent 8c4c85c commit 0659516
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 8 deletions.
8 changes: 7 additions & 1 deletion configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,11 @@
valid_intl_modes = ('none', 'small-icu', 'full-icu', 'system-icu')
icu_versions = json.loads((tools_path / 'icu' / 'icu_versions.json').read_text(encoding='utf-8'))

# builtins may be removed later if they have been disabled by options
shareable_builtins = {'cjs_module_lexer/lexer': 'deps/cjs-module-lexer/lexer.js',
'cjs_module_lexer/dist/lexer': 'deps/cjs-module-lexer/dist/lexer.js',
'undici/undici': 'deps/undici/undici.js'
'undici/undici': 'deps/undici/undici.js',
'amaro/dist/index': 'deps/amaro/dist/index.js'
}

# create option groups
Expand Down Expand Up @@ -2199,6 +2201,10 @@ def make_bin_override():
configure_inspector(output)
configure_section_file(output)

# remove builtins that have been disabled
if options.without_amaro:
del shareable_builtins['amaro/dist/index']

# configure shareable builtins
output['variables']['node_builtin_shareable_builtins'] = []
for builtin, value in shareable_builtins.items():
Expand Down
1 change: 1 addition & 0 deletions node.gni
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ declare_args() {
"deps/cjs-module-lexer/lexer.js",
"deps/cjs-module-lexer/dist/lexer.js",
"deps/undici/undici.js",
"deps/amaro/dist/index.js",
]
}

Expand Down
5 changes: 0 additions & 5 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -465,11 +465,6 @@
}, {
'use_openssl_def%': 0,
}],
[ 'node_use_amaro=="true"', {
'deps_files': [
'deps/amaro/dist/index.js',
]
} ]
],
},

Expand Down
7 changes: 7 additions & 0 deletions src/node_builtins.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ BuiltinLoader::BuiltinLoader()
AddExternalizedBuiltin("internal/deps/undici/undici",
STRINGIFY(NODE_SHARED_BUILTIN_UNDICI_UNDICI_PATH));
#endif // NODE_SHARED_BUILTIN_UNDICI_UNDICI_PATH

#if HAVE_AMARO
#ifdef NODE_SHARED_BUILTIN_AMARO_DIST_INDEX_PATH
AddExternalizedBuiltin("internal/deps/amaro/dist/index",
STRINGIFY(NODE_SHARED_BUILTIN_AMARO_DIST_INDEX_PATH));
#endif // NODE_SHARED_BUILTIN_AMARO_DIST_INDEX_PATH
#endif // HAVE_AMARO
}

bool BuiltinLoader::Exists(const char* id) {
Expand Down
2 changes: 2 additions & 0 deletions src/node_metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,11 @@ Metadata::Versions::Versions() {
cjs_module_lexer = CJS_MODULE_LEXER_VERSION;
uvwasi = UVWASI_VERSION_STRING;

#ifndef NODE_SHARED_BUILTIN_AMARO_DIST_INDEX_PATH
#if HAVE_AMARO
amaro = AMARO_VERSION;
#endif
#endif

#if HAVE_OPENSSL
openssl = GetOpenSSLVersion();
Expand Down
2 changes: 1 addition & 1 deletion src/node_metadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace node {
#define NODE_HAS_RELEASE_URLS
#endif

#if HAVE_AMARO
#if HAVE_AMARO && !defined(NODE_SHARED_BUILTIN_AMARO_DIST_INDEX_PATH)
#define NODE_VERSIONS_KEY_AMARO(V) V(amaro)
#else
#define NODE_VERSIONS_KEY_AMARO(V)
Expand Down
5 changes: 4 additions & 1 deletion test/parallel/test-process-versions.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,12 @@ const expected_keys = [
];

const hasUndici = process.config.variables.node_builtin_shareable_builtins.includes('deps/undici/undici.js');
const hasAmaro = process.config.variables.node_builtin_shareable_builtins.includes('deps/amaro/dist/index.js');

if (process.config.variables.node_use_amaro) {
expected_keys.push('amaro');
if (hasAmaro) {
expected_keys.push('amaro');
}
}
if (hasUndici) {
expected_keys.push('undici');
Expand Down

0 comments on commit 0659516

Please sign in to comment.