Updating install location for quickstart plugin

[psschwei]

Changes

• 🎁 Update the plugin install location to be ~/.config/kn/plugins so that it will be discoverable by kn

/kind enhancement

Fixes #23

Please double check this... I ran the commands through irb and everything seems to be correct, but I don't have a working homebrew to fully test out this change.

/assign @rhuss

[psschwei]

comment from @julz in slack: seems probably best to fix by respecting PATH so it works for all plugins, not just QuickStart (and so brew install works for all users, not just the one who ran the brew command; also consistent with other plugins with similar model, like git/kubectl etc)

[csantanapr]

I agree with @psschwei and @julz the kn CLI should use the PATH to search for plugin same way as kubectl

I think we should merge this, and once kn is released with the capability to search in PATH then we can make new versions of quickstart plugin install in bin PATH

[maximilien]

@csantanapr kn has and always had the ability to search for plugins in PATH. It's just not the default. Users can enable by setting up that in config.yml options path-lookup

[maximilien]

See comment about kn search for plugins

[maximilien]

We've debated this... on the one hand installing in the normal location by brew would allow easy clean up. But that requires users to specify lookup in path for kn. See my comment above.

Ideally a symlink would be better since that would allow both PATH and lookup-path to work

[julz]

@maximilien do you remember why we didn't/is there any reason we shouldn't make that the default, matching git/kubectl? (otherwise it's not much use because in terms of this use case since we can't really expect users to edit a json file to install a "Quick"Start plugin :))

[maximilien]

Long discussion @julz way back when I submitted first PR for plugins in fall 2018. It actually looked up in PATH by default since I was trying to keep it like kubectl but lots of push backs and reasoning that I actually agree with for a central plugin repo while also allowing PATH lookup. So we ended up where we are.

I suggested a way out to @psschwei that might work. See comment above about symlink.

[psschwei]

Long discussion ...

Right before I saw this, I opened knative/client#1399 about changing the default behavior. If that discussion has already been had, then feel free to close. But if perhaps given that some time has passed, maybe it's worth revisiting?

[julz]

lots of push backs and reasoning

Any chance these are documented anywhere so I could read up? Without wanting to dredge up a solved issue, I would love to understand more because the advantage of working consistently with git/kubectl, and of working out of the box with brew without writing stuff in to a home dir (which will only work for the user that happened to run brew..) seem quite compelling.

[maximilien]

/ok-to-test
/approve
/lgtm

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maximilien, psschwei

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [maximilien]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[maximilien]

I approved this too fast. There are some issues we need to resolve @psschwei. First, the require 'Dir' may not be something we can do in a formulae. When I tested it manually I get:

Error: quickstart: cannot load such file -- Dir
/usr/local/Homebrew/Library/Homebrew/formulary.rb:84:in `rescue in block in load_formula'
/usr/local/Homebrew/Library/Homebrew/formulary.rb:77:in `block in load_formula'
/usr/local/Homebrew/Library/Homebrew/formulary.rb:90:in `load_formula'
/usr/local/Homebrew/Library/Homebrew/formulary.rb:110:in `load_formula_from_path'
/usr/local/Homebrew/Library/Homebrew/formulary.rb:195:in `load_file'
/usr/local/Homebrew/Library/Homebrew/formulary.rb:185:in `klass'
/usr/local/Homebrew/Library/Homebrew/formulary.rb:180:in `get_formula'
/usr/local/Homebrew/Library/Homebrew/formulary.rb:412:in `factory'
/usr/local/Homebrew/Library/Homebrew/formulary.rb:116:in `resolve'
/usr/local/Homebrew/Library/Homebrew/cli/named_args.rb:170:in `resolve_formula'
/usr/local/Homebrew/Library/Homebrew/cli/named_args.rb:106:in `load_formula_or_cask'
/usr/local/Homebrew/Library/Homebrew/cli/named_args.rb:59:in `block in to_formulae_and_casks'
/usr/local/Homebrew/Library/Homebrew/cli/named_args.rb:58:in `each'
/usr/local/Homebrew/Library/Homebrew/cli/named_args.rb:58:in `flat_map'
/usr/local/Homebrew/Library/Homebrew/cli/named_args.rb:58:in `to_formulae_and_casks'
/usr/local/Homebrew/Library/Homebrew/cli/named_args.rb:176:in `to_resolved_formulae'
/usr/local/Homebrew/Library/Homebrew/dev-cmd/test.rb:45:in `test'
/usr/local/Homebrew/Library/Homebrew/brew.rb:131:in `<main>'

[maximilien]

And 'Dir' is usually included in Ruby processes so you don't normally need to require it. I tried to remove it same thing...

I suggest trying to test manually with: brew test --debug --verbose ./quickstart.rb

[maximilien]

Can you revert your changes please... as we work on figuring out a solution. Thx.