Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

Homebrew should no longer manage npm. #27479

Closed
wants to merge 7 commits into from
Closed

Homebrew should no longer manage npm. #27479

wants to merge 7 commits into from

Conversation

chevcast
Copy link

This discussion has been ongoing for a while now and it seems to have come to a consensus. When homebrew installs npm it breaks the ability for npm to update itself. This wouldn't be a big deal if we never let npm update itslef, but the reality is that's not really practical.

With npm you can run npm update -g (which is standard practice) and it will update all global node modules, including npm itself. When this happens after homebrew installed npm it completely breaks npm so that it no longer functions at all (unless you use sudo every time, which obviously isn't ideal). The only way to fix it is to re-install npm. Installing this brew formula without npm and then installing npm separately fixes the issue and allows npm to manage itself appropriately.

We came up with two options:

  1. Brew formula should only install node and simply display the curl script to install npm at the end (what this PR does).
  2. Brew should install and manage node and then run the curl script itself at the end.

I'm not familiar enough with homebrew or Ruby to even know if it's possible to run the curl script at the end of the node install so I went ahead and did my best to implement option 1. Please feel free to point out any issues with my changes; as I said, I'm not a Ruby developer, so I wouldn't be surprised if I made a few mistakes.

Option 2 would be the best since it would allow brew to install npm initially, but then let it update itself thereafter, but the original issue is obviously a permissions issue; it seems likely that the same issue would occur with this option. If someone reading this thinks it's possible to do that then please do so; I know that lots of us would be very grateful :)

@MikeMcQuaid
Copy link
Member

I want @isaacs or other input on this before we merge it. We can't keep yo-yoing the Homebrew formula on this.

@chevcast
Copy link
Author

Totally agree. This workaround of not letting homebrew manage npm is actually his workaround. Well, he tells others to completely opt out of homebrew altogether when it comes to node, but later in the thread everyone realized only npm needs to not be managed by homebrew.

end
end
def caveats; <<-end.undent
Homebrew has NOT installed npm. If you want npm (you do right?)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would absolutely rather we provide a link to a webpage that provides these instructions, so the caveats would be "To install npm, read the instructions at http://blah."

@chevcast
Copy link
Author

Makes sense to me. Where would be the best place to host such instructions? Or does one already exist?

I will say that install script has not changed for quite some time. Idk if that makes a difference or not.

@adamv
Copy link
Contributor

adamv commented Mar 12, 2014

Presumably the NPM project has a homepage with these instructions on it. They could even make a Homebrew specific page that we could link to.

@chevcast
Copy link
Author

Well I've searched around and I cannot find one. If you know of one then please let me know. I've seen the one-liner install script all over the place but I can't find any page dedicated to install instructions that includes the easy one-liner. Should I create one? I'd be more than happy to throw up a permanent page on my domain. Though, is this really worth the hassle? What is your reasoning for not liking the one-line npm install just being displayed right in the caveat?

@MikeMcQuaid
Copy link
Member

@adamv @chevex I want this PR to sit open for at least a week before we merge it. We need to take the time to do this properly, finally.

@chevcast
Copy link
Author

That's fine with me. I'm in no way trying to rush anything. Just discussing. I will stay on top of this PR discussion and make any changes in my fork that we decide to make.

@adamv
Copy link
Contributor

adamv commented Mar 12, 2014

My reasoning is that NPM should provide install instructions, and if they change their install instructions we shouldn't have to update the caveat, since we wouldn't actually be packaging NPM anymore.

Surely they have a page with installation instructions somewhere on the Internet.

@chevcast
Copy link
Author

@adamv Surely you can find it and link to it then, because I sure can't.

Since npm is packaged with node I'd bet most people get it that way so maybe they don't feel it necessary to provide an install page for npm since they have one for node. Anytime I've seen npm installed by itself without node it has always been that curl script. They've maintained that shell script since I can remember and even when they've changed and moved stuff around that shell script has always been there. They just keep redirecting that url to wherever it needs to go (hence the -L so that it will follow 301 redirects).

@chevcast
Copy link
Author

This is the best I could find, and it tells users to install node to get npm. There is a "fancy pants" install section below that with the script in it. I suppose it does make sense to just link to the README. Only issue is that it doesn't actually instruct them how to easily DL and run the script; no mention of curl.

What do you think @adamv?

@adamv
Copy link
Contributor

adamv commented Mar 12, 2014

This homepage is indirect as heck but it "works for me" in the sense that I don't use npm myself.

@chevcast
Copy link
Author

I think I'll leave it the way it is for now; either that or submit a PR to npm. I don't like that the README doesn't show any simple way to download and install npm. Even if they think to use curl they then have to figure out that they must supply the -L option to force it to follow the redirects. If they update the README with such information then I'd happily link to it. For now I think it's easier to have this line right in the formula where others can come correct it if it ever really did change (which I doubt anytime soon).

Most people using brew are power users and are going to want the simple one-liner and it really sucks to force everyone to google when we know what the one-liner is. Also, since it's brew, we only care about the unix install and none of the others. I think this makes the most sense for now. But again, updating the npm README probably makes the most sense.

@adamv
Copy link
Contributor

adamv commented Mar 12, 2014

I won't be the maintainer who ultimately pulls this, but I'll leave this as a note to future interested maintainers: I'm strongly opposed to directly including the command-line to install NPM in node's caveats, and would very much prefer a link to npm documentation. Said documentation doesn't seem to exist in a usable state, but perhaps that is a sign to npm to improve it.

This change assumes that the npm PR will be merged.
@chevcast
Copy link
Author

I found the npm README and sent a PR to change it. I then updated this PR with a link to the README. That should do what you want @adamv.

#{npm_prefix}/bin
end
end
def caveats; <<-end.undent
Copy link
Contributor

Choose a reason for hiding this comment

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

Use EOS.undent and put in the ending EOS.

Copy link
Author

Choose a reason for hiding this comment

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

Will do. Thank you :)

