setting `SDKMAN_DIR` doesn't work

scarf005 commented 3 weeks ago

Summary

https://github.com/reitzig/sdkman-for-fish/blob/555203d56e534d91cde87ad600cbbf6f2d112a03/README.md?plain=1#L28-L40

despite the description in https://github.com/reitzig/sdkman-for-fish?tab=readme-ov-file#install says to either set SDKMAN_DIR or __sdkman_custom_dir, only the __sdkman_custom_dir method works.

Log

nemo@ip-192-168-20-95 ~> sdk
You don't seem to have SDKMAN! installed. Install now? [y/N] n
nemo@ip-192-168-20-95 ~> echo $SDKMAN_DIR
/Users/nemo/.local/share/sdkman
nemo@ip-192-168-20-95 ~> ls ~/.local/share/sdkman/
bin/        contrib/    ext/        src/        var/
candidates/ etc/        libexec/    tmp/
nemo@ip-192-168-20-95 ~>

Workaround

insert set -g __sdkman_custom_dir $SDKMAN_DIR to $HOME/.config/fish/functions/sdk.fish, which suggests sdk.fish gain access to SDKMAN_DIR defined in $HOME/.config/fish/config.fish

System environment

OS: macOS Monterey 12.5 arm64
Kernel: Darwin 21.6.0
Shell: fish 3.7.1
reitzig/sdkman-for-fish@v2.1.0

$ sdk version
SDKMAN!
script: 5.18.2
native: 0.4.6

reitzig commented 3 weeks ago

The feature is tested: ✅ test/features/corner_cases.feature#8

I suspect you set SDKMAN_DIR "too late". conf.d/sdk.fish is executed at shell startup; you need to set the variable before that, e.g. in .config/fish/config.fish.

If that is not the case, can you contribute a failing test?

scarf005 commented 3 weeks ago

I suspect you set SDKMAN_DIR "too late". conf.d/sdk.fish is executed at shell startup; you need to set the variable before that, e.g. in .config/fish/config.fish.

but i did set up SDKMAN_DIR in .config/fish/config.fish:

insert set -g __sdkman_custom_dir $SDKMAN_DIR to $HOME/.config/fish/functions/sdk.fish, which suggests sdk.fish gain access to SDKMAN_DIR defined in $HOME/.config/fish/config.fish

i do not have access to my work laptop right now; will try it tomorrow just in case.

reitzig commented 3 weeks ago

$HOME/.config/fish/functions/sdk.fish

There, the actual function run each time you type sdk is defined.

Handling of custom installation dirs is currently implemented in $HOME/.config/fish/conf.d/sdk.fish for reasons, which runs once per shell instance, during its startup.

Still. Huh. 🤔

(googles)

Ah. 💡 I had it backwards: conf.d/*.fish are sourced before config.fish (cf. official docs). So that explains it.

FWIW: The tests are green because there, we call fish as a subprocess after setting the env var; that is not how we use Fish in real life, though.

There are three options I see:

Change the documentation to explain where/how to set the environment variable so it's effective (and add a test for that) (simple)
- They are executed in order of their filename, sorted (like globs) in a natural order (i.e. “01” sorts before “2”).
So creating a file that is sourced before sdk.fish due to a well-chosen filename would work.
- set -Ux SDKMAN_DIR /my/dir seems to work as well.
Change the code (and docs) to remove that option altogether. (simple)
Change the code to resolve SDKMAN! installation dir on each call to sdk (instead of upfront). (bigger change)

reitzig commented 3 weeks ago

@scarf005 Please confirm that the plugin works as intended when you

remove the workaround,
set SDKMAN_DIR using set -Ux, and
open a new shell.

If so, I'll update the README to make clear that that's the supported way, replacing the vague

set environment variable SDKMAN_DIR to that path using your preferred method, or

reitzig / sdkman-for-fish