Asdf: System Version Broken

[mwean]

Steps to reproduce

> asdf plugin-add ruby
> asdf local ruby system
> ruby -v

Expected behavior

Print out the system ruby version

Actual behavior

Hangs indefinitely

Environment

OS: macOS 10.13.6

asdf version: 0.6.0

I tracked the issue down to the get_executable_path function, where it removes the shim directory from the path and tries to find the executable if the version is system:

https://github.com/asdf-vm/asdf/blob/dab04107ff88a34d95b09a44a94320d27a2af470/lib/utils.sh#L230-L231

The problem is it's only removing ASDF_DIR and not ASDF_USER_SHIMS. I was going to make a PR just adding that, but it seems really weird that we have both defined. I would expect there to be one shims directory, either defined by the user or a default. What do you think?

[Stratus3D]

You are correct. There should only be one shims directory. My guess is that line should be ...s|$ASDF_DATA_DIR/shims||g.... This is likely a bug.

[mwean]

Sorry, I totally forgot to include the main point of the issue:

https://github.com/asdf-vm/asdf/blob/dab04107ff88a34d95b09a44a94320d27a2af470/asdf.sh#L19-L20

So we have a variable for _both_ shims directories, which seems wrong to me. I would expect to have one, which would be used here.

[rliebz]

Looking through the code, it seems like the only references left to ${ASDF_DIR}/shims are in asdf.sh to add it to the path, asdf.fish for completion, and a whole bunch of test files.

I'm guessing ${ASDF_DIR}/shims is meant to be deprecated based on that, the discussion in a few of the recent PRs (https://github.com/asdf-vm/asdf/pull/335, https://github.com/asdf-vm/asdf/pull/364), and the fact that the following line of code fixes the issue on my machine:

    path=$(echo "$PATH" | sed -e "s|$(asdf_data_dir)/shims||g; s|::|:|g")

Here the difference between $(asdf_data_dir) and $ASDF_DATA_DIR is just whether we're handling the default value.

The only question that leaves me is whether for users who upgraded from earlier versions need the original shims directory there to avoid breaking their local installations, or if it's safe to remove completely. If it's the former, the safest answer is probably just to remove both directories from the PATH:

    path=$(echo "$PATH" | sed -e "s|$ASDF_DIR/shims||g; s|$(asdf_data_dir)/shims||g; s|::|:|g")

[mwean]

Hmm I hadn't thought about compatibility with earlier versions... On the other hand, it seems like a lot of things would be broken already if most of the code is using the new variable. I would prefer just getting rid of the old one everywhere.

[Stratus3D]

@rliebz support for the new asdf_data_dir feature is not backwards compatible, as there was no equivalent functionality in older versions. I think $(asdf_data_dir) is what we want.