lib/"node_modules/npm/lib/utils/completion.sh" => "npm"
end
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

This end needs to remain as the end of def install.

Copy link
Author

Choose a reason for hiding this comment

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

Done. Thanks again.

@isaacs
Copy link
Contributor

isaacs commented Mar 13, 2014

If you're familiar with the history here, I did once upon a time send a pull request to Homebrew that removed npm entirely. After massive push-back on that, I sent a new pull request that installed npm in a more standard fashion, such that it would not disable features or be unable to upgrade itself. After much back and forth with the Homebrew team, we settled on something that we could all be equally dissatisfied with. That was some time ago, and the current state of the npm brew recipe is to be perpetually broken and weird.

So, in short, huge +1 on this from me. Several years overdue.

@MikeMcQuaid
Copy link
Member

@isaacs Ok, thanks. Is this: https://github.com/npm/npm#fancy-install-unix the best link for us? Alternatively, can we rely on https://npmjs.org/install.sh to work for a long time? Thanks again!

@joeyhoer
Copy link
Contributor

One-line npm install: npm_config_prefix=/usr/local/lib/node_modules; curl -sSL https://npmjs.org/install.sh | bash

Edit: On inspection of the PR I see you've already done this, and subsequently removed it.

@chevcast
Copy link
Author

Think this has been open long enough that we can finally get this fixed? :)

@aredridel
Copy link
Contributor

y'know, 👍. It's a nice compromise, and considering how npm works so compatibly with brew if you do it this way, I'm a fan.

@jfelchner
Copy link

@mistydemeo I stand corrected. I guess I'll revise to say "No developer I know of installs Ruby/Rubygems via Homebrew". Everyone I've ever met uses a version manager (eg RVM, rbenv, chruby, etc) to handle that.

@franciscolourenco
Copy link
Contributor

What about a patch in the formula which prevents npm from updating itself?
Then npm can be updated by homebrew like every other formula.

@chevcast
Copy link
Author

How would you do that though? When I run

npm update -g

It updates all global modules, including npm itself.

@jacknagel
Copy link
Contributor

Yeah, I don't think that suggestion would be an improvement for anyone; people want to keep npm up to date (for good reasons).

@isaacs
Copy link
Contributor

isaacs commented Mar 24, 2014

What about a patch in the formula which prevents npm from updating itself?

Please see the long gory history of this formula, and the discussions around it. What you suggest IS the problem.

@MikeMcQuaid
Copy link
Member

I'm going to actively work on this to see if we can find a solution that allows self-update without patching by installing npm in a different/correct place.

alrra added a commit to alrra/dotfiles that referenced this pull request Mar 24, 2014
Installing Node.js and npm using Homebrew can be problematic:
Homebrew/legacy-homebrew#27479
@MikeMcQuaid
Copy link
Member

In #28075 I've got a PR to install npm in a way that allows it to self-update and manage its own symlinks. This appears to be the best scenario and seems to work nicely given my tests. I'd advise people who care here to go test that.

@chevcast chevcast closed this Apr 3, 2014
@chevcast
Copy link
Author

chevcast commented Apr 3, 2014

Closing since #28075 is the solution we all really wanted.

MikeMcQuaid added a commit that referenced this pull request Apr 4, 2014
Install npm to the expected location using the upstream tarball. This
should make everyone happier.

Closes #27479.
@MikeMcQuaid
Copy link
Member

Now fixed in f900829.

ehershey pushed a commit to ehershey/homebrew that referenced this pull request Apr 4, 2014
Install npm to the expected location using the upstream tarball. This
should make everyone happier.

Closes Homebrew#27479.
domenic pushed a commit to npm/npm that referenced this pull request Apr 18, 2014
Added a simple example of installing npm by itself via curl. This helps solve [an issue](Homebrew/legacy-homebrew#27479) with the node homebrew formula where users will want a simple way to install npm after installing node. Homebrew maintainers would rather see this curl example in the npm README rather than directly in the node formula.

It also just helps in general to have an example when users find the npm README and want to use the shell script.
porada added a commit to porada/dotfiles that referenced this pull request Apr 27, 2014
It no longer conflicts with Homebrew (Homebrew/legacy-homebrew#27479).

This reverts commit 5a70ba8.
alrra added a commit to alrra/dotfiles that referenced this pull request Jun 29, 2014
Now that the `npm` related issue (see: Homebrew/legacy-homebrew#27479) has been
fixed (see: Homebrew/legacy-homebrew#28075), revert back to using `Homebrew` to
install `Node.js` and `npm` on OS X.
@DomT4 DomT4 mentioned this pull request Jan 14, 2015
yeled pushed a commit to yeled/homebrew that referenced this pull request Mar 10, 2015
Install npm to the expected location using the upstream tarball. This
should make everyone happier.

Closes Homebrew#27479.
@Homebrew Homebrew locked and limited conversation to collaborators Feb 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.