nodejs / corepack

Zero-runtime-dependency package acting as bridge between Node projects and their package managers
MIT License
2.31k stars 146 forks source link

fix: remove npm #418

Open darcyclarke opened 4 months ago

darcyclarke commented 4 months ago

This PR removes npm from Corepack. npm was supposed to be left out of Corepack & many people were/still are under the impression Corepack would not touch npm (although corepack enable npm has persisted); This PR makes it clear only yarn & pnpm will be managed & distributed by Corepack (likely superseding #407).

Fixes: #210, #419

@nodejs/npm

arcanis commented 4 months ago

I don't use npm, but when I do, it's through Corepack. I don't think we gain anything from this.

This is certainly going to be a loss of functionality for some people, but Corepack shouldn't include projects against the wish of their maintainers. It can be added back at a later time in a separate discussion which will involve the npm team if we find a common ground, but that's not the case today, and not something we pursue for bringing Corepack to stable.

@darcyclarke I'm generally +1, but please address @merceyz's other concern and get formal approval from someone from the actual npm team. As far as I'm aware you're working on a competing product, which makes you kind of the wrong person to make this request (part of why I didn't merge my own #407).

aduh95 commented 4 months ago

Corepack shouldn't include projects against the wish of their maintainers

I disagree, I think users should be in control. We should respect the license chose by the author of the software, and AFAICT Corepack use is perfectly within the bounds of npm-cli license: ~https://github.com/npm/cli/blob/af3c48e074d03caebaa8ed24d39405329f545497/LICENSE#L105C1-L112C30~ EDIT: as pointed out by Michaël, we do not distribute the npm source, we download the tarball from the npm registry.

targos commented 4 months ago

I agree with @aduh95 but I don't think this section of the license is relevant. Corepack doesn't include or distribute npm's source code.

arcanis commented 4 months ago

I disagree, I think users should be in control. We should respect the license chose by the author of the software

My line of thinking was that some basic level of support should be preferred, as it impacts the developer experience of the users of those tool (for instance, taking an extreme example, I'd certainly dislike it very much if the corepack keyword was expected to be added in front of every yarn command - it'd compromise our devx).

With that said I also understand your point - as far as I know Debian / Ubuntu / etc don't request permission to include softwares into their package lists (including npm and yarn), and our teams have obviously no control on how should apt be implemented, so perhaps the situation is similar here.

darcyclarke commented 4 months ago

As far as I'm aware you're working on a competing product, which makes you kind of the wrong person to make this request (part of why I didn't merge my own #407)

I didn't realize there was extra criteria for contributing to this project. Who is "the right person"?

I'm sad you're questioning either my ethics, experience or insight in regards to even just proposing a change.

To be clear, it seemed less-ethical to propose changes to Corepack while I was still managing npm (@nodejs/npm can speak for themselves since I'm no longer affiliated).

Objectively, I'm actively working against my own interests (ie. building a startup) spending time here/contributing. I could be doing anything else & am instead sticking my neck out because others aren't willing to engage here. It would be better for me to stay back & let the chaos continue without trying to advocate for a better experience for end-users (myself included) with actual code changes.

Using affiliation to discredit contributions discourages people from engaging & can easily be turned around on you (ex. "were you the 'right person' to propose adding yarn or Corepack to core?" or "are you the 'right person' to merge your own PRs?" - you don't have to answer those because I think they're unfair & believe you have good intentions - but everyone has bias)

Removing npm properly should have been done years ago. It can always be added back.

aduh95 commented 4 months ago

it seemed less-ethical to propose changes to Corepack while I was still managing npm

FWIW I don't think there's anything ethically wrong with npm folks contributing to Corepack, quite the opposite.

melroy89 commented 3 months ago

I don't use npm, but when I do, it's through Corepack. I don't think we gain anything from this.

Agreed. We just should make a split between runtime and package manager.

EDIT: Also Corepack takes care of auto installing the package manager. So it make sense to keep npm part of Corepack.

ruyadorno commented 3 months ago

@lukekarrys if I understand well your comment an important question for the @nodejs/tsc here is whether we would be ok with enabling an unsupported install path for npm in node core?

merceyz commented 3 months ago

This PR makes it clear only yarn & pnpm will be managed & distributed by Corepack

Note that if this PR lands it will most likely only make using npm through Corepack inconvenient, not impossible, since URLs are supported https://github.com/nodejs/corepack/pull/359.

ronag commented 3 months ago

@darcyclarke or anyone else in favor of this PR. Could you please address the following objection:

... it's a fairly clear statement that the current setup is satisfying enough to the npm team ...

Seems to me that the motivation for this PR is not valid? Or am I missing something?

styfle commented 3 months ago

I see two responses from the npm team from two different people that both claim corepack enable npm is fine.

My request would be that if corepack is enabled by default that npm support remain behind an additional flag or command that would be opt-in for developers. –MylesBorins

corepack enable npm won't be an officially supported install path for npm, but users should be able to run this command on their own system if they want Corepack to manage npm. –lukekarrys

I must be missing something 🤔

darcyclarke commented 3 months ago

@ronag I took @lukekarrys' response (on behalf of the npm team) to be extremely diplomatic & yet contradictory.

... users should be able to run this command on their own system if they want Corepack to manage npm.

This is pretty clear, the npm team doesn't care how users install & manage npm.

corepack enable npm won't be an officially supported install path for npm ...

This is pretty clear, the npm team doesn't want to support users that install & manage npm with corepack.

The question this leaves for me (& everyone here should ask themselves) is why we care about & want to maintain an install path for npm when they do not want it for themselves or support it for their end-users? This response & outlook is in stark contrast to the other projects (ie. yarn & pnpm) that are in full support of this project & method of installation.

My stance remains that npm should not be included in this experiment (as it was expressed to everyone it would not be) as we should not be burdening ourselves with supporting an unofficial/unsupported installation method for npm (as has been identified & reiterated by that team many many times).

If objections remain, this PR should be voted on by the TSC (afaict, it can happen async of any other discussion about corepack's future)

@nodejs/tsc

darcyclarke commented 3 months ago

One other important note in regards to licensing (which @aduh95 / @targos commented on): I believe the specific provision which should be reviewed & understood by legal counsel (to ensure Corepack is compliant) is actually 4. ("Distribution of Modified Versions of the Package as Source ref. https://github.com/npm/cli/blob/af3c48e074d03caebaa8ed24d39405329f545497/LICENSE#L120-L137)

...

    "Standard Version" refers to the Package if it has not been
    modified, or has been modified only in ways explicitly requested
    by the Copyright Holder.

    "Modified Version" means the Package, if it has been changed, and
    such changes were not explicitly requested by the Copyright
    Holder.

...

Distribution of Modified Versions of the Package as Source

(4)  You may Distribute your Modified Version as Source (either gratis
or for a Distributor Fee, and with or without a Compiled form of the
Modified Version) provided that you clearly document how it differs
from the Standard Version, including, but not limited to, documenting
any non-standard features, executables, or modules, and provided that
you do at least ONE of the following:

    (a)  make the Modified Version available to the Copyright Holder
    of the Standard Version, under the Original License, so that the
    Copyright Holder may include your modifications in the Standard
    Version.

    (b)  ensure that installation of your Modified Version does not
    prevent the user installing or running the Standard Version. In
    addition, the Modified Version must bear a name that is different
    from the name of the Standard Version.

    (c)  allow anyone who receives a copy of the Modified Version to
    make the Source form of the Modified Version available to others
    under

        (i)  the Original License or

        (ii)  a license that permits the licensee to freely copy,
        modify and redistribute the Modified Version using the same
        licensing terms that apply to the copy that the licensee
        received, and requires that the Source form of the Modified
        Version, and of any works derived from it, be made freely
        available in that license fees are prohibited but Distributor
        Fees are allowed.

I am not a lawyer, but Corepack distributing an npm binary (ie. a "Modified Version") which blocks the installation/running of the "Standard Version" (ie. the real npm) & has not changed it's name, seems to be in violation of this license. If it continued to - & this PR was overturned - we'd likely need to comply with 4.a or 4.c & whether or not we are already compliant within the bounds of 4.c.ii. is, again, a question for a lawyer (ie. can npm Inc./GitHub modify & redistribute Corepack using the same license they distribute npm with, under the current license in use today?).

Both the OpenJS Foundation & GitHub's own lawyers should probably look at this licensing & implementation to ensure there are no violations (especially given that the whole intention of this implementation is to impersonate another trademarked software).

bensternthal commented 3 months ago

Hi folks, OpenJS will bring in our legal counsel to provide an opinion on Darcy's license question.

styfle commented 3 months ago

I am not a lawyer, but Corepack distributing an npm binary (ie. a "Modified Version")

In what way does Corepack modify npm? Isn't it installing exactly whats on the registry?

darcyclarke commented 3 months ago

In what way does Corepack modify npm?

I am not a lawyer & I expect the OpenJS Foundation's legal counsel to provide an authoritive interpretation/response as @bensternthal notes above (not sure if GitHub will also provide their interpretation or engage their lawyers).

That said, from my perspective - as an engineer/end-user - when Corepack places a npm binary on your system (which is not npm's "Standard Version"/source) it is distributing a "Modified Version" (this interpretation may or may not be shared by others/counsel).

If we chose a different name then "npm" for the binary then you could say it was a different thing but this project intentionally names these binaries the same as the respective sources in order to intercept calls to them &, subsequently, execute or prevent the execution of the respective source/"Standard Version" (ref. see the behavior defined for COREPACK_ENABLE_STRICT - which is on by default - which gates the installation/execution of respective sources based on a project's packageManager field).

npm's license was drafted in such a way to explicitly protect against scenarios like this where software would mimic/impersonate/copy it while at the same time preventing access to it's upstream source. I do believe Corepack is likely violating the license, if not the spirit of the license (again, this is my opinion/interpretation & may not be shared).

The installation of npm's "Standard Version" post-validation is not in question. The question is whether Corepack's distribution of a "Modified Version" of npm, mechanism to prevent access to npm's "Standard Version" or license itself violate npm's licensing or not (ie. see the specific terms I previously referenced, there may also be other relevant terms I'm not aware of - again, ianal).

Notably, whether it's deemed to be in violation or not, my previous reasoning & support for this PR remains.

isaacs commented 3 months ago

I am not a lawyer. And, any IP rights I might have to npm's source code were long ago handed over to npm, Inc. and then subsequently to GitHub, so I don't have standing here.

But, my belief is that including the npm cli in corepack is indeed a violation of the letter of the license, and I am 100% confident that it is a violation of the spirit of the license.

The Artistic License 2.0 as chosen very deliberately to disallow distribution of a modified form of "npm" that was being distributed in Homebrew: jumper binaries that download source, wrap the original version, hard-code configurations, and so on. Essentially, exactly what corepack is doing.

Like I said, I'm not a lawyer, and don't have standing, since I sold my rights to GitHub. But for what it's worth, I am angry and disappointed to learn that npm is being distributed in this way, when I released it under a license that specifically disallowed this. At the very least, you should check with your own lawyers and possibly any contributors who didn't sign away their IP, since their objections may hold legal weight.

If you want to include npm in corepack, please call it something else other than "npm", so that it's clear that it's not the actual thing. Doing otherwise is misleading and unethical.

ronag commented 3 months ago

Adding to tsc agenda due to the legal concerns raised.

mcollina commented 3 months ago

We should just remove it, as quickly as possible. I don't think spending money on lawyers is worth it here.

Thanks @darcyclarke and @isaacs for bringing this up.

isaacs commented 3 months ago

To be 100% clear: I am specifically not making a legal threat or claim of any sort. I can't. Even if it was a very clear violation of the license, I don't own any of the IP licensed under Artistic 2.0. If GitHub says ok, then it's allowed. (Edit to add: also any other contributors who were not GitHub employees or otherwise signed over their rights, of course.)

I am saying that it makes me feel some kind of way, as the author who released it under that license for that purpose. Just a human having emotions. Maybe that's not enough to outweigh other considerations (and legally, it isn't even relevant at all), but I'd still consider it a data point if I was on the other side of this.

styfle commented 3 months ago

This would be a sad day for developers if this was a violation of the license.

Something like alias npm="socket npm" might also be a violation, if true.

https://socket.dev/blog/introducing-safe-npm

rginn commented 3 months ago

In initially reviewing the license and related issues with our OpenJS legal counsel, it appears that there is no clear legal answer. If we decided to continue pursuing this investigation with our legal counsel, we would need to formally bring in independent technical experts given the interpretive nature of the issue, according to our attorney.

Unfortunately, the OpenJS Foundation cannot do a legal investigation of how all our projects interact with all third-party software - we often don’t have the rights or resources for that ongoing responsibility. We do support our project communities with legal support, such as trademarks, government compliance, and policy, but again with limited resources.

And as an umbrella organization, we don’t make technical decisions for our projects. In this case, the Node.js TSC is clearly autonomous to make technical decisions on what’s best for the project. I’m encouraged by these open discussions and advocate for an alternative technical approach working collaboratively with these project communities, and our CPC and Collaboration Spaces when possible.

darcyclarke commented 3 months ago

Something like alias npm="socket npm" might also be a violation, if true.

Creating an alias or developing something for personal use on your own machine is not the same as distributing/redistributing modified software, links or interfaces.

In the above, creating an alias yourself would be fine but depending on how you got socket npm & what it does may or may not indeed violate the license.

mcollina commented 3 months ago

Based on my understanding of the technology, because npm is not enabled by default the legal argument is a gray area. I don’t think we would be able to enable it by default either. Nor we plan to, at least on Node.js core.

With my Board hat on, I would say there is little legal risk to keeping this as is. It's not worth litigating this issue with foundation attorneys, which would run up large legal costs. It's better to solve this for what's the best technical decision for the developer experience.

jasnell commented 3 months ago

I'm not going to weigh in on any legal issue. What I would comment on is that there doesn't seem to be any real consideration in this conversation about what is in our user's best interest. What makes user's lives easier or harder is a much more interesting conversation. I'm not going to offer an opinion one way or the other on this particular PR but it would be nice if this conversation could be more productively focused.

GeoffreyBooth commented 3 months ago

What makes user's lives easier or harder is a much more interesting conversation.

If Node unbundles Corepack then it can do whatever it wants, including enabling support for npm by default, without any concerns from us. Of course this entails a tradeoff, in that users would run npm install -g corepack instead of npm enable corepack, but in return they'd get a much more powerful Corepack that has no restrictions imposed on it by Node.

isaacs commented 3 months ago

@jasnell

What I would comment on is that there doesn't seem to be any real consideration in this conversation about what is in our user's best interest.

Fwiw, the original choice to say "you are not allowed to ship not-npm and call it npm" is almost entirely motivated by user confusion and the frustration of being unable to support people who came to me with bugs that I could not fix. That's a terrible user experience that I observed repeatedly back then, and it's one that I think is inevitable when software is overly magical in ways that the authors do not anticipate.

I say "almost" entirely motivated because of course there's also the reputational impact of those people thinking I'd created bugs that I hadn't, and that's a bit more self-serving.

legobeat commented 1 day ago

But, my belief is that including the npm cli in corepack is indeed a violation of the letter of the license, and I am 100% confident that it is a violation of the spirit of the license.

I may be missing something but that was never the situation I believe?

corepack is a utility which downloads and installs package managers locally on behalf of users and does not actually ship with npm (and AFAIK this was never the topic here). Very much like nvm does for Node.js. Or npm itself does for other packages. This is true for sources as well as the packaged releases.

This is opposed to distributions like Fedora, Alpine Linux, Debian, and Arch, which do repackage and redistribute it. (Neither in violation of actual letter or spirit of license, AFAICT)

Am I missing something, or is the whole "potential legal issue with regards to licensing and distribution" just a misunderstanding and actually not one?

(Personal feelings aside. They are valid and, as you already noted, irrelevant to the conclusion of this PR, which seemed to be concluded with "closing" until confusion ensued).

"corepack can keep supporting npm for users who desire" and "corepack is not the currently recommended npm installation method from npm" don't seem to be in conflict at all?


IMO: npm is a package manager among others. That Node.js ships with both corepack and npm bundled by default (disabling use of the latter for the former until explicitly opted in by the user) seems to be fine at least for the time being?