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

Post-install hook #23

Open
ii14 opened this issue Feb 12, 2022 · 9 comments
Open

Post-install hook #23

ii14 opened this issue Feb 12, 2022 · 9 comments

Comments

@ii14
Copy link
Member

ii14 commented Feb 12, 2022

A somewhat common thing to do is having post-install hooks. Maybe it could be included in the spec?

A couple of ideas for the syntax (not 100% sure about the name make):

make = "./configure && make"

-- multiline string, maybe should work just like makefiles?
make = [[
  ./configure
  make
]]

-- same, but as an array
make = {
  "./configure",
  "make",
}

-- platform specific
make = {
  linux = "./configure && make",
  win = "...",
  ["*"] = "...", -- wildcard for all platforms?
  ["linux,mac"] = "...", -- or just autocmd-like syntax
}

-- maybe vim commands too, but not sure about it being
-- shared as the same field with shell commands
make = ":UpdateRemotePlugins"
@wbthomason
Copy link
Contributor

Nitpick: I'd suggest "build" or "run" over "make", to reflect the more general possibilities for "things I want to do after an install or update".

We might also want to be able to say if a hook should run only after install, after install and update, etc

@mjlbach
Copy link
Contributor

mjlbach commented Feb 13, 2022

build is also what rockspec uses (not that we are tied to it), but I also agree with the naming. Should we specify it by platform? It might get a bit tricky, need to make assumptions if running in powershell, cmd, bash, etc.

@wbthomason
Copy link
Contributor

We could also call it install_hook, update_hook, hook, etc.

@ii14
Copy link
Member Author

ii14 commented Feb 13, 2022

Some sort of cleanup hook could be useful (to uninstall previous version for example), but is there actually any use case for having the update hook (I assume it's after git pull/fetch)?

@ii14
Copy link
Member Author

ii14 commented Feb 13, 2022

hooks = {
  ["linux,mac"] = {
    build = [[...]],
    clean = [[...]],
  },
  ["win"] = {
    build = [[...]],
    clean = [[...]],
  },
}

hooks = {
  build = {
    ["linux,mac"] = [[...]],
    ["win"] = [[...]],
  },
  clean = {
    ["linux,mac"] = [[...]],
    ["win"] = [[...]],
  },
}

@mjlbach
Copy link
Contributor

mjlbach commented Feb 19, 2022

Just a random thought. We could encourage people to use lua/neovim as the environment via which to run these commands:

build = {
  install = "require('lspconfig').install()",
  update = "require('lspconfig').update()"
}

And instead ask users to call the build scripts via lua? Otherwise I'm a bit worried about the complexity of having to deal with powershell vs cmd on windows, fish vs. sh vs. zsh vs bash on unix, etc. Otherwise we are going to have to pick a blessed shell per platform and document how the package manager should deal with this (maybe not a big deal).

@mhanberg
Copy link

One thing to think about is whether this would be opt-in or opt-out.

I think with the current set of package managers, they are all opt-in (do to the nature of things currently) and I think some security focused people might want it to stay opt-in, even at a potential lost in ergonomics.

@ii14
Copy link
Member Author

ii14 commented Mar 29, 2022

@mhanberg I think how is it handled should just be implementation defined, and a package manager could let you audit what it is that it's about to run. And plugins can already screw you up if they wanted to, I don't see any particular reason why some malicious actor would prefer doing something nasty here instead of directly inside the plugin.

@brianhuster
Copy link

vim-addon-manager also uses json to manage dependencies, its "packspec" addon-info.json also specifies hooks

https://github.com/MarcWeber/vim-addon-manager/blob/master/doc/vim-addon-manager-additional-documentation.txt?plain=1#L440

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants