Jabba-Team / jabba

(cross-platform) Java Version Manager
Apache License 2.0
141 stars 11 forks source link

JABBA_HOME_TO_EXPORT resolves to runtime home, rather than installation location #27

Open MIJOTHY opened 1 year ago

MIJOTHY commented 1 year ago

The lines here: https://github.com/Jabba-Team/jabba/blob/ac5cb4fd85156e68455e08c0545c1057aa8b2a71/install.sh#L7-L12 cause the exported JABBA_HOME in the installed $JABBA_HOME/jabba.sh file to potentially resolve to a different location than the installation location. For example, in a Dockerfile like

ENV HOME "/root"
ENV JABBA_HOME "$HOME/.jabba"
# ... install jabba

The contents of /root/.jabba/jabba.sh will be

# https://github.com/Jabba-Team/jabba
# This file is intended to be "sourced" (i.e. ". ~/.jabba/jabba.sh")

export JABBA_HOME="$HOME/.jabba"

jabba() {
    local fd3=$(mktemp /tmp/jabba-fd3.XXXXXX)
    (JABBA_SHELL_INTEGRATION=ON $HOME/.jabba/bin/jabba "$@" 3>| ${fd3})
    local exit_code=$?
    eval $(cat ${fd3})
    rm -f ${fd3}
    return ${exit_code}
}

if [ ! -z "$(jabba alias default)" ]; then
    jabba use default
fi

There is no guarantee that the jabba.sh invoker's $HOME is the same as the installer's $HOME, as we can see with GitHub Actions which has a $HOME of /github/home:

/root/.jabba/jabba.sh:8: /github/home/.jabba/bin/jabba: No such file or directory

It's unclear to me what the purpose of the runtime$JABBA_HOME resolution is here: https://github.com/Jabba-Team/jabba/blob/ac5cb4fd85156e68455e08c0545c1057aa8b2a71/install.sh#L9

Workaround

This can be worked around by supplying a dummy home to the installation script, such that this check evaluates to false https://github.com/Jabba-Team/jabba/blob/ac5cb4fd85156e68455e08c0545c1057aa8b2a71/install.sh#L7 allowing for the correct setting of JABBA_HOME_TO_EXPORT. For example,

ENV REAL_HOME "/root"
ENV JABBA_HOME "$REAL_HOME/.jabba"

ENV HOME "/"
# ... install jabba
ENV HOME "$REAL_HOME"

As $HOME is additionally used when creating the shell hooks, this also means you probably want to supply --skip-rc to the script, and write your own shellhooks.

Alternatively, you can patch $JABBA_HOME/jabba.sh with the desired export.

Possible Fix

I think the core issue here is the defaulting to runtime resolution of $HOME, and this default not being overridable without $HOME manipulations. I would think the sensible default would be to export the location which we know has jabba.sh, i.e. expand at install-time (as was done prior to https://github.com/Jabba-Team/jabba/commit/be9eef43585ee384f63ae64ec6e3497f4d614748). If you allowing setting of JABBA_HOME_TO_EXPORT by install-script callers, the first part of the script becomes simpler. Instead of https://github.com/Jabba-Team/jabba/blob/ac5cb4fd85156e68455e08c0545c1057aa8b2a71/install.sh#L7C1-L12 you can have something like

if [ "$JABBA_HOME" == "" ]; then
    JABBA_HOME="$HOME/.jabba"
fi
JABBA_HOME_TO_EXPORT=${JABBA_HOME_TO_EXPORT:-JABBA_HOME}

If users want to have this exported home be runtime-user-home-based, they can call the installation script like JABBA_HOME_TO_EXPORT='$HOME/.jabba'.

Thoughts?

patrick-mccourt commented 1 year ago

Hi @MIJOTHY

Thanks for the very detailed issue, will certainly help when addressing it. I've been a little slack lately in Jabba maintenance as my wife and I welcomed our daughter into the world. Once baby is settled and I can dedicate time to this again I can take a look.