search

reitzig / sdkman-for-fish

Adds support for SDKMAN! to fish
MIT License
280 stars 13 forks source link

(Successful) Calls to sdk overwrite the environment, including PATH #19

Closed reitzig closed 4 years ago

reitzig commented 5 years ago

Expected

 $ java -version
java version "10.0.1" 2018-04-17
$ sdk use java 8.0.171-oracle 

Using java version 8.0.171-oracle in this shell.
$ java -version
java version "1.8.0_171"
$ sdk install ant foo
<snip>
$ java -version
java version  "1.8.0_171"

Actual

 $ java -version
java version "10.0.1" 2018-04-17
$ sdk use java 8.0.171-oracle 

Using java version 8.0.171-oracle in this shell.
$ java -version
java version "1.8.0_171"
$ sdk install ant foo
<snip>
$ java -version
java version "10.0.1" 2018-04-17

Analysis

We delegate command source $sdkman_init && sdk $argv to bash. We have to, because otherwise function sdk is not defined in that shell.

This initialization overwrites the environment variables inherited from the parent fish shell, including PATH (relevant) and SDKMAN_* (probably not?). After it completes, fish copies over the environment. It has to, otherwise no command would have any effect (cf issue #8).

Plan

Without changes in SDKMAN! that separate function declaration from environment initialization, it's probably gonna be hacky.

Idea: hard-code resetting the environment into the delegation command (between source and sdk).

reitzig commented 5 years ago

Idea 2: source grep -v 'export' ~/.sdkman/bin/sdkman-init.sh (roughly speaking, three empty if clauses remain) and hope that nothing breaks.

reitzig commented 5 years ago

Huh. sed -e 's/^\(\s*\).*\(export\|to_path\).*$/\1:/g' "$__fish_sdkman_init" -- or, in clumsy human-speak, remove all lines that write to PATH -- seems to actually do it.

reitzig commented 5 years ago

The current hack is not robust against updates of SDKMAN! (or users editing sdkman-init.sh): the old edited version will stay around.

reitzig commented 5 years ago

Tentatively merging this into master because the risk of the hack is offset by, well, sdk working at all. Leaving this open in the hope that a better solution arises.

reitzig commented 4 years ago

Closing for now: the workaround seems to be effective.

We may need a little more finesse for some corner cases; such will be dealt with in separate issues if and when they arise.