Use a git repository for installing plugins. #183

doughsay · 2017-03-26T23:46:23Z

This is my attempt at the proposal I opened: #181

It's using a temporary repository under my account for now: https://github.com/doughsay/asdf-plugins.

Thoughts? Feedback?

danhper · 2017-04-18T14:31:17Z

lib/utils.sh

+
+  if [ -d $repository_path ]; then
+    echo "updating plugin repository..."
+    (cd $repository_path && git pull)


For these this of update I think git fetch && git reset --hard origin/master is more predictable than git pull.

danhper · 2017-04-18T14:31:47Z

lib/utils.sh

+  local repository_url=$(asdf_repository_url)
+  local repository_path=$(asdf_dir)/repository
+
+  if [ -d $repository_path ]; then


This will probably fail if $asdf_dir contains spaces. Adding quotes should do the job.

danhper · 2017-04-18T14:34:30Z

Thank you very much for the PR, and sorry for the late reply.
I think this is very promising, and simple enough to be integrated into asdf.
I added a few comments.
Maybe it would be good if you could run shellcheck on the code, and fix potential warnings.

doughsay · 2017-04-20T03:44:22Z

Thanks for the review, I've addressed the feedback and run shellcheck. There are many warnings produced by code I didn't touch; I've only removed all warnings from the code I added or modified. I'd also like to add tests to make sure this works as expected, but I'm not familiar with bats... How hard is it to test this, given that it relies on git?

danhper · 2017-05-06T10:28:42Z

Sorry for the delay and thanks for updating!
Sure, don't worry about the existing shellcheck warnings.

Our tests are not really pure unit tests, and some already rely on git and even on cloning remote repository, so I think it's fine to do the same thing for the parts of this feature that rely on git.

If you could just at least maybe add a test for the asdf plugin-add PLUGIN command, I think we could maybe merge, and get some feedback before releasing the feature in the next version.

@asdf-vm/maintainers Thoughts?

vic · 2017-07-23T11:35:33Z

Hm, interesting idea. Yes, please add some tests for this :), I like the idea in general, so I guess the list of available plugins would be inside the main asdf repo ? or shall we have another repo for that?.

On my recent dockerized asdf project asdf-alpine I'm holding the plugin url in a simple file like this that gets read by a tiny script.

doughsay · 2017-07-23T16:09:25Z

@vic The idea would be to create a separate repo to list all available "official" plugins. Contributors could open PRs to that repo to add new plugins. Anyone wanting to add a not-yet-listed plugin could still use the asdf plugin-add command with a URL. This PR relies on a repo that I created under my name to show the proof of concept, but I'd want an asdf maintainer to create asdf-vm/asdf-plugins before releasing this. I haven't gotten to adding the tests because I've never dealt with bats before... I'll take a stab at it this morning to see if it's not too hard.

doughsay · 2017-07-23T16:37:12Z

@vic @tuvistavie added a simple test for plugin-add with no URL provided. Is that good enough? It does rely on actually cloning the plugins repo and installing a real plugin, rather than a dummy plugin like the other tests.

vic · 2017-07-24T14:44:59Z

@doughsay nice! Code looks ok :) no problem on actually clonning the asdf-repository and the real elixir plugin afterwards.

I'm 👍 on this. Would you mind adding the current list of repo urls to the asdf-repository? Also please update the README.md file to mention the repository that the plugin-url is now optional. And perhaps also at creating-plugins.md as most plugin authors will need to read that.

We could move your repository to the asdf-vm org just before releasing this.

vic · 2017-07-24T14:47:12Z

Oh forgot to mention, just add another test for the case where I dont provide the url and the plugin is not registered on the repository. It should just show the error message and exit 1. :)

vic · 2017-07-24T17:12:08Z

Just wondering, once this gets merged, would it be possible to tell people, to just install asdf and afterwards go directly with asdf list-all elixir or even to asdf install elixir 1.4.5 without having to be explicit about plugin-add elixir ? I mean, once we have the plugins repo, plugin-add is looks to me like only really useful for non-official plugins.

doughsay · 2017-07-26T00:21:28Z

@vic I've added all the current plugins to the plugins repo here: https://github.com/doughsay/asdf-plugins. (I left out the link plugin though, since I see it's not really meant to be added on its own.)

I've also added the extra test and documentation updates you suggested.

As for your other comments, it does make sense that all plugin commands could automatically attempt to add a plugin if the plugin doesn't yet exist. Consider typos though, would you want asdf install elxr 1.4.5 to update the plugins repo and check if there's an elixr plugin before erroring out? It's pretty fast, so I don't think it would be a big deal; just something to consider.

vic · 2017-07-26T03:06:13Z

@doughsay awesome! Thank you very much. I guess this PR is close to ready.

The plugins repo looks nice, no problem on leaving link out.

I'd say if only the asdf install command was aware of the plugins-repo it'd be an awesome first-time experience that just after downloading asdf, to be able to install any officially supported tool without further ado.

So I guess it's up to you, if you'd like to implement this, for asdf install foo 1.0 checkout the plugins-repo and look for foo in it. Maybe we could work on this in another Issue/PR. Also other thing we might consider for other PR is being able to list all known tools from the plugin-repo, perhaps asdf plugin-list --remote

Anyways, work on this branch looks like done to me, I've downloaded @doughsay asdf to give it a try and the new plugin-add feature works fine.

@doughsay would you mind transferring the asdf-plugins repo to the asdf org? and then just update the url in the code? I guess that would be all for being able to merge this branch. Thank you :)

@tuvistavie @asdf-vm/maintainers anything I might be missing?

HashNuke · 2017-07-26T03:48:04Z

People, this is awesome work ~! ❤️ 💚 💙

doughsay · 2017-07-26T04:06:44Z

