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

setting SDKMAN_DIR doesn't work #55

Open
scarf005 opened this issue Aug 19, 2024 · 4 comments
Open

setting SDKMAN_DIR doesn't work #55

scarf005 opened this issue Aug 19, 2024 · 4 comments
Assignees
Labels
documentation enhancement New feature or request

Comments

@scarf005
Copy link

scarf005 commented Aug 19, 2024

Summary

- Only compatible with fisher v4 upwards; v3 is no longer supported.
- You have to install [SDKMAN!] separately.
- If you have installed SDKMAN! at a custom location, you need to either
- set environment variable `SDKMAN_DIR` to that path using your preferred method, or
- add
```fish
set -g __sdkman_custom_dir /your/path/to/sdkman
```
to a fish config file
[run _before_](https://fishshell.com/docs/current/language.html#configuration-files)
`.config/fish/conf.d/sdk.fish`;
for example, you can use `.config/fish/conf.d/config_sdk.fish`.
- If _both_ are set, `__sdkman_custom_dir` is used.

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/[email protected]

$ sdk version
SDKMAN!
script: 5.18.2
native: 0.4.6
@reitzig
Copy link
Owner

reitzig commented Aug 19, 2024

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
Copy link
Author

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
Copy link
Owner

reitzig commented Aug 19, 2024

$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 reitzig added bug Something isn't working documentation labels Aug 19, 2024
@reitzig reitzig self-assigned this Aug 19, 2024
@reitzig reitzig added enhancement New feature or request and removed bug Something isn't working labels Aug 19, 2024
@reitzig
Copy link
Owner

reitzig commented Aug 19, 2024

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants