-
Notifications
You must be signed in to change notification settings - Fork 170
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
fix: remove npm #418
base: main
Are you sure you want to change the base?
fix: remove npm #418
Conversation
ed0350e
to
9bfded8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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). |
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: |
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. |
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 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am reading that comment differently than you.
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.
This is exactly how corepack works today so no need to remove npm.
-1 on this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point, it's a fairly clear statement that the current setup is satisfying enough to the npm team. -1 for me then.
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 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 |
FWIW I don't think there's anything ethically wrong with npm folks contributing to Corepack, quite the opposite. |
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the position of the npm team that we do not think it is within our purview to dictate how a 3rd party tool interacts with or manages the npm CLI on a developers system when fetched dynamically.
The only request we've had to Corepack at this time is to not enable npm by default as it would complicate distribution via Node.js. If Corepack were not distributing with Node.js we would have 0 requests to the Corepack team regarding the management of local npm installation.
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 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? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Note that if this PR lands it will most likely only make using npm through Corepack inconvenient, not impossible, since URLs are supported #359. |
@darcyclarke or anyone else in favor of this PR. Could you please address the following objection:
Seems to me that the motivation for this PR is not valid? Or am I missing something? |
I see two responses from the npm team from two different people that both claim
I must be missing something 🤔 |
@ronag I took @lukekarrys' response (on behalf of the npm team) to be extremely diplomatic & yet contradictory.
This is pretty clear, the npm team doesn't care how users install & manage 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 |
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
I am not a lawyer, but Corepack distributing an 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). |
Hi folks, OpenJS will bring in our legal counsel to provide an opinion on Darcy's license question. |
In what way does Corepack modify npm? Isn't it installing exactly whats on the registry? |
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 If we chose a different name then 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. |
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. |
Adding to tsc agenda due to the legal concerns raised. |
We should just remove it, as quickly as possible. I don't Thanks @darcyclarke and @isaacs for bringing this up. |
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. |
This would be a sad day for developers if this was a violation of the license. Something like |
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. |
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 |
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. |
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. |
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 |
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. |
I may be missing something but that was never the situation I believe?
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? |
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 touchnpm
(althoughcorepack enable npm
has persisted); This PR makes it clear onlyyarn
&pnpm
will be managed & distributed by Corepack (likely superseding #407).Fixes: #210, #419
@nodejs/npm