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

feat: add bash plugin #856

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: add bash plugin #856

wants to merge 2 commits into from

Conversation

xaf
Copy link

@xaf xaf commented Jul 10, 2023

Summary

Description:

Checklist

  • Format with scripts/format.bash
  • Test locally with scripts/test_plugin.bash --file plugins/<your_new_plugin_name>
  • Your plugin CI tests are green
    • Tip: use the plugin_test action from asdf-actions in your plugin CI

@xaf xaf requested a review from a team as a code owner July 10, 2023 15:55
@xaf xaf changed the title Add bash plugin feat: add bash plugin Jul 10, 2023
@xaf
Copy link
Author

xaf commented Jul 10, 2023

So... this might not work with asdf shims, since it creates a dependency loop.
Would require asdf to avoid using /usr/bin/env bash in the shim (at least the shim for bash) but use the system version instead.

@xaf xaf force-pushed the xaf/add_bash_plugin branch from 59c399e to 8f5bb58 Compare July 10, 2023 16:23
@hyperupcall
Copy link
Contributor

Yeah, some code on the asdf-side might need to be modified for this to work. Changing the shebang, or something like this are possible fixes.

@xaf
Copy link
Author

xaf commented Jul 13, 2023

Yeah, might make a PR for asdf to add a new binary file to use as custom shebang, which can specifically look for bash in the PATH after removing asdf. That would make asdf less dependent on bash from the env itself, while at the same time following the env, which would be a good thing!

@jthegedus
Copy link
Contributor

Ping me when ready to merge (when it is adequately supportable).

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

Successfully merging this pull request may close these issues.

3 participants