-
Notifications
You must be signed in to change notification settings - Fork 811
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
System package dependencies #196
Comments
👍 I like the idea! It's awesome! Better than implementing hand check over |
Here's a proof-of-concept that I have working: https://github.com/coderstephen/oh-my-fish/tree/feature/available-dependencies |
Haven't tested it but it looks good at first glance. Say, does |
@oranja No, and it isn't meant to. We already have a
in its bundle file. OMF can't install OpenSSL, but we can at least warn the user that OpenSSL should be installed first before the package will work. |
Hey @CoderStephen I think It might be better to add support for this inside a package.json-similar way. What do you think? We could start a discussion around this configuration file. |
Yes. I agree with $ cat package
name: balias
type: theme
description: a description override (default is from github repo) Dependencies are already handled in |
I'm not sure if we should keep both bundle and package file. Shouldn't we merge them? |
I also don't see any reason to keep more than one file for package metadata. |
Ok, so what about this:
|
👍 👍 |
I have the following understanding
And I also think about
In this understanding, I don't thing we need |
Is there any benefit on us supporting command dependencies? Not having it doesn't seem like a problem to be handled by the package dependency. It seems to me that we only need to care at a metadata level about other plugin dependencies. And each plugin handles their dependency however the like to. When I am using bundler to install gems, the only thing it handles for me is the dependent gems installation and nothing else. |
A dev/maintainer should be aware and should notify others if a package requires special tools. I am still going back and forth trying to convince myself if omf should encourage packages to specify different kinds of dependencies separately, or leave it as an option to when it's really needed (like Also, I'm not familiar with what is "HEAD's |
We end up with a extremely complex framework. If you install rvm plugin and don't have rvm installed. The rvm plugin should handle it. Its easier to think if the plugin was for ruby and any ruby manager could be used so it would automatically fallback to rbenv/chruby. |
Regarding built-in dependencies I don't think we should support it either. Basically they come with fish versions, we either support a specific version of fish or we don't. Same could be applied to plugins, though this fish version dependency for plugins could come in the future if ever necessary. |
Perhaps we should add an install trigger mechanism then for plugins? Let's say plugin X depends on rvm. Installing X would trigger the installation of the rvm plugin. But if rvm isn't installed, the plugin should reject its own installation in its installation trigger, which in turn should reject installation of plugin X since its dependencies cannot be satisfied. |
It kind of bugs me that eventually these plugins will have to "reinvent" the same procedure over and over again, when it can be centralized in omf's package management, but the final score says "go lean!". So I agree with @CoderStephen's suggested flow. |
I was talking to @derekstavis and we came up with the same idea with a "install" event, similar to "uninstall" and "init". However, as you have expressed it might be an easy win to support basic dependencies (checked using the available function") on the metadata. |
Yeah. Let's keep the idea of a centralised system for checking command availability. It doesn't cost too much on framework complexity and give a DRY treatment to packages. |
@CoderStephen : there are multiple ways to install rvm though. I wouldn't want it to attempt to install rvm via say yum, when I prefer the version from source. |
composer calls these kinds of dependencies platform-dependencies and has a flag to disable resolution of platform dependencies in case you plan on installing it after the fact or isn't needed on the production server (a dev env dep). Obviously the 2nd use case isn't that import antto us, but the first is. It's especially important if you install your stuff from a dotfiles repo before you might have the requisite packages installed. |
@jrobeson I think @CoderStephen only meant that OMF plugins would be installed automatically. For platform-dependencies (I think it's a good name too), either the plugin or OMF should check for availability, but not auto-install. Adding a helpful tip on how to install is as far as I would go for platform-deps. |
Correct. It isn't up to OMF to install platform packages for you, but to prevent redundant code in every plugin, OMF should warn and/or prevent installation of a plugin that depends on a platform package to be installed. |
This is now better solved via the new hooks system introduced by #286 -- packages can simply check for environment and system expectations it may have in an install hook. |
Now that we have package dependencies, we should add a feature to check for system dependencies as well for packages. For example, the weather plugin depends on the shark plugin, but it also depends on
jq
being installed. I propose enhancing the bundle file to iterate overavailable
requirements as well, which would be verified by using theavailable
function.Package bundle files could now look like this:
If one or more
available
dependencies are not satisfied, the package should not be installed and the missing dependencies should be listed. We shouldn't try to install them ourselves.We'll have to go through the whole file first to test the
available
lines first, since we shouldn't install any dependent OMF packages either if the dependencies aren't satisfied.Thoughts?
The text was updated successfully, but these errors were encountered: