Skip to content

fix: use absolute path in latest-stable callback when calling list-all #144

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

Merged
merged 2 commits into from
Apr 8, 2025

Conversation

navinpeiris
Copy link
Contributor

Fixes #143

@Stratus3D
Copy link
Member

@navinpeiris can you explain how this fixes #143?

@Eiji7 can you test this?

@Eiji7
Copy link
Contributor

Eiji7 commented Apr 7, 2025

@Stratus3D Not sure if remember correctly, but there may be problems if $SCRIPT_DIR contains a whitespace. This should not be a case when calling a script using a dot. To fix this we should wrap $SCRIPT_DIR/list-all in double quotes. That's said calling with a dot is not perfect in every case.

Bash and environment-specific edge cases related to paths are really complicated. Here are some possible problems:

  1. Whitespace in the path
  2. Symlinks
  3. User-defined custom cd
    and possibly many others I just don't remember right now …

That's why initially I used the asdf command to do that. Relying on the Go program is the safest option as far as I know, but don't believe me in my words as Bash is not the easiest language.- I know it well, but also I'm definitely not an expert. For more information, see:
https://stackoverflow.com/a/246128
There is a more complete script example for getting a script directory and some notes on symlinks as well.

btw. Sorry for asking it many times, but any idea if #142 PR is going to be accepted?

# calling a script to list all elixir releases
# reject releases before 1.0.0 (starting with 0)
# reject main and master branches
# reject releases with otp version
# reject release candidates
# reject empty line
# finally take last one
ASDF_ELIXIR_LATEST_VERSION=$(./list-all | tr " " "\n" | grep -Ev "^0|^main|^master|otp|rc|^$" | tail -n 1)
ASDF_ELIXIR_LATEST_VERSION=$($SCRIPT_DIR/list-all | tr " " "\n" | grep -Ev "^0|^main|^master|otp|rc|^$" | tail -n 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think $SCRIPT_DIR here should be wrapped in quotes, otherwise this looks good to me.

@navinpeiris
Copy link
Contributor Author

@navinpeiris can you explain how this fixes #143?

Sorry, should have done that when submitting the PR!

The call to list-all used to fail as bash would try to execute that script as if it's in the current directory that the user is invoking asdf latest elixir in, and not in the directory that the latest-stable script resides as was intended.

This would then cause ASDF_ELIXIR_LATEST_VERSION to resolve to an empty string, and when echoing "$ASDF_ELIXIR_LATEST_VERSION-otp-$ASDF_ELIXIR_LATEST_OTP", would result in -otp-27 being printed in the terminal.

SCRIPT_DIR resolves to the absolute path of the directory containing the latest-stable script, and now that we prefix list-all with that absolute path, we can resolve the latest elixir version correctly.

I've wrapped SCRIPT_DIR in double quotes to account for directories with spaces, and this version of SCRIPT_DIR accounts for symlinks as well.

@Stratus3D Stratus3D changed the title Use absolute path in latest-stable when calling list-all fix: use absolute path in latest-stable callback when calling list-all Apr 8, 2025
@Stratus3D Stratus3D merged commit fb53c45 into asdf-vm:master Apr 8, 2025
4 checks passed
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.

Calling asdf latest elixir results in -otp-27
3 participants