node src: sync NODE_REPL_EXTERNAL_MODULE and kDisableNodeOptionsEnv

Fixes: https://github.com/nodejs/node/issues/51227

Currently, I'm throwing kBootstrapError, but I'm almost sure this is not the appropriate error. In this case, should I create a new error?

Technically, this is a fix for kDisableNodeOptionsEnv, but it can be considered a breaking change for the ones relying on the current behavior (which is unlikely).

May 08 '24 17:05 RafaelGSS

Review requested:

[ ] @nodejs/startup

May 08 '24 17:05 nodejs-github-bot

IMO if it's a breaking change it should be semver-minor or notable-change

(But as a triage member, this isn't my decision, nor my place)

May 08 '24 18:05 redyetidev

Failed to start CI

   ⚠  No approving reviews found
   ✘  Refusing to run CI on potentially unsafe PR

https://github.com/nodejs/node/actions/runs/9008046172

May 08 '24 20:05 github-actions[bot]

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

May 09 '24 13:05 nodejs-github-bot

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

May 09 '24 18:05 nodejs-github-bot

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

May 10 '24 18:05 nodejs-github-bot

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

May 12 '24 16:05 nodejs-github-bot

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

May 13 '24 01:05 nodejs-github-bot

No opinion from me on semver for this, but the code change LGTM.

Actually, with a little more thought, I think this is semver patch, so I agree with @RafaelGSS. But I'll defer to Releasers who think otherwise or collaborators more knowledgable about how these features are used.

May 14 '24 21:05 Trott

No opinion from me on semver for this, but the code change LGTM.

Actually, with a little more thought, I think this is semver patch

Whatever you say I'm happy with, I'm not a collaborator, so I really don't have much of an opinion (and experience) to really weigh in

May 15 '24 00:05 redyetidev

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

May 16 '24 15:05 nodejs-github-bot

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

May 17 '24 14:05 nodejs-github-bot

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

May 17 '24 16:05 nodejs-github-bot

I'm getting

14:08:05 /home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/embedtest: error while loading shared libraries: libcrypto.so.1.1: cannot open shared object file: No such file or directory

I don't see why my changes would cause this. Any idea? @nodejs/build

May 18 '24 01:05 RafaelGSS

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

May 20 '24 18:05 nodejs-github-bot

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

May 21 '24 20:05 nodejs-github-bot

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

May 21 '24 22:05 nodejs-github-bot

I'm getting

14:08:05 /home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/embedtest: error while loading shared libraries: libcrypto.so.1.1: cannot open shared object file: No such file or directory

I don't see why my changes would cause this. Any idea? @nodejs/build

In your new test you're overwriting the environment and losing the environment variables required to spawn node when dynamically linked against dependencies (e.g. OpenSSL). On Linux that would LD_LIBRARY_PATH but could be LIBPATH (AIX) or DYLD_LIBRARY_PATH (macOS). The usual approach in tests is to extend the env rather than replace it.

May 21 '24 22:05 richardlau

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

May 22 '24 14:05 nodejs-github-bot

Commit Queue failed

- Loading data for nodejs/node/pull/52905
✔  Done loading data for nodejs/node/pull/52905
----------------------------------- PR info ------------------------------------
Title      src: sync NODE_REPL_EXTERNAL_MODULE and kDisableNodeOptionsEnv (#52905)
Author     Rafael Gonzaga  (@RafaelGSS)
Branch     RafaelGSS:disable-external-module -> nodejs:main
Labels     c++, repl, author ready, needs-ci, commit-queue-squash
Commits    2
 - src: fix external module env and kDisableNodeOptionsEnv
 - fixup! src: fix external module env and kDisableNodeOptionsEnv
Committers 1
 - RafaelGSS 
PR-URL: https://github.com/nodejs/node/pull/52905
Fixes: https://github.com/nodejs/node/issues/51227
Reviewed-By: Rich Trott 
Reviewed-By: Joyee Cheung 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/52905
Fixes: https://github.com/nodejs/node/issues/51227
Reviewed-By: Rich Trott 
Reviewed-By: Joyee Cheung 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - fixup! src: fix external module env and kDisableNodeOptionsEnv
   ℹ  This PR was created on Wed, 08 May 2024 17:55:32 GMT
   ✔  Approvals: 2
   ✔  - Rich Trott (@Trott): https://github.com/nodejs/node/pull/52905#pullrequestreview-2056494543
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/52905#pullrequestreview-2069699196
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-05-22T14:10:03Z: https://ci.nodejs.org/job/node-test-pull-request/59352/
- Querying data for job/node-test-pull-request/59352/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/9200620708

May 23 '24 01:05 nodejs-github-bot

Landed in b9ad94b6dadc2c34e7183e2b5f16702132d97c8d

May 23 '24 18:05 nodejs-github-bot

node node copied to clipboard

src: sync NODE_REPL_EXTERNAL_MODULE and kDisableNodeOptionsEnv

node
node copied to clipboard