FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: add env and cli option to disable global search paths by zcbenz · Pull Request #39754 · nodejs/node · GitHub

/ node Public

src: add env and cli option to disable global search paths - #39754

Closed
zcbenz wants to merge 2 commits into
nodejs:masterfrom
zcbenz:no-global-search-patsh
ConversationCommits (2)ChecksFiles changed
Closed

src: add env and cli option to disable global search paths#39754
zcbenz wants to merge 2 commits into
nodejs:masterfrom
zcbenz:no-global-search-patsh

Conversation

zcbenz commented Aug 13, 2021

Copy link
Copy Markdown
Contributor

For embedders it is usually not allowed for Node.js to search modules from global paths like $HOME/.node_modules, otherwise the behavior of the app could be changed because of a global installed module. For example, a global installed electron module should not break all require('electron') calls.

This PR adds an option to disable searching modules from global paths.

Also note that this change will have a merge conflict with #39712.

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 Aug 13, 2021
zcbenz force-pushed the no-global-search-patsh branch from 6deacca to 14b6713 Compare August 13, 2021 02:27
addaleax added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. and removed needs-ci PRs that need a full CI run. labels Aug 13, 2021

nodejs-github-bot commented Aug 13, 2021

Copy link
Copy Markdown
Collaborator

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

addaleax added the embedding Issues and PRs related to embedding Node.js in another project. label Aug 13, 2021
aduh95 reviewed Aug 13, 2021
View reviewed changes
Comment thread lib/internal/bootstrap/pre_execution.js Outdated
Comment thread lib/internal/options.js Outdated
Comment thread lib/internal/options.js Outdated

bmeck commented Aug 13, 2021

Copy link
Copy Markdown
Member

i know it might be slightly out of scope, but could we add a CLI option for this or something so that stock installs could test against this behavior?

zcbenz force-pushed the no-global-search-patsh branch from 14b6713 to d3ea98b Compare August 16, 2021 01:01

zcbenz commented Aug 16, 2021

Copy link
Copy Markdown
Contributor Author

i know it might be slightly out of scope, but could we add a CLI option for this or something so that stock installs could test against this behavior?

I think it is a good idea, I have added --no-global-search-paths cli option in this PR.

Note that the version number in doc/api/cli.md is left as 16.?.? since I don't know what to fill.

zcbenz changed the title src: add option to disable global search paths src: add env and cli option to disable global search paths Aug 16, 2021
zcbenz force-pushed the no-global-search-patsh branch from d3ea98b to b540122 Compare August 16, 2021 08:01
aduh95 mentioned this pull request Aug 16, 2021
Closed
aduh95 reviewed Aug 16, 2021
View reviewed changes
Comment thread doc/api/cli.md Outdated
zcbenz force-pushed the no-global-search-patsh branch from b540122 to 3dc0320 Compare August 16, 2021 11:10
addaleax reviewed Aug 16, 2021
View reviewed changes
Comment thread src/env.cc Outdated
zcbenz force-pushed the no-global-search-patsh branch from 3dc0320 to 7e7127d Compare August 17, 2021 01:34

jasnell commented Aug 17, 2021

Copy link
Copy Markdown
Member

Needs a rebase and a new CI run. Removing the author ready label for now.

jasnell removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Aug 17, 2021
zcbenz force-pushed the no-global-search-patsh branch from 7e7127d to f961b96 Compare August 18, 2021 07:19
addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Sep 1, 2021

nodejs-github-bot commented Sep 1, 2021

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Sep 3, 2021

Copy link
Copy Markdown
Collaborator

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

aduh95 commented Sep 16, 2021

Copy link
Copy Markdown
Contributor

@zcbenz Can you please rebase again to fix the git conflict?

zcbenz force-pushed the no-global-search-patsh branch from f961b96 to bc14859 Compare September 17, 2021 00:23
gengjiawen added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 17, 2021
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 17, 2021

nodejs-github-bot commented Sep 17, 2021

Copy link
Copy Markdown
Collaborator

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

aduh95 reviewed Sep 17, 2021
View reviewed changes
Comment thread src/node_options.cc
targos added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 18, 2021
targos pushed a commit that referenced this pull request Sep 18, 2021
src: add option to disable global search paths
e4852e3 
PR-URL: #39754
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
targos pushed a commit that referenced this pull request Sep 18, 2021
src: add --no-global-search-paths cli option
d9791d9 
PR-URL: #39754
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>

targos commented Sep 18, 2021

Copy link
Copy Markdown
Member

Landed in 40c6e83...d9791d9

targos closed this Sep 18, 2021
BethGriggs pushed a commit that referenced this pull request Sep 21, 2021
src: add option to disable global search paths
a23ea5f 
PR-URL: #39754
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
BethGriggs pushed a commit that referenced this pull request Sep 21, 2021
src: add --no-global-search-paths cli option
2083b83 
PR-URL: #39754
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
BethGriggs added a commit that referenced this pull request Sep 21, 2021
2021-09-22, Version 16.10.0 (Current)
45a042b 
Notable changes:

crypto:
  * (SEMVER-MINOR) add rsa-pss keygen parameters (Filip Skokan) #39927
doc:
  * add Ayase-252 to collaborators (Qingyu Deng) #40078
fs:
  * (SEMVER-MINOR) make `open` and `close` stream override optional when unused (Antoine du Hamel) #40013
http:
  * (SEMVER-MINOR) limit requests per connection (Artur K) #40082
src:
  * (SEMVER-MINOR) add --no-global-search-paths cli option (Cheng Zhao) #39754
  * (SEMVER-MINOR) add option to disable global search paths (Cheng Zhao) #39754
  * (SEMVER-MINOR) make napi_create_reference accept symbol (JckXia) #39926
stream:
  * (SEMVER-MINOR) add signal support to pipeline generators (Robert Nagy) #39067

PR-URL: TODO
BethGriggs pushed a commit that referenced this pull request Sep 21, 2021
src: add option to disable global search paths
51f9ad4 
PR-URL: #39754
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
BethGriggs pushed a commit that referenced this pull request Sep 21, 2021
src: add --no-global-search-paths cli option
9a67296 
PR-URL: #39754
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
BethGriggs added a commit that referenced this pull request Sep 21, 2021
2021-09-22, Version 16.10.0 (Current)
10b4fe1 
Notable changes:

crypto:
  * (SEMVER-MINOR) add rsa-pss keygen parameters (Filip Skokan) #39927
doc:
  * add Ayase-252 to collaborators (Qingyu Deng) #40078
fs:
  * (SEMVER-MINOR) make `open` and `close` stream override optional when unused (Antoine du Hamel) #40013
http:
  * (SEMVER-MINOR) limit requests per connection (Artur K) #40082
src:
  * (SEMVER-MINOR) add --no-global-search-paths cli option (Cheng Zhao) #39754
  * (SEMVER-MINOR) add option to disable global search paths (Cheng Zhao) #39754
  * (SEMVER-MINOR) make napi_create_reference accept symbol (JckXia) #39926
stream:
  * (SEMVER-MINOR) add signal support to pipeline generators (Robert Nagy) #39067

PR-URL: TODO
BethGriggs mentioned this pull request Sep 21, 2021
Merged
1 task
BethGriggs added a commit that referenced this pull request Sep 22, 2021
2021-09-22, Version 16.10.0 (Current)
aff2a0a 
Notable changes:

crypto:
  * (SEMVER-MINOR) add rsa-pss keygen parameters (Filip Skokan) #39927
doc:
  * add Ayase-252 to collaborators (Qingyu Deng) #40078
fs:
  * (SEMVER-MINOR) make `open` and `close` stream override optional when unused (Antoine du Hamel) #40013
http:
  * (SEMVER-MINOR) limit requests per connection (Artur K) #40082
src:
  * (SEMVER-MINOR) add --no-global-search-paths cli option (Cheng Zhao) #39754
  * (SEMVER-MINOR) add option to disable global search paths (Cheng Zhao) #39754
  * (SEMVER-MINOR) make napi_create_reference accept symbol (JckXia) #39926
stream:
  * (SEMVER-MINOR) add signal support to pipeline generators (Robert Nagy) #39067

PR-URL: #40175
BethGriggs added a commit that referenced this pull request Sep 22, 2021
2021-09-22, Version 16.10.0 (Current)
8d5675e 
Notable changes:

crypto:
  * (SEMVER-MINOR) add rsa-pss keygen parameters (Filip Skokan) #39927
doc:
  * add Ayase-252 to collaborators (Qingyu Deng) #40078
fs:
  * (SEMVER-MINOR) make `open` and `close` stream override optional when unused (Antoine du Hamel) #40013
http:
  * (SEMVER-MINOR) limit requests per connection (Artur K) #40082
src:
  * (SEMVER-MINOR) add --no-global-search-paths cli option (Cheng Zhao) #39754
  * (SEMVER-MINOR) add option to disable global search paths (Cheng Zhao) #39754
  * (SEMVER-MINOR) make napi_create_reference accept symbol (JckXia) #39926
stream:
  * (SEMVER-MINOR) add signal support to pipeline generators (Robert Nagy) #39067

PR-URL: #40175

ljharb commented Sep 22, 2021

Copy link
Copy Markdown
Member

Any chance the default for this could be switched in a semver-major?

zcbenz commented Sep 23, 2021

Copy link
Copy Markdown
Contributor Author

Any chance the default for this could be switched in a semver-major?

It will probably break utilities installed with npm i -g.

Some Linux distributions also provide packages for node modules, and they rely on global search paths to work.
https://packages.ubuntu.com/search?keywords=node&searchon=names&suite=hirsute&section=all

ljharb commented Sep 23, 2021

Copy link
Copy Markdown
Member

Globally installed modules are pretty community-discouraged, as i think are distro-distributed npm packages, but fair callout.

codebytere added a commit to electron/electron that referenced this pull request Sep 23, 2021
src: add env and cli option to disable global search paths
6f5b56e 
nodejs/node#39754
codebytere added a commit to electron/electron that referenced this pull request Sep 27, 2021
src: add env and cli option to disable global search paths
83c15bc 
nodejs/node#39754
codebytere added a commit to electron/electron that referenced this pull request Sep 27, 2021
src: add env and cli option to disable global search paths
5dbcbcf 
nodejs/node#39754
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

addaleax addaleax approved these changes
aduh95 aduh95 left review comments
jasnell jasnell approved these changes
tniessen tniessen approved these changes
richardlau richardlau approved these changes
+2 more reviewers
fhinkel fhinkel approved these changes
targos targos approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. embedding Issues and PRs related to embedding Node.js in another project. lib / src Issues and PRs related to general changes in the lib or src directory. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Back | FazBrowse Home | New Git URL