-
-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
asdf: install into libexec and bin.write_exec_script to opt_libexec/bin/asdf #73173
Conversation
Install a libexec under bin to maintain relative paths for sourcing scripts. This will target `bin/asdf` that will source other shell scripts, like `source "$(dirname "$(dirname "$0")")/lib/utils.bash"` Probably need approval and eyes from maintainer(s) Fix: asdf-vm/asdf#891
Remove dependencies unrelated to this formula. It doesn't fall on this formula to require those since it works without them.
No longer conflicting with `homeshick` as we're not polluting `/usr/local/lib` with commands or whatever it claimed.
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.
@seivan A few questions:
- what ramifications will this change have for someone updating via
brew
? - will this cleanup existing location of resources, installed plugins, shims etc? - will users need to reinstall everything managed by their current
asdf
setup?
Please define the migration path before making this change.
Users who are sourcing files in their Shell configurations will presumably need to update those as you comment here asdf-vm/asdf#607 (comment) ? Won't this break people's systems?
asdf uses both the presence of a file OR git to check if updates are possible or not. No need to add a file, since git will fail on the fact that it's not a repo. Fix: Homebrew#73173 (comment)
Good question, @jthegedus If you look closely I have bumped the revision here https://github.com/Homebrew/homebrew-core/pull/73173/files#diff-298ebac561811ce26c00772e4542290af157aebc91a0e05fb9270ea3e5fbbb2eR7 First of lets talk about how Homebrew does file organization for packages and updates. Which means that the new path will be So lets take shims for a second. So my current shims are defined as #!/usr/bin/env bash
# asdf-plugin: elixir 1.11.3
exec /usr/local/Cellar/asdf/0.8.0_1/libexec/bin/asdf exec "iex" "$@" But with this update, they need to be #!/usr/bin/env bash
# asdf-plugin: elixir 1.11.3
exec /usr/local/Cellar/asdf/0.8.0_2/libexec/bin/asdf exec "iex" "$@" Again this is just how Homebrew does updates. So you need to delete shims and call So we got two choices here, A) This pull request on asdf gets accepted where def post_install
system bin/"asdf", "reshim"
end The benefits for this approach is that we always call reshim on asdf updates and those might have new changes necessary, which is a good idea. B) The alternative is that we remove the cellar version That means the shims would be defined as #!/usr/bin/env bash
# asdf-plugin: elixir 1.11.3
exec /usr/local/opt/asdf/libexec/bin/asdf exec "iex" "$@" I am not sure what approach is better. If demanding reshims on asdf updates is considered bad, then the second option is a good choice. But I personally think that a new version of asdfs warrants a reshim just to keep things maintained. But for now I've decide to use
Simple answer: no, nothing needs to be reinstalled. The migration itself is simple, but for a verbose explanation of my testing, these are the steps I did for finding out what needs to be changed.
Migration
To expand on step 2. So normally if you'd want to run Ruby, you'd do These changes make asdf itself work out of the box even if you don't setup the shims in If your shims are in the default directory ( However, asdf lets you designate a custom directory, so it's basically just more of the same inside your env profile, e.g export ASDF_DATA_DIR="$HOME/.config/versioned_tools/asdf"
export PATH="$ASDF_DATA_DIR/shims:$PATH" This is actual configuration that I can get behind and mimics other shell configurations most people got.
Correct, but that's because current installation of asdf via Homebrew is in my opinion faulty and so far the suggestions have been work arounds by requiring people to source a file when it should just read ENV variables defined at your shell environment, e.g |
@jthegedus I've added support for making the migration a little bit less painful here - this would work as post install on Homebrew, but I could use some context on why it was just comparing meta-data before rewriting shims. |
I've renamed the title of your PR to conform to homebrew/core style [1], but feel free to edit it if it doesn't reflect the changes you're proposing. [1] We put the name of the formula first in commit-message headings. Refer to the documentation for more details. |
This is better, sorry I should have done better than the default one. Thanks! |
Switch to `opt_libexec` which translates to `opt_prefix/"libexec`, this means that there will be no migration steps for shims. However till adding a `$ asdf reshim` `post_install` hook incase there is an update that needs to be done on the shims when `asdf` itself updates to a new version or formula revision. There are changes pending on [`asdf-vm`](asdf-vm/asdf#893) that would allow regenerating the shims in case they need to be.
Switch to |
Co-authored-by: Carlo Cabrera <[email protected]>
Fix lint issues.
Adding adding "asdf_updates_disabled" file to libexec, to disallow updates since it might be a git repo on m1/arm. Fix: https://github.com/Homebrew/homebrew-core/pull/73173/files/b517eb00222262961583460e96f3379bb01c8d57#r594787883
Add `git` and `coreutils` as dependencies because `asdf` needs to be written as cross platform this will help maintainers not to worry about platforms.
Fix CI lint issues.
More linting crap.
Both should be correct. The implementation uses environment variables to restrict change to lifespan of that single session. The longer living scopes I believe this was adopted from
Yeah, since So, to get feature parity, the original: source "$(brew --prefix asdf)/asdf.sh" will need to become either:
EDIT: Not too sure of Homebrew's stance on this, but another alternative for backwards-compatible migration would be to consider additional wrapper scripts. It really depends on if a cleaner install is better vs. lower user impact (or preventing future asdf/brew GitHub issues due to the fact that most people would never see the documentation in this PR and just find asdf not working after Maybe something like: (prefix/"asdf.sh").write "source #{opt_libexec}/asdf.sh\n"
(prefix/"asdf.fish").write "source #{opt_libexec}/asdf.fish\n" |
Ah, thanks for explaining that!
Yeah, I actually think this is the best approach. I personally will only do export PATH="$HOME/.asdf/shims:$PATH" in my Because I don't need But this is something you can explain inside
No, please don't do this. Don't source code automatically for people, it's already bad that enough that they modify my I personally don't need asdf_cmd() {
local ASDF_CMD_FILE args_offset
if [ "shell" == "$1" ]; then
echo "Shell integration is not enabled. Please ensure you source asdf in your shell setup." >&2
exit 1
fi Have that explain how to set up |
This doesn't source code automatically for anyone. One still needs to source |
Ah you are correct. I'll leave that up to you because I don't really care about Everything else is secondary, you guys come to an agreement on what you want and make a suggestion. |
@seivan @carlocab I'll add a review comment so that further discussion can be contained there. I did apply the PR Patch File along with these extra writes, and I think the result was a no-impact upgrade where all asdf commands, installs, and shims kept working. I also did install a version with only the PR changes. In my case, I usually contain all source/eval in If we decide not to do this, then part of migration would probably end up passed to @jthegedus and others on ASDF side since some Homebrew users will end up going to that repo first. ASDF will need to have corresponding documentation ready for when the PR gets merged to avoid unnecessary issues opened. EDIT: I'll note that outside of these last migration/documentation concerns, I think everything else in PR looks good to go based on my perusal and some basic local install tests. |
end | ||
|
||
def post_install | ||
system bin/"asdf", "reshim" | ||
end | ||
|
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.
Another optional change mainly for documentation/migration.
May require further editing depending on recommendations from Homebrew maintainers on best way to present caveats and depending on decision for the previous optional change.
Caveat text based on autojump.rb and chruby.rb
def caveats | |
<<~EOS | |
Add the following line to your ~/.bash_profile or ~/.zshrc file: | |
source #{opt_libexec}/asdf.sh | |
If you use Fish shell then add the following line to your ~/.config/fish/config.fish: | |
source #{opt_libexec}/asdf.fish | |
Restart your terminal for the settings to take effect. | |
EOS | |
end | |
For users who just want all of asdf working after install, I think this is the best option.
Since asdf documentation recommends sourcing this for all users (Linux/macOS and git/homebrew), I think this should be default recommendation.
For users who want to do the minimal changes like only exporting shim paths, I think this should be left up to the users to understand how asdf.sh works and apply the correct exports themselves. This avoids asdf issues created due to doing something not documented.
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.
@cho-m
Again, wouldn't that modify your $PATH
and also add the asdf binary again which should already be there?
In my opinion both asdf.fish
and asdf.sh
should be ignored by Homebrew users and they should go straight to the actual files
(using .
instead of source
)
source #{opt_libexec}/lib/asdf.sh
That way $ asdf shell
works and no one messes with your $PATH
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.
@seivan
The goal is low-impact migration and low-impact maintenance.
In order to get the same behavior as before, Homebrew users would have to do at least an export
(shim path) + source
(shell function). Neither of these steps are currently documented by ASDF.
There is also the potential impact of missing export ASDF_DIR
. Some plugins like asdf-nodejs directly use this variable. If missing this causes any breakage, then Homebrew users would have to now do 2 export
+ 1 source
for feature parity.
Additionally, if ASDF does a change to their shell scripts, Homebrew users should not start raising ASDF issues due to following a set of instructions that were not directly recommended by ASDF. If a user wants to clean up their environment variables / PATH, they should do so with full understanding that this is not directly supported yet and any problems they encounter may be due to their own changes.
I personally think it is better to perform a migration in small stages:
- Update Homebrew formula + show caveat without any noticeable change to existing users
- Update ASDF documentation to show new location to source.
- Consider updating ASDF shell scripts to avoid adding
$ASDF_DIR/bin
to path for Homebrew users - Consider adding new documentation/support for ASDF feature specific enablement, i.e.
- describe manually add
${ASDF_DATA_DIR:-$HOME/.asdf}/shims
to PATH for base features - sourcing
lib/asdf.sh
for shell feature.
- describe manually add
- After some time and on a new ASDF semantic version, Homebrew can consider dropping support for previous
(prefix/"asdf.sh")
. Since we are really only doing a patch update (0.8.0_1 -> 0.8.0_2), it is best to have no noticeable impact.
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.
The goal is low-impact migration and low-impact maintenance.
I am with you in trying to make as little manual intervention as possible for migrating, but not at the cost of fixing the issue now or messing things up further to prolong the fix.
As I've stated earlier, as long as $ asdf
and $ asdf help
is broken on install everything else is moot.
The current setup is already broken and requires manual intervention.
Current users has most likely "mitigated" that and will require manual intervention on upgrade, there is no way around these two things.
It's better to fix it now than to prolong it and your steps are flawed, I'll go through each manually.
There is also the potential impact of missing export ASDF_DIR. Some plugins like asdf-nodejs directly use this variable. If missing this causes any breakage, then Homebrew users would have to now do 2 export + 1 source for feature parity.
Incorrect on two parts.
If someone has customized the ASDF_DATA_DIR
they have also exported that variable which asdf_data_dir()
function reads and it will work as intended.
If they have not, it will default to $HOME/.asdf
and it will work as intended.
That nodejs plugin is confusing $ASD_DIR
with $ASDF_DATA_DIR
.
I have not, and it worked as intended.
Additionally, if ASDF does a change to their shell scripts, Homebrew users should not start raising ASDF issues due to following a set of instructions that were not directly recommended by ASDF.
Which instructions are you referring to?
If a user wants to clean up their environment variables / PATH, they should do so with full understanding that this is not directly supported yet and any problems they encounter may be due to their own changes.
What changes are these?
In general, I agree with you previous statement about the caveat, you just got the paths wrong
If they need asdf shell
then explain to them that they should source
. #{opt_libexec}/lib/asdf.sh
and to add executables to their $PATH
the need to set this in their .profile
or .zshenv
export PATH="${ASDF_DATA_DIR:-$HOME/.asdf}/shims"
That should be the caveat, that's a complete setup for bash/zsh, and something different for Fish I'd presume.
I personally think it is better to perform a migration in small stages:
Hard disagree in this case, as the setup is broken. Any other case, I'd agree with you.
- Update Homebrew formula + show caveat without any noticeable change to existing users
- Update ASDF documentation to show new location to source.
Depends on what update, and if the caveat refers to something that will be inadequate in the future.
Then you're just prolonging it
- Consider updating ASDF shell scripts to avoid adding $ASDF_DIR/bin to path for Homebrew users
Consider adding new documentation/support for ASDF feature specific enablement, i.e.
describe manually add ${ASDF_DATA_DIR:-$HOME/.asdf}/shims to PATH for base features
sourcing lib/asdf.sh for shell feature.
Yeah, again, hard disagree.
After some time and on a new ASDF semantic version, Homebrew can consider dropping support for previous (prefix/"asdf.sh"). Since we are really only doing a patch update (0.8.0_1 -> 0.8.0_2), it is best to have no noticeable impact.
You should do this now, because you're just prolonging the issues and the mess continues.
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.
If it was unclear:
I think your code suggestions are solid for making it work for existing users migrating - assuming you have tested an existing setup with these added later.
(prefix/"asdf.sh").write "source #{opt_libexec}/asdf.sh\n"
(prefix/"asdf.fish").write "source #{opt_libexec}/asdf.fish\n"
I just disagree with the caveats, that's all. They should not explain the "old" approach.
Correct me if I am wrong, but with your changes suggested:
- A reshim is not necessary
- It will work out of the box for current users that have used the suggested steps from asdf docs.
Are both points correct?
That being said, I am surprised that existing shims would work.
My shims are written as
exec /usr/local/opt/asdf/libexec/bin/asdf exec "iex" "$@"
I don't think adding
(prefix/"asdf.sh").write "source #{opt_libexec}/asdf.sh\n"
Will fix that, it just makes existing settings that source that file to still work, which is fine on its own, but doesn't serve a purpose if a user has to reshim. If they're doing that manual intervention, might as will do a full on migration.
Again, I could be wrong, if you could confirm that with those changes added your shim paths to old executable would still work, then it's game.
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.
For users who want to do the minimal changes like only exporting shim paths, I think this should be left up to the users to understand how asdf.sh works and apply the correct exports themselves. This avoids asdf issues created due to doing something not documented.
Maybe, however I don't agree but then again these are more opinions than facts. Let me make my case, and if you still dislike it, I'll revert.
First off, I don't want stuff added to my $PATH
, I know macOS does this, but that's the OS, it gets a pass.
I think it's better for asdf
has this in their documentation as this is where you're adding all the compilers, interpreters and other binaries, it's more explicit in what's going on.
For this reason, I think exporting shims in path should be very explicit.
Then there is the issue with a specific feature of asdf that requires sourcing. I see this as a separate thing and not related to exporting shims to $PATH
What I'm more indifferent about is if the caveat should be explicit about why you're sourcing a file.
I've added this
To support package version per session using asdf shell <name> <version>
Add the following line to your #{shell_profile} file:
. #{opt_libexec}/lib/asdf.sh
If you use Fish shell then add the following line to your ~/.config/fish/config.fish:
source #{opt_libexec}/lib/asdf.fish
Restart your terminal for the settings to take effect.
So the choices for caveats as I see is
- Source the script that modifies path and adds shell function (Your suggestion)
- Source the script that adds shell function, keep $PATH explicit (Mix of both ours)
- Keep $PATH explicit, mention optional sourcing of shell function (Currently in place)
@cho-m So, how do you feel about option 3?
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.
I think this question should be redirected to Homebrew maintainers (who may have explicit rules on what is allowed in caveats) and ASDF maintainers (who may have opinions on recommended installation steps).
I am neither, so I want to avoid suggesting something that doesn't line up with both projects' ideology.
Hopefully one of the Homebrew members already subscribed to this PR can respond.
They should know better on how the general user-base deals with caveat info.
I see you also have discussion on ASDF asdf-vm/asdf#891 so maybe see if there are any opinions are on that side. The decision here may require changes for ASDF's own documentation.
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.
@cho-m Fair enough, both @carlocab and @jthegedus wanna chime in here?
Edit: Added suggestions on how to change docs in asdf-vm/asdf#898
This should technically make it work for existing users who are migrating. Per Homebrew#73173 (review) Co-authored-by: Michael Cho <[email protected]>
Add caveat for migrating.
Add caveat for new settings.
Fix trailing whitespace for CI.
Formula/asdf.rb
Outdated
(prefix/"asdf.sh").write ". #{opt_libexec}/asdf.sh\n" | ||
(prefix/"asdf.fish").write ". #{opt_libexec}/asdf.fish\n" |
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.
@cho-m So I tested, and these two lines will make sure that currently documented (on asdf website) "caveat" works for sourcing.
But I don't see how that prevents the need for a rm -rf ~/.asdf/shims; asdf reshim
- can you confirm that old shims will point to the correct binary?
My shims are defined as
cat .asdf/shims/iex
#!/usr/bin/env bash
# asdf-plugin: elixir 1.11.3
exec /usr/local/opt/asdf/libexec/bin/asdf exec "iex" "$@"
The new executable asdf will no longer be versioned and instead always be the active one. I have opinions on this, but for now this will be the best approach until $ asdf reshim
gets some fixes to it.
But /usr/local/opt/asdf/libexec/bin/asd
is not the same as the old path was. Which makes me think that the purposes of these two lines are negated since a migration step is necessary, at least for removing and reshimming.
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.
Using ASDF's documented installation steps with currently available Formula, my shims appear to be using $HOMEBREW_PREFIX/opt/asdf/bin/asdf
. Pretty much multiple rounds of redirection. Forcing a reshim can help remove one exec, but it at least works as is.
I'm on M1 mac so my exact path is different: /opt/homebrew/opt/asdf/bin/asdf
.
Also, in my example, I'm using Unix/Linux-style XDG paths.
After installing patch:
❯ brew info asdf | grep Cellar
/opt/homebrew/Cellar/asdf/0.8.0_2 (119 files, 244.0KB) *
❯ asdf current python
python 3.9.2 system /Users/cho-m/.config/asdf/tool-versions
❯ python --version
Python 3.9.2
❯ pip --version
pip 21.0.1 from /Users/cho-m/.local/share/asdf/installs/python/3.9.2/lib/python3.9/site-packages/pip (python 3.9)
❯ python
Python 3.9.2 (default, Feb 21 2021, 22:26:49)
[Clang 12.0.0 (clang-1200.0.32.29)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import toml
>>> toml.__file__
'/Users/cho-m/.local/share/asdf/installs/python/3.9.2/lib/python3.9/site-packages/toml/__init__.py'
❯ asdf current ruby
ruby 3.0.0 /Users/cho-m/.config/asdf/tool-versions
❯ ruby --version
ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [arm64-darwin20]
❯ head -n3 "$ASDF_DATA_DIR/shims/ruby"
#!/usr/bin/env bash
# asdf-plugin: ruby 3.0.0
exec /opt/homebrew/opt/asdf/bin/asdf exec "ruby" "$@"
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.
Oh my God, this is amazing news.
@jthegedus This should be of interest to you.
Just to see if I understood it...
Current shims are linked to opt/asdf/bin/asdf
directly!
My PR change adds a new opt/asdf/bin/asdf
that points to opt/asdf/libexec/bin/asdf
via
bin.write_exec_script (opt_libexec/"bin/asdf")
So new shims will get less indirection, but older shims still work just fine.
I didn't actually expect that to migrate so well, this is amazing news.
This combined with your added changes to create the files current users are currently sourcing
(prefix/"asdf.sh").write ". #{opt_libexec}/asdf.sh\n"
Makes this a no change migration!
Correct?
All that's left is discussing the caveat.
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.
Yes, that is what I see on my machine.
Should be a seamless migration for existing users.
Formula/asdf.rb
Outdated
touch libexec/"asdf_updates_disabled" | ||
bin.write_exec_script (opt_libexec/"bin/asdf") | ||
(prefix/"asdf.sh").write ". #{opt_libexec}/asdf.sh\n" | ||
(prefix/"asdf.fish").write ". #{opt_libexec}/asdf.fish\n" |
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.
Fish is not a POSIX shell and has deprecated '.' support
. (a single period) is an alias for the source command. The use of . is deprecated in favour of source, and . will be removed in a future version of fish.
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.
@cho-m Thanks!
Edit, do you know how exports look like for fish?
set -l asdf_user_shims (
if test -n "$ASDF_DATA_DIR"
echo $ASDF_DATA_DIR/shims
else
echo $HOME/.asdf/shims
end
)
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.
Exports in Fish shell are done through the set -x
command (though may need -g
if not already global variable).
I am not entirely familiar with Fish's idiosyncrasies since the spec is still in active development compared to POSIX.
I think PATH
is a special already-global variable.
Looking at Fish tutorial, prepending 2 directories is:
set PATH /usr/local/bin /usr/sbin $PATH
The part you quoted is due to Fish shell not supporting POSIX parameter expansion.
Thus, rather than ${ASDF_DATA_DIR:-$HOME/.asdf}
, they need a conditional check.
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.
set PATH (
if test -n "$ASDF_DATA_DIR"
echo $ASDF_DATA_DIR/shims
else
echo $HOME/.asdf/shims
end
) $PATH
Should be good enough for the caveat?
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.
I think it should work.
A side question is if the caveat is becoming too long. I don't know if there is a style recommendation.
Try to get a Homebrew maintainer to review everything again and decide the direction of what the caveat should say.
Hopefully should be last review to finalize PR merge.
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.
@cho-m Earlier suggestions I had was to put all this somewhere as a command, like $ asdf setup
and have that as a post_install
hook, but then again not sure if that also breaks Homebrew rules, @carlocab
I suppose the reason why this wasn't possible now was because $ asdf help
would just error out, but with this fix, it works out of the box regardless of sourcing or updating path.
Fish is not posix.
Should be support for this, since I've seen $PATH updates for various python packages and chruby.
Formula/asdf.rb
Outdated
def caveats | ||
<<~EOS | ||
Add shims in $PATH by having the following line your ~/.zshenv or #{shell_profile}: | ||
export PATH="${ASDF_DATA_DIR:-$HOME/.asdf}/shims:$PATH" | ||
|
||
To support package version per session using asdf shell <name> <version> | ||
Add the following line to your #{shell_profile} file: | ||
. #{opt_libexec}/lib/asdf.sh | ||
If you use Fish shell then add the following line to your ~/.config/fish/config.fish: | ||
source #{opt_libexec}/lib/asdf.fish | ||
Restart your terminal for the settings to take effect. | ||
EOS | ||
end |
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.
Fix indentation, using spaces.
Reflect changes introduced in Homebrew/homebrew-core#73173
Taking into account that macOS `path_helper` might trip some people up.
What's the impetus for closing this and accompanying PRs? |
Inactive? |
Speaking for myself, I've had very little time to put towards |
Right, do you want me to open it again? I assumed lack of interest. |
I am very interested in improving Though I am not going to be free to review anything in detail for at least another week. Perhaps @Stratus3D may have the time between now and then? |
Changes
lib
/usr/local/bin/
will point to/usr/local/opt/asdf/libexec/bin/asdf
to keep the shims from breaking on updates.asdf/libexec/lib/asdf.sh
if needing$ asdf shell <package> <version>
$PATH
sinceasdf/libexec/lib/asdf.sh
does not do that. As opposed toasdf/libexec/asdf.sh
asdf/asdf.sh
will not break as it points toasdf/libexec/asdf.sh
$PATH
if they are currently sourcingasdf/asdf.sh
as it points toasdf/libexec/asdf.sh
Migration
No need with migration for existing installation.
So, existing solutions such as
source "$(brew --prefix asdf)/asdf.sh"
will work fineBut can also add shim manually to
$PATH
and optionally do
source "$(brew --prefix asdf)/libexec/lib/asdf.sh"
for$ asdf shell <package> <version>
Caveat
The caveat currently pending review mentions the "newer" approach, it's not necessary to follow through for current users.
Updating
$PATH
=> MandatorySourcing
libexec/lib/asdf.sh"
=> OptionalRelevant PR(s):
Code
asdf-vm/asdf#897
Documentation
asdf-vm/asdf#898
Motivation
/usr/local/lib
asdf help
after installing.$PATH
, instead expect users themselves to define what's needed in ENV variables, if that's a customasdf
directory, or setting their shims directory under$PATH
, for zsh that would fall under.zshenv
and not.zshrc
.If a user can't call
asdf help
without getting an error after installing it without additional configuration, then I consider it a bug.There are situations where additional setup is mandatory, like databases, but asdf is and should not be treated as such
Beyond that, the current Homebrew formula for a what essentially constitutes a collection of shell script that depend on each other is implemented wrong.
This is the most important take away from this.
Fixing:
asdf-vm/asdf#785
asdf-vm/asdf#891
asdf-vm/asdf#607
asdf-vm/asdf#394
asdf-vm/asdf#428
brew install --build-from-source <formula>
, where<formula>
is the name of the formula you're submitting?brew test <formula>
, where<formula>
is the name of the formula you're submitting?brew audit --strict <formula>
(after doingbrew install <formula>
)?