-
Notifications
You must be signed in to change notification settings - Fork 77
feat: Add support to install dpk-deps #128
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
Conversation
This allows basher-installable packages to setup required linux tools. Signed-off-by: ChetaN KS <chetan.kumarsanga@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for installing Debian package dependencies defined in the DPKG_DEPS environment variable by splitting on : and invoking apt install for each.
- Parses
DPKG_DEPSand installs each listed package viasudo apt install -y. - Injects this new block immediately after the existing
basher-installloop.
Comments suppressed due to low confidence (1)
libexec/basher-_deps:34
- The new block handling
DPKG_DEPSdoesn’t appear to have accompanying bats tests. Please add tests covering cases with multiple dependencies, emptyDPKG_DEPS, and error scenarios.
IFS=: read -ra deps <<< "$DPKG_DEPS"
|
This wouldn't work on a Mac. Also, where is DPKG_DEPS coming from? |
Signed-off-by: ChetaN KS <chetan.kumarsanga@gmail.com>
Thanks. Updated to select brew/apt for installation based on os type. And DPKG_DEPS comes from package.sh, with syntax similar to DEPS. |
|
Thanks for the clarification. I find this feature difficult to support. It means I could install something on my machine as a side effect of installing something with basher, which to me is unexpected. Also, even if we somehow designed a way to ask for user confirmation, we'd still have to account to all possible Linux distributions. I use nixos, for example, and packages are not installed with commands. |
|
I'm not in favor of this feature. IMO the right way to integrate with brew/apt would be to package the Basher apps you're interested in as such (apt packages, brew packages). This way you'll be able to declare their dependencies using the same system. Just like pip, npm, go get, etc. don't allow specifying system deps, I don't think Basher should. |
|
@chetanc10 Do you have a specific repo in mind, so we can try to solve the problem in a different way? |
|
With your insights, I think I hasted to change basher's behavior. Most of my scripts need to install tools that are not present in the system by default. And I'm only a Linux user, and I didn't think of all parameters when I did this. I'll still make my packages basher-installable without changing basher. But I'll update my packages to install the required dependencies - using similar approach as in basher package deps. BTW @juanibiapina basher is just awesome! |
This allows basher-installable packages to setup required linux tools.
Did bats test as per Contributing.md guidelines.
My 1st pull request. Correct me if anything's missing.