Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: add --install runtime flag #57593

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Mar 22, 2025

This PR adds a way to download stuff from a lock file. The idea is to give folks an option to download a package manager and a specific node version. It could also enables a use-case to use a project without needing any package manager (useful for CI, and users who don't need to add/remove/update any deps, just consume them as is).

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/config
  • @nodejs/security-wg
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added 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 Mar 22, 2025
Copy link
Member

@GeoffreyBooth GeoffreyBooth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What lockfile formats would this support, or are we inventing our own?

How would we avoid the secret key issue that recently affected Corepack?

@@ -27,6 +27,7 @@ declare_args() {
"deps/cjs-module-lexer/dist/lexer.js",
"deps/undici/undici.js",
"deps/amaro/dist/index.js",
"deps/corepack/dist/lib/download.js",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this would make Corepack a dependency of Node.js core? I'm not sure how I feel about that. I still have concerns about how it differs so much from the Node codebase (written in TypeScript, dependent on Yarn) and it has an uncertain future. This seems to make the project responsible for Corepack's continued development. It also seems to contradict the recent vote to remove Corepack (I understand it would become a hidden dependency and no longer available as a binary, but presumably it would be accessible via --expose-internals?)

We already bundle npm and it is the canonical dependency downloader; why not use that? Or more targeted dependencies of npm like Arborist.

Copy link
Contributor Author

@aduh95 aduh95 Mar 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npm has arguably an even more uncertain future, it is not maintained by us. But in any case, which code does the download matters very little (Corepack as a dep, copy the code from Corepack into core, another lib, etc.), what’s more important to establish is whether we want the feature

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Define “the feature”. Is it installing npm's lockfile? Yarn's or pnpm's? All of them? Something else? Does it depend on the package manager being downloaded first?

@aduh95 aduh95 requested a review from GeoffreyBooth March 23, 2025 17:45
@aduh95
Copy link
Contributor Author

aduh95 commented Mar 23, 2025

How would we avoid the secret key issue that recently affected Corepack?

It’s a public key, not a secret key. The lock files always contain a trusted hash, so there’s no signature to check with the current implementation (also it doesn’t have the concept of a registry, it only accepts URLs)

@aduh95 aduh95 removed the request for review from GeoffreyBooth March 23, 2025 17:53
@AugustinMauroy
Copy link
Member

If we are going to do some npm like stuff, we should not do an equivalent to corepack and just use node --install instead of N° package manager

@ljharb
Copy link
Member

ljharb commented Mar 25, 2025

Installing packages seems like the sole purview of a package manager and not a platform; I think that conceptually this isn't a feature that node should add.

@GeoffreyBooth
Copy link
Member

I’m not sure who the target audience is for this feature. Presumably it’s users who want to install dependencies from a lockfile but really want to avoid using npm at all, either to do that install or to install their preferred package manager to do that install. I understand that there are such users out there, such as the maintainers of Corepack, but my sense is that the vast majority of our users aren’t so offended by npm that they wouldn’t consider using it at least as a means to get their preferred package manager. Even for the npm-avoiding users, they could use another method like brew or curl to get their package manager of choice.

We already ship a way to install dependencies from a lockfile: our bundled npm. Adding another method would need to add meaningful, useful functionality (like support for different lockfile formats or improved performance) to justify the maintenance burden.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants