src: add option to disable loading native addons #39977

d3lm · 2021-09-02T13:37:09Z

This PR adds a new CLI option --no-addons which disables loading native addons. When this option is specified, loading a native addon will throw from the internal side (node_binding.cc#DLOpen). In addition, using this option will enable a new resolution condition no-addons. The motivation behind this PR originates from this proposal. The TLDR is that we wanted to propose a way to disable native addons and allow open source maintainers to conditionally export different entry points, e.g. an entry point that uses native addons vs. an entry point that for instance uses WebAssembly to enable it to run in environments that don't allow running native code such as browsers.

For the initial PR I have decided to go with --no-addons for the CLI option, but if anyone has suggestions I am happy to change it.

Tests added
make -j4 test passes

doc/api/cli.md

src/node_binding.cc

test/parallel/test-no-addons-resolution-condition.js

targos · 2021-09-02T14:12:41Z

/cc @nodejs/modules

doc/api/cli.md

src/node_options.cc

doc/api/cli.md

bmeck

Needs test cleanup so they are all passing, but seems good. Needs an ESM test using import() under test/es-modules and it should be good to go I think.

d3lm · 2021-09-02T19:34:45Z

Thanks @bmeck for your review. I'll add a test for ESM as well as fix the tests.

cjihrig · 2021-09-02T19:36:39Z

This seems like something that would also be a good addition to policies.

bmeck · 2021-09-02T19:37:53Z

@cjihrig i think a more general forced CLI exec flags would make more sense since there are lots of these that are potentially useful.

addaleax

You probably want to add this to EnvironmentFlags in node.h and forward it to workers like we do for e.g. track_unmanaged_fds/kTrackUnmanagedFds

src/node_binding.cc

bmeck

LGTM

nodejs-github-bot · 2021-09-03T14:51:16Z

CI: https://ci.nodejs.org/job/node-test-pull-request/39755/

GeoffreyBooth · 2021-09-06T05:10:35Z

In addition, using this option will enable a new resolution condition no-addons.

cc @guybedford

bmeck · 2021-09-08T20:35:36Z

The failures seem to be shared across a couple different PRs and do not seem to be related to the current PR. I will merge tomorrow if there are no objections.

nodejs-github-bot · 2021-09-09T14:51:35Z

CI: https://ci.nodejs.org/job/node-test-pull-request/39881/

mhdawson · 2021-09-09T14:52:03Z

Did a resume build, hopefully that will be green.

d3lm · 2021-09-09T17:53:07Z

@mhdawson Doesn't seem to make CI more happy. And it does indeed look like other PRs suffer from the same issue so it's unrelated to this PR.

nodejs-github-bot · 2021-09-09T21:15:46Z

CI: https://ci.nodejs.org/job/node-test-pull-request/39888/

mhdawson · 2021-09-09T21:55:14Z

Remaining failure looks to be: #38226.

Our guidance says to try to get to green by resuming build as opposed to landing with Red, but I think since that failure is well known we can go ahead and land. I'm assuming @bmeck is going to do that.

nodejs-github-bot · 2021-09-10T11:01:54Z

CI: https://ci.nodejs.org/job/node-test-pull-request/39897/

PR-URL: #39977 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

bmeck · 2021-09-10T15:55:26Z

Landed in a9dd03b

PR-URL: #39977 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

Backport-PR-URL: #40094 PR-URL: #39977 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

Notable changes: Corepack: Node.js now includes Corepack, a script that acts as a bridge between Node.js projects and the package managers they are intended to be used with during development. In practical terms, Corepack will let you use Yarn and pnpm without having to install them - just like what currently happens with npm, which is shipped in Node.js by default. Contributed by Maël Nison - #39608 ICU updated: ICU has been updated to 70.1. This updates timezone database to 2021a3, including bringing forward the start for DST for Jordan from March to February. Contributed by Michaël Zasso - #40658 New option to disable loading of native addons: A new command line option `--no-addons` has been added to disallow loading of native addons. Contributed by Dominic Elm - #39977 Updated Root Certificates: Root certificates have been updated to those from Mozilla's Network Security Services 3.71. Contributed by Richard Lau - #40280 Other Notable Changes: crypto: * (SEMVER-MINOR) make FIPS related options always available (Vít Ondruch) #36341 lib: * (SEMVER-MINOR) add unsubscribe method to non-active DC channels (simon-id) #40433 * (SEMVER-MINOR) add return value for DC channel.unsubscribe (simon-id) #40433 module: * (SEMVER-MINOR) support pattern trailers (Guy Bedford) #39635 src: * (SEMVER-MINOR) make napi_create_reference accept symbol (JckXia) #39926 PR-URL: #41696

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [node](https://github.com/nodejs/node) | stage | minor | `14.18.3-bullseye-slim` -> `14.19.0-bullseye-slim` | --- ### Release Notes <details> <summary>nodejs/node</summary> ### [`v14.19.0`](https://github.com/nodejs/node/releases/v14.19.0) [Compare Source](nodejs/node@v14.18.3...v14.19.0) ##### Notable Changes ##### Corepack Node.js now includes Corepack, a script that acts as a bridge between Node.js projects and the package managers they are intended to be used with during development. In practical terms, **Corepack will let you use Yarn and pnpm without having to install them** - just like what currently happens with npm, which is shipped in Node.js by default. Please head over to the [Corepack documentation page](https://nodejs.org/dist/latest-v14.x/docs/api/corepack.html) for more information on how to use it. Contributed by Maël Nison - [#39608](nodejs/node#39608) ##### ICU updated ICU has been updated to 70.1. This updates timezone database to 2021a3, including bringing forward the start for DST for Jordan from March to February. Contributed by Michaël Zasso - [#40658](nodejs/node#40658) ##### New option to disable loading of native addons A new command line option `--no-addons` has been added to disallow loading of native addons. Contributed by Dominic Elm - [#39977](nodejs/node#39977) ##### Updated Root Certificates Root certificates have been updated to those from Mozilla's Network Security Services 3.71. Contributed by Richard Lau - [#40280](nodejs/node#40280) ##### Other Notable Changes - \[[`0d448eaab5`](nodejs/node@0d448eaab5)] - **(SEMVER-MINOR)** **crypto**: make FIPS related options always available (Vít Ondruch) [#36341](nodejs/node#36341) - \[[`004eafbebf`](nodejs/node@004eafbebf)] - **(SEMVER-MINOR)** **lib**: add unsubscribe method to non-active DC channels (simon-id) [#40433](nodejs/node#40433) - \[[`625be7585d`](nodejs/node@625be7585d)] - **(SEMVER-MINOR)** **lib**: add return value for DC channel.unsubscribe (simon-id) [#40433](nodejs/node#40433) - \[[`607bc74eae`](nodejs/node@607bc74eae)] - **(SEMVER-MINOR)** **module**: support pattern trailers (Guy Bedford) [#39635](nodejs/node#39635) - \[[`f74fe2a59c`](nodejs/node@f74fe2a59c)] - **(SEMVER-MINOR)** **src**: make napi_create_reference accept symbol (JckXia) [#39926](nodejs/node#39926) ##### Commits - \[[`0231ffa501`](nodejs/node@0231ffa501)] - **build**: add `--without-corepack` (Jonah Snider) [#41060](nodejs/node#41060) - \[[`5389b8ab05`](nodejs/node@5389b8ab05)] - **crypto**: update root certificates (Richard Lau) [#40280](nodejs/node#40280) - \[[`0d448eaab5`](nodejs/node@0d448eaab5)] - **(SEMVER-MINOR)** **crypto**: make FIPS related options always available (Vít Ondruch) [#36341](nodejs/node#36341) - \[[`cd20ecc7cb`](nodejs/node@cd20ecc7cb)] - **deps**: upgrade Corepack to 0.10 (Maël Nison) [#40374](nodejs/node#40374) - \[[`737df75e17`](nodejs/node@737df75e17)] - **(SEMVER-MINOR)** **deps**: add corepack (Maël Nison) [#39608](nodejs/node#39608) - \[[`b85aa5a143`](nodejs/node@b85aa5a143)] - **deps**: upgrade npm to 6.14.16 (Ruy Adorno) [#41603](nodejs/node#41603) - \[[`2755d391a5`](nodejs/node@2755d391a5)] - **deps**: update ICU to 70.1 (Michaël Zasso) [#40658](nodejs/node#40658) - \[[`3089326d89`](nodejs/node@3089326d89)] - **deps**: update archs files for OpenSSL-1.1.1m (Richard Lau) [#41173](nodejs/node#41173) - \[[`59da7c12aa`](nodejs/node@59da7c12aa)] - **deps**: upgrade openssl sources to 1.1.1m (Richard Lau) [#41173](nodejs/node#41173) - \[[`cede1f26f6`](nodejs/node@cede1f26f6)] - **deps**: add -fno-strict-aliasing flag to libuv (Daniel Bevenius) [#40631](nodejs/node#40631) - \[[`4477da858f`](nodejs/node@4477da858f)] - **doc**: fix corepack grammar for `--force` flag (Steven) [#40762](nodejs/node#40762) - \[[`5971d58600`](nodejs/node@5971d58600)] - **doc**: add missing YAML tag in `esm.md` (Antoine du Hamel) [#41516](nodejs/node#41516) - \[[`e903798ae1`](nodejs/node@e903798ae1)] - **doc**: add note regarding unfinished TLA (Antoine du Hamel) [#41434](nodejs/node#41434) - \[[`a90defebcf`](nodejs/node@a90defebcf)] - **esm**: make `process.exit()` default to exit code 0 (Gang Chen) [#41388](nodejs/node#41388) - \[[`fc328f1ab0`](nodejs/node@fc328f1ab0)] - **fs**: nullish coalescing to respect zero positional reads (Omar El-Mihilmy) [#40716](nodejs/node#40716) - \[[`004eafbebf`](nodejs/node@004eafbebf)] - **(SEMVER-MINOR)** **lib**: add unsubscribe method to non-active DC channels (simon-id) [#40433](nodejs/node#40433) - \[[`625be7585d`](nodejs/node@625be7585d)] - **(SEMVER-MINOR)** **lib**: add return value for DC channel.unsubscribe (simon-id) [#40433](nodejs/node#40433) - \[[`2c365961d0`](nodejs/node@2c365961d0)] - **module**: support pattern trailers for imports field (Guy Bedford) [#40041](nodejs/node#40041) - \[[`607bc74eae`](nodejs/node@607bc74eae)] - **(SEMVER-MINOR)** **module**: support pattern trailers (Guy Bedford) [#39635](nodejs/node#39635) - \[[`f74fe2a59c`](nodejs/node@f74fe2a59c)] - **(SEMVER-MINOR)** **src**: make napi_create_reference accept symbol (JckXia) [#39926](nodejs/node#39926) - \[[`b050c65885`](nodejs/node@b050c65885)] - **src**: add option to disable loading native addons (Dominic Elm) [#39977](nodejs/node#39977) - \[[`c1695ac68a`](nodejs/node@c1695ac68a)] - **tools**: update certdata.txt (Richard Lau) [#40280](nodejs/node#40280) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Reviewed-on: https://git.walbeck.it/mwalbeck/docker-jellyfin-livestream/pulls/97 Co-authored-by: renovate-bot <[email protected]> Co-committed-by: renovate-bot <[email protected]>

src: add option to disable loading native addons

3a8a4c3

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Sep 2, 2021

jasnell reviewed Sep 2, 2021

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

jasnell reviewed Sep 2, 2021

View reviewed changes

src/node_binding.cc Outdated Show resolved Hide resolved

jasnell reviewed Sep 2, 2021

View reviewed changes

test/parallel/test-no-addons-resolution-condition.js Outdated Show resolved Hide resolved

fixup: fix linting issues and address feedback

403e522

d3lm requested a review from jasnell September 2, 2021 14:13

fixup: remove unused definition

099540b

VoltrexKeyva reviewed Sep 2, 2021

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

src/node_options.cc Show resolved Hide resolved

bmeck reviewed Sep 2, 2021

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

bmeck reviewed Sep 2, 2021

View reviewed changes

addaleax reviewed Sep 3, 2021

View reviewed changes

src/node_binding.cc Outdated Show resolved Hide resolved

fixup: address feedback

59eeea9

addaleax approved these changes Sep 3, 2021

View reviewed changes

d3lm requested review from addaleax, VoltrexKeyva and bmeck September 3, 2021 14:10

VoltrexKeyva approved these changes Sep 3, 2021

View reviewed changes

bmeck approved these changes Sep 3, 2021

View reviewed changes

fixup: fix linter errors

71e0945

fixup: add test for process.dlopen

c676d72

d3lm requested a review from VoltrexKeyva September 7, 2021 10:37

mhdawson mentioned this pull request Sep 9, 2021

investigate flaky test-crypto-timing-safe-equal-benchmarks #38226

Closed

bmeck closed this Sep 10, 2021

d3lm deleted the feat/no-addons branch September 10, 2021 15:56

d3lm mentioned this pull request Sep 13, 2021

[14.x backport] src: add option to disable loading native addons #40094

Closed

This was referenced Sep 14, 2021

CI Reliability 2021-09-14 nodejs/reliability#58

Open

CI Reliability 2021-09-15 nodejs/reliability#59

Open

EricSimons mentioned this pull request Sep 15, 2021

RFC: Env flag for npm packages to conditionally load WASM binaries instead of native binaries stackblitz/webcontainer-core#286

Open

This was referenced Sep 16, 2021

CI Reliability 2021-09-16 nodejs/reliability#60

Open

CI Reliability 2021-09-17 nodejs/reliability#61

Open

CI Reliability 2021-09-18 nodejs/reliability#62

Open

CI Reliability 2021-09-19 nodejs/reliability#63

Open

BethGriggs mentioned this pull request Sep 21, 2021

v16.10.0 proposal #40175

Merged

1 task

richardlau mentioned this pull request Jan 25, 2022

v14.19.0 proposal #41696

Merged

EricSimons mentioned this pull request Mar 29, 2022

Does not work in stackblitz parcel-bundler/watcher#99

Closed

richardlau mentioned this pull request Dec 16, 2022

src,process: initial permission model implementation #44004

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

src: add option to disable loading native addons #39977

src: add option to disable loading native addons #39977

d3lm commented Sep 2, 2021 •

edited

Loading

targos commented Sep 2, 2021

bmeck left a comment

d3lm commented Sep 2, 2021 •

edited

Loading

cjihrig commented Sep 2, 2021

bmeck commented Sep 2, 2021 •

edited

Loading

addaleax left a comment

bmeck left a comment

nodejs-github-bot commented Sep 3, 2021

GeoffreyBooth commented Sep 6, 2021

bmeck commented Sep 8, 2021

nodejs-github-bot commented Sep 9, 2021

mhdawson commented Sep 9, 2021

d3lm commented Sep 9, 2021

nodejs-github-bot commented Sep 9, 2021

mhdawson commented Sep 9, 2021

nodejs-github-bot commented Sep 10, 2021

bmeck commented Sep 10, 2021

src: add option to disable loading native addons #39977

src: add option to disable loading native addons #39977

Conversation

d3lm commented Sep 2, 2021 • edited Loading

targos commented Sep 2, 2021

bmeck left a comment

Choose a reason for hiding this comment

d3lm commented Sep 2, 2021 • edited Loading

cjihrig commented Sep 2, 2021

bmeck commented Sep 2, 2021 • edited Loading

addaleax left a comment

Choose a reason for hiding this comment

bmeck left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Sep 3, 2021

GeoffreyBooth commented Sep 6, 2021

bmeck commented Sep 8, 2021

nodejs-github-bot commented Sep 9, 2021

mhdawson commented Sep 9, 2021

d3lm commented Sep 9, 2021

nodejs-github-bot commented Sep 9, 2021

mhdawson commented Sep 9, 2021

nodejs-github-bot commented Sep 10, 2021

bmeck commented Sep 10, 2021

d3lm commented Sep 2, 2021 •

edited

Loading

d3lm commented Sep 2, 2021 •

edited

Loading

bmeck commented Sep 2, 2021 •

edited

Loading