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

Keep SDKMAN_DIR from environment variables #46

Closed
wants to merge 1 commit into from

Conversation

xtexChooser
Copy link

sdkman-init.sh also do so

@reitzig
Copy link
Owner

reitzig commented Jan 2, 2024

Please explain shortly which problem this solves?

@reitzig
Copy link
Owner

reitzig commented Jan 2, 2024

Also, I've fixed the test setup; you'll note that you broke one of the tests with your change.

@xtexChooser
Copy link
Author

when SDKMAN_DIR is defined through environment variable, it should not be over-written by sdk.fish.

@reitzig
Copy link
Owner

reitzig commented Apr 1, 2024

@xtexChooser Since you never responded, I'll close this for now.

If this is still relevant to you, please open an issue and explain:

  • Which setups are affected by ignoring pre-set SDKMAN_DIR? How do you set the environment variable?
  • Is the workaround with __sdkman_custom_dir (as mentioned in the docs) necessary at all?
  • If both variables are set, which value should take precedence, and why?

Until I have answers to these questions to mull over, I can't work off your PR to add/adapt tests.

@reitzig reitzig closed this Apr 1, 2024
@xtexChooser
Copy link
Author

Hi, I set SDKMAN_DIR globally, through environment.d with system.

I think __sdkman_custom_dir may be redundant because SDKMAN_DIR is the way sdkman-cli is using to declare a custom installation location.

In my opinion, if both are defined, __sdkman_custom_dir should be preferred.

xtex@xtex ~ (main)> echo $SDKMAN_DIR
/home/xtex/.sdkman
xtex@xtex ~ (main)> sdk update
You don't seem to have SDKMAN! installed. Install now? [y/N] 
xtex@xtex ~ (main)> ls /home/xtex/.sdkman
ls: cannot access '/home/xtex/.sdkman': No such file or directory
xtex@xtex ~ (main) [2]> bash
xtex@xtex:~> echo $SDKMAN_DIR
/opt/sdkman
xtex@xtex:~> ls /opt/sdkman
bin  candidates  contrib  etc  ext  libexec  src  tmp  var

@reitzig
Copy link
Owner

reitzig commented Apr 3, 2024

Fix released with v2.1.0, thanks!

@xtexChooser
Copy link
Author

xtexChooser commented Apr 3, 2024 via email

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.

None yet

2 participants