Thanks a lot everyone, for making this awesome tool and for letting me contribute. It's been fun!

@vic I tried transferring the ownership of the repository but apparently it won't let me unless I have permission to create new repositories under the organization. If you'd like, you could just create a new empty repository and I'll copy-paste everything I've done into a PR for the new repo.

Also, I like your ideas for the asdf install and asdf plugin-list --remote commands; I think we should finish this PR as is, but I would be happy to try implementing those ideas next.

As soon as a new repo exists where I can upload the plugin configs, I'll change the URL in this PR.

Thanks!

doughsay · 2017-07-26T04:08:23Z

Ah, I read the fine print a bit more carefully: "Transfer this repository to another user or to an organization where you have the ability to create repositories."

That means I could transfer the repo to a user easily, just not an org. Would you like me to transfer it to you @vic, and then you can transfer to the org?

vic · 2017-07-26T04:43:59Z

@doughsay here's the new repo :) https://github.com/asdf-vm/asdf-plugins

danhper · 2017-07-26T15:41:42Z

Thanks for working on this, it looks great and will be super useful as we are having more and more plugins.
I think we can merge and start to try it out.
@vic @doughsay I agree it would be nice if we could check if there is an available plugin when we do not find it locally. asdf plugin-list --remote will be useful too. I will open a new issue for both of these features.

doughsay · 2017-07-26T16:23:19Z

URL has been updated, should be ready to go! 👍

danhper · 2017-07-26T16:33:04Z

Great, thank you very much!!
Let's give it a try 😄

I opened two new tickets for the new features discussed here:
#210
#211

Stratus3D

Looks good! Left comments on a few minor things.

Stratus3D · 2017-09-07T01:05:54Z

help.txt

@@ -1,5 +1,5 @@
 MANAGE PLUGINS
-  asdf plugin-add <name> <git-url>     Add git repo as plugin
+  asdf plugin-add <name> [<git-url>]   Add a plugin


I think the help message here should be a little more detailed. Perhaps something like Add a plugin from the plugin repo OR add a Git repo as a plugin by specifying name and repo url?

Stratus3D · 2017-09-07T01:10:55Z

lib/utils.sh

+
+asdf_repository_url() {
+  echo "https://github.com/asdf-vm/asdf-plugins.git"
+}


Maybe move this to the top of the file under the asdf_version function since it's basically a constant.

Stratus3D · 2017-09-07T01:38:43Z

No need to address those two comments I left during the review. I went and updated master with those changes. Thanks @doughsay !

Use a git repository for installing plugins.

20cdbfe

danhper reviewed Apr 18, 2017

View reviewed changes

doughsay added 3 commits April 19, 2017 20:26

code review feedback: git reset instead of git pull

96c24e9

code review feedback: fix all shellcheck warnings.

f399254

a little too zealous on the shellcheck warnings.

5255f0e

Add simple test for plugin-add command.

a4fed4e

doughsay added 2 commits July 25, 2017 17:03

Add test for bad plugin name case.

23f767c

Update README and creating-pugins doc to reflect new plugin-add command.

1bdaf22

Update plugins repo url.

a0e5573

danhper merged commit 78fbbaf into asdf-vm:master Jul 26, 2017

Stratus3D reviewed Sep 7, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a git repository for installing plugins. #183

Use a git repository for installing plugins. #183

doughsay commented Mar 26, 2017

danhper Apr 18, 2017

danhper Apr 18, 2017 •

edited

Loading

danhper commented Apr 18, 2017

doughsay commented Apr 20, 2017

danhper commented May 6, 2017 •

edited

Loading

vic commented Jul 23, 2017 •

edited

Loading

doughsay commented Jul 23, 2017

doughsay commented Jul 23, 2017

vic commented Jul 24, 2017

vic commented Jul 24, 2017

vic commented Jul 24, 2017

doughsay commented Jul 26, 2017

vic commented Jul 26, 2017

HashNuke commented Jul 26, 2017

doughsay commented Jul 26, 2017

doughsay commented Jul 26, 2017

vic commented Jul 26, 2017

danhper commented Jul 26, 2017

doughsay commented Jul 26, 2017

danhper commented Jul 26, 2017

Stratus3D left a comment

Stratus3D Sep 7, 2017

Stratus3D Sep 7, 2017

Stratus3D commented Sep 7, 2017

Use a git repository for installing plugins. #183

Use a git repository for installing plugins. #183

Conversation

doughsay commented Mar 26, 2017

danhper Apr 18, 2017

Choose a reason for hiding this comment

danhper Apr 18, 2017 • edited Loading

Choose a reason for hiding this comment

danhper commented Apr 18, 2017

doughsay commented Apr 20, 2017

danhper commented May 6, 2017 • edited Loading

vic commented Jul 23, 2017 • edited Loading

doughsay commented Jul 23, 2017

doughsay commented Jul 23, 2017

vic commented Jul 24, 2017

vic commented Jul 24, 2017

vic commented Jul 24, 2017

doughsay commented Jul 26, 2017

vic commented Jul 26, 2017

HashNuke commented Jul 26, 2017

doughsay commented Jul 26, 2017

doughsay commented Jul 26, 2017

vic commented Jul 26, 2017

danhper commented Jul 26, 2017

doughsay commented Jul 26, 2017

danhper commented Jul 26, 2017

Stratus3D left a comment

Choose a reason for hiding this comment

Stratus3D Sep 7, 2017

Choose a reason for hiding this comment

Stratus3D Sep 7, 2017

Choose a reason for hiding this comment

Stratus3D commented Sep 7, 2017

danhper Apr 18, 2017 •

edited

Loading

danhper commented May 6, 2017 •

edited

Loading

vic commented Jul 23, 2017 •

edited

Loading