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

krew: remove due upstream maintainers request #49428

Closed
wants to merge 1 commit into from

Conversation

glensc
Copy link
Contributor

@glensc glensc commented Jan 27, 2020

From kubernetes-sigs/krew#475:

It seems like Krew was added to Homebrew formulae list. and it's currently available to users. Without documenting it, I suspect most people won't install it via brew.

Sadly, Krew is designed to install and update itself, just like any other kubectl plugin. We don't wish to support installation via other package managers as krew is a package manager itself (and while Krew is cross-platform, others aren't).

Homebrew makes sense for binaries like kubectl, but krew has extra installation instructions.

My recommendation would be to get Krew removed from Homebrew in the short term.
Any help is appreciated.

Originally added in #47128, updated to 0.3.3 in #47415.

@ahmetb
Copy link
Contributor

ahmetb commented Jan 27, 2020

I am supportive. Thanks.

@alebcay alebcay added maintainer feedback Additional maintainers' opinions may be needed marked for removal/rejection PR is probably going to be closed or formula deleted labels Jan 27, 2020
@SMillerDev
Copy link
Member

That's not really how the homebrew process works though. If people want to install it through homebrew they can if they do the work, upstream has very little say in that.

@ahmetb
Copy link
Contributor

ahmetb commented Jan 27, 2020

Absolutely, and that's understandable. We might not able to support the case, and we might occasionally break. I suspect Homebrew doesn't distribute other package manager so this is unexplored territory.

@corneliusweig
Copy link

@SMillerDev there are two problems with the formula:

  1. It provides an independent installation of krew, so that krew cannot update itself anymore. I agree that it's the users' choice from where they want to pull their binaries. And in general, homebrew is an excellent choice. Nevertheless, in this special case, installing via krew itself is even better.

  2. The installation via homebrew is incomplete. Krew only becomes usable when the PATH variable gets updated (see https://github.com/kubernetes-sigs/krew/#bash-and-zsh). The current version of the brew formula does not handle this at all, nor does it print a warning message. As a result, krew itself will work, but the kubectl plugins it installs will be broken. Thus, users may think that krew itself is buggy which we'd like to avoid.

@SMillerDev
Copy link
Member

I suspect Homebrew doesn't distribute other package manager so this is unexplored territory.

Not really, homebrew packages a lot of package managers. Npm, gradle, you name it.

@SMillerDev
Copy link
Member

Krew only becomes usable when the PATH variable gets updated.

This is in the upstream docs, which is always the place to follow regardless of the way of installing. Homebrew doesn't print warnings unless there's a homebrew specific caveat that people need to know about.

@ahmetb
Copy link
Contributor

ahmetb commented Jan 27, 2020

In case of NPM, is NPM normally expected to update itself on a non-brew set-up? Or do these package managers explicitly rely on another package manager to be set up?

@corneliusweig
Copy link

corneliusweig commented Jan 27, 2020

This is in the upstream docs, which is always the place to follow regardless of the way of installing.

I have been a brew user for quite a while, when I had a Mac. But I never read any upstream docs after installing a package. I think that the overwhelming majority of users assumes that a package simply works if the install finishes correctly, don't you think?

@SMillerDev
Copy link
Member

In case of NPM, is NPM normally expected to update itself on a non-brew set-up?

It is, but this isn't supported by brew. Same for composer and some others.

@ahmetb
Copy link
Contributor

ahmetb commented Jan 28, 2020

OK, it seems like this works, for now. This is something we'll have to keep in mind, and hope it doesn't break with the assumptions we already make or will make.

We'll try to get post-install message and git dependency added.

@glensc glensc changed the title krew: remove due mantainers request krew: remove due upstream maintainers request Jan 29, 2020
@glensc
Copy link
Contributor Author

glensc commented Jan 29, 2020

@ahmetb

  1. is there specific version of git required to operate? as typically git is installed (needed for brew operation) and rejected to be added to formulas
  2. adding caveat to update PATH is quite common. if you submit the PR, please cross link it here as well. thanks

@ahmetb
Copy link
Contributor

ahmetb commented Jan 29, 2020

@glensc

  1. you're right macOS git would do fine. I don't think we rely on newest features.

  2. I'll make a PR.

We can close this –sorry for bothering you.

@ahmetb
Copy link
Contributor

ahmetb commented Jan 29, 2020

I've opened #49547 to supersede this.

@glensc glensc closed this Jan 30, 2020
@lock lock bot added the outdated PR was locked due to age label Feb 29, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
maintainer feedback Additional maintainers' opinions may be needed marked for removal/rejection PR is probably going to be closed or formula deleted outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants