Skip to content

Speed up initialization in popular shells #1470

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.

Sign up for GitHub

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

Closed
azhuchkov wants to merge 1 commit into from
Closed

Speed up initialization in popular shells #1470

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 25 additions & 15 deletions src/main/bash/sdkman-path-helpers.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,28 +50,38 @@ function __sdkman_set_candidate_home() {
}

function __sdkman_export_candidate_home() {
local candidate_name="$1"
local candidate_dir="$2"
local candidate_home_var="$(echo ${candidate_name} | tr '[:lower:]' '[:upper:]')_HOME"
export $(echo "$candidate_home_var")="$candidate_dir"
}
local candidate_name="$1" candidate_dir="$2" upcase_name

function __sdkman_determine_candidate_bin_dir() {
local candidate_dir="$1"
if [[ -d "${candidate_dir}/bin" ]]; then
echo "${candidate_dir}/bin"
# Generate home variable name using shell-specific methods
# to avoid heavy-weight fork/exec system calls.
Copy link
Member

@marc0der marc0der Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer not to have comments unless it is absolutely necessary. Having all the details in the Git commit message is sufficient.


if [ -n "${ZSH_VERSION-}" ]; then
Copy link
Member

@marc0der marc0der Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a global variable that you can use here.

# zsh: uppercase via ${value:u}
upcase_name="${candidate_name:u}"
elif [ -n "${BASH_VERSION-}" ] && [ "${BASH_VERSINFO[0]:-0}" -ge 4 ] 2>/dev/null; then
Copy link
Member

@marc0der marc0der Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also use the global definition here. Do we need to bolster our global definition with BASH_VERSIONINFO too?

# bash 4.0+: uppercase via ${value^^}
upcase_name="${candidate_name^^}"
else
echo "$candidate_dir"
# POSIX fallback: call external tool for conversion
upcase_name="$(printf %s "$candidate_name" | tr '[:lower:]' '[:upper:]')"
fi

export "${upcase_name}_HOME=$candidate_dir"
}

function __sdkman_prepend_candidate_to_path() {
local candidate_dir candidate_bin_dir
local candidate_dir="$1"

# Caution: external commands are costly here
# since this function runs inside a loop.

candidate_dir="$1"
candidate_bin_dir=$(__sdkman_determine_candidate_bin_dir "$candidate_dir")
echo "$PATH" | grep -q "$candidate_dir" || PATH="${candidate_bin_dir}:${PATH}"
unset CANDIDATE_BIN_DIR
# replace the original candidate_dir with the 'bin' subfolder if it exists
if [ -d "${candidate_dir}/bin" ]; then
candidate_dir="${candidate_dir}/bin"
Copy link
Member

@marc0der marc0der Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not correct, as candidate_dir describes the base directory of the candidate (the candidate's home directory), not its bin directory. For readability, I prefer a separate variable for the candidate_bin_dir as we had before.

fi
# prepend PATH with the candidate_dir if it is not already there,
# assuming the dir doesn't contain meta-characters like ? * []
[[ ":$PATH:" == *":$candidate_dir:"* ]] || PATH="${candidate_dir}:${PATH}"
}

function __sdkman_link_candidate_version() {
Expand Down