Skip to content

Improving go helper #2023

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

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

Improving go helper #2023

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
85fff7c
Improving go helper
yalh76 Dec 29, 2024
d5820fb
Merge branch 'dev' into go_helper
yalh76 Jan 5, 2025
f382928
Update go
yalh76 Jan 5, 2025
cd12849
Update helpers/helpers.v1.d/go
yalh76 Jan 12, 2025
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
133 changes: 75 additions & 58 deletions helpers/helpers.v1.d/go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,41 +18,32 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#

ynh_go_try_bash_extension() {
if [ -x src/configure ]; then
src/configure && make -C src || {
ynh_print_info --message="Optional bash extension failed to build, but things will still work normally."
}
fi
}

goenv_install_dir="/opt/goenv"
go_version_path="$goenv_install_dir/versions"
# goenv_ROOT is the directory of goenv, it needs to be loaded as a environment variable.

# GOENV_ROOT is the directory of goenv, it needs to be loaded as a environment variable.
export GOENV_ROOT="$goenv_install_dir"
export goenv_root="$goenv_install_dir"
Copy link
Member

Choose a reason for hiding this comment

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

(are we sure that having that same var in lowercase is actually useful for anything x_x ... i guess it comes from copypasting the same stuff as for ruby but that sounds very meh)

Copy link
Contributor Author

@yalh76 yalh76 Jan 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think, it's usefull, but was in ruby helper


# Load the version of Go for an app, and set variables.
#
# ynh_use_go has to be used in any app scripts before using Go for the first time.
# This helper will provide alias and variables to use in your scripts.
#
# To use gem or Go, use the alias `ynh_gem` and `ynh_go`
# Those alias will use the correct version installed for the app
# For example: use `ynh_gem install` instead of `gem install`
# To use Go, use the alias `ynh_go`
# This alias will use the correct version installed for the app
#
# With `sudo` or `ynh_exec_as`, use instead the fallback variables `$ynh_gem` and `$ynh_go`
# With `sudo` or `ynh_exec_as`, use instead the fallback variables `$ynh_go`
# And propagate $PATH to sudo with $ynh_go_load_path
# Exemple: `ynh_exec_as $app $ynh_go_load_path $ynh_gem install`
#
# $PATH contains the path of the requested version of Go.
# However, $PATH is duplicated into $go_path to outlast any manipulation of $PATH
# You can use the variable `$ynh_go_load_path` to quickly load your Go version
# in $PATH for an usage into a separate script.
# Exemple: `$ynh_go_load_path $install_dir/script_that_use_gem.sh`
#
#
# Finally, to start a Go service with the correct version, 2 solutions
# Either the app is dependent of Go or gem, but does not called it directly.
# Either the app is dependent of Go, but does not called it directly.
# In such situation, you need to load PATH
# `Environment="__YNH_GO_LOAD_PATH__"`
# `ExecStart=__INSTALL_DIR__/my_app`
Expand Down Expand Up @@ -81,7 +72,6 @@ ynh_use_go() {
# Create an alias for the specific version of Go and a variable as fallback
ynh_go="$go_path/go"
alias ynh_go="$ynh_go"

# Load the path of this version of Go in $PATH
if [[ :$PATH: != *":$go_path"* ]]; then
PATH="$go_path:$PATH"
Expand All @@ -90,7 +80,7 @@ ynh_use_go() {
ynh_go_load_path="PATH=$PATH"

# Sets the local application-specific Go version
pushd $install_dir
pushd ${install_dir:-$install_dir}
$goenv_install_dir/bin/goenv local $go_version
popd
}
Expand All @@ -105,7 +95,7 @@ ynh_use_go() {
# Don't forget to execute go-dependent command in a login environment
# (e.g. sudo --login option)
# When not possible (e.g. in systemd service definition), please use direct path
# to goenv shims (e.g. $goenv_ROOT/shims/bundle)
# to goenv shims (e.g. $GOENV_ROOT/shims/bundle)
#
# usage: ynh_install_go --go_version=go_version
# | arg: -v, --go_version= - Version of go to install.
Expand All @@ -130,36 +120,51 @@ ynh_install_go() {

# Install or update goenv
mkdir -p $goenv_install_dir
pushd "$goenv_install_dir"
if ! [ -x "$goenv_install_dir/bin/goenv" ]; then
ynh_print_info --message="Downloading goenv..."
git init -q
git remote add origin https://github.com/syndbg/goenv.git
goenv="$(command -v goenv $goenv_install_dir/bin/goenv | grep "$goenv_install_dir/bin/goenv" | head -1)"
Copy link
Contributor

Choose a reason for hiding this comment

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

This command will fail if goenv is not present. You should something like that instead:

if command -v …

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In that case

yunohost/helpers/helpers.v1.d/ruby

Line 135 in a96cb9b

rbenv="$(command -v rbenv $rbenv_install_dir/bin/rbenv | grep "$rbenv_install_dir/bin/rbenv" | head -1)"

and

yunohost/helpers/helpers.v2.1.d/ruby

Line 77 in a96cb9b

rbenv="$(command -v rbenv $RBENV_INSTALL_DIR/bin/rbenv | grep "$RBENV_INSTALL_DIR/bin/rbenv" | head -1)"

should not be ok

Copy link
Member

Choose a reason for hiding this comment

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

To me what's troublesome is : what is it doing in the first place x_X

It looks like this is meant to verify that $RBENV_INSTALL_DIR/bin/rbenv exists (and maybe is executable) ? Shouldn't we just use something like

rbenv="$RBENV_INSTALL_DIR/bin/rbenv"
if [[ -x "$rbenv" ]]
then
     ...
else
     ...
fi

(In fact that was the original code x_x)

if [ -n "$goenv" ]; then
pushd "${goenv%/*/*}"
if git remote -v 2> /dev/null | grep "https://github.com/go-nv/goenv.git"; then
ynh_print_info --message="Updating goenv..."
git pull -q --tags origin master
ynh_go_try_bash_extension
else
ynh_print_info --message="Reinstalling goenv..."
cd ..
ynh_secure_remove --file=$goenv_install_dir
mkdir -p $goenv_install_dir
cd $goenv_install_dir
git init -q
git remote add -f -t master origin https://github.com/go-nv/goenv.git > /dev/null 2>&1
git checkout -q -b master origin/master
ynh_go_try_bash_extension
goenv=$goenv_install_dir/bin/goenv
fi
popd
else
ynh_print_info --message="Updating goenv..."
ynh_print_info --message="Installing goenv..."
pushd $goenv_install_dir
git init -q
git remote add -f -t master origin https://github.com/go-nv/goenv.git > /dev/null 2>&1
git checkout -q -b master origin/master
ynh_go_try_bash_extension
goenv=$goenv_install_dir/bin/goenv
popd
fi
git fetch -q --tags --prune origin
local git_latest_tag=$(git describe --tags "$(git rev-list --tags --max-count=1)")
git checkout -q "$git_latest_tag"
ynh_go_try_bash_extension
goenv=$goenv_install_dir/bin/goenv
popd

# Install or update xxenv-latest
goenv_latest_dir="$goenv_install_dir/plugins/xxenv-latest"
mkdir -p "$goenv_latest_dir"
pushd "$goenv_latest_dir"
if ! [ -x "$goenv_latest_dir/bin/goenv-latest" ]; then
ynh_print_info --message="Downloading xxenv-latest..."
git init -q
git remote add origin https://github.com/momo-lab/xxenv-latest.git
mkdir -p "${goenv_install_dir}/plugins"

goenv_latest="$(command -v "$goenv_install_dir"/plugins/*/bin/goenv-latest goenv-latest | head -1)"
if [ -n "$goenv_latest" ]; then
pushd "${goenv_latest%/*/*}"
if git remote -v 2> /dev/null | grep "https://github.com/momo-lab/xxenv-latest.git"; then
ynh_print_info --message="Updating xxenv-latest..."
git pull -q origin master
fi
popd
else
ynh_print_info --message="Updating xxenv-latest..."
ynh_print_info --message="Installing xxenv-latest..."
git clone -q https://github.com/momo-lab/xxenv-latest.git "${goenv_install_dir}/plugins/xxenv-latest"
fi
git fetch -q --tags --prune origin
local git_latest_tag=$(git describe --tags "$(git rev-list --tags --max-count=1)")
git checkout -q "$git_latest_tag"
popd

# Enable caching
mkdir -p "${goenv_install_dir}/cache"
Expand All @@ -174,12 +179,15 @@ ynh_install_go() {
test -x /usr/bin/go_goenv && mv /usr/bin/go_goenv /usr/bin/go

# Install the requested version of Go
local final_go_version=$("$goenv_latest_dir/bin/goenv-latest" --print "$go_version")
ynh_print_info --message="Installation of Go-$final_go_version"
goenv install --skip-existing "$final_go_version"
local final_go_version=$(goenv latest --print $go_version)
if ! [ -n "$final_go_version" ]; then
final_go_version=$go_version
fi
ynh_print_info --message="Installing Go $final_go_version"
goenv install --skip-existing $final_go_version > /dev/null 2>&1

# Store go_version into the config of this app
ynh_app_setting_set --app="$app" --key="go_version" --value="$final_go_version"
ynh_app_setting_set --app=$app --key=go_version --value=$final_go_version

# Cleanup Go versions
ynh_cleanup_go
Expand All @@ -192,7 +200,8 @@ eval \"\$(goenv init -)\"
#goenv" > /etc/profile.d/goenv.sh

# Load the environment
HOME=$install_dir eval "$(goenv init -)"
export HOME=${HOME:-"/root/"}
eval "$(goenv init -)"
Comment on lines -195 to +204
Copy link
Member

Choose a reason for hiding this comment

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

I would tend to keep the previous syntax such that HOME doesn't get change in the rest of the script which could have side effect idk ... also we probably want to make sure that HOME is the app's install dir and not /root/ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should remove :
the part :

yunohost/helpers/helpers.v2.1.d/go

Lines 150 to 159 in cd12849

# Set environment for Go users
echo "#goenv
export GOENV_ROOT=$GOENV_INSTALL_DIR
export PATH=\"$GOENV_INSTALL_DIR/bin:$PATH\"
eval \"\$(goenv init -)\"
#goenv" > /etc/profile.d/goenv.sh
# Load the environment
export HOME=${HOME:-"/root/"}
eval "$(goenv init -)"

from each helper Go and Ruby; helper 2 and helper2.1

The first part /etc/profile.d/**** is to set the environment variables at startup of the Bash shell. But we don't need go and ruby environment variables set to each users.

The second part eval \"\$(goenv init -)\" is to initialise environment for current user, but we don't need that either, as all we need is done in _ynh_load_go_in_path_and_other_tweaks same for ruby

}

# Remove the version of Go used by the app.
Expand All @@ -201,7 +210,7 @@ eval \"\$(goenv init -)\"
#
# usage: ynh_remove_go
ynh_remove_go() {
local go_version=$(ynh_app_setting_get --app="$app" --key="go_version")
local go_version=$(ynh_app_setting_get --app=$app --key=go_version)

# Load goenv path in PATH
local CLEAR_PATH="$goenv_install_dir/bin:$PATH"
Expand All @@ -210,7 +219,7 @@ ynh_remove_go() {
PATH=$(echo $CLEAR_PATH | sed 's@/usr/local/bin:@@')

# Remove the line for this app
ynh_app_setting_delete --app="$app" --key="go_version"
ynh_app_setting_delete --app=$app --key=go_version

# Cleanup Go versions
ynh_cleanup_go
Expand All @@ -226,29 +235,37 @@ ynh_remove_go() {
ynh_cleanup_go() {

# List required Go versions
local installed_apps=$(yunohost app list --output-as json --quiet | jq -r .apps[].id)
local installed_apps=$(yunohost app list | grep -oP 'id: \K.*$')
Comment on lines -229 to +238
Copy link
Member

Choose a reason for hiding this comment

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

Uuuuh not sure why switching to the new version ... the current one with json and jq seems much more clean and robust ? Is it that the ruby helpers still use an hacky grep ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes ruby helper still using grep

yunohost/helpers/helpers.v2.1.d/ruby

Line 231 in f7ed895

local installed_apps=$(yunohost app list | grep -oP 'id: \K.*$')

and

yunohost/helpers/helpers.v1.d/ruby

Line 285 in f7ed895

local installed_apps=$(yunohost app list | grep -oP 'id: \K.*$')

local required_go_versions=""
for installed_app in $installed_apps; do
local installed_app_go_version=$(ynh_app_setting_get --app=$installed_app --key="go_version")
if [[ $installed_app_go_version ]]; then
if [[ -n "$installed_app_go_version" ]]; then
required_go_versions="${installed_app_go_version}\n${required_go_versions}"
fi
done

# Remove no more needed Go versions
local installed_go_versions=$(goenv versions --bare --skip-aliases | grep -Ev '/')
for installed_go_version in $installed_go_versions; do
if ! $(echo ${required_go_versions} | grep "${installed_go_version}" 1> /dev/null 2>&1); then
ynh_print_info --message="Removing of Go-$installed_go_version"
$goenv_install_dir/bin/goenv uninstall --force "$installed_go_version"
if ! echo ${required_go_versions} | grep -q "${installed_go_version}"; then
ynh_print_info --message="Removing Go-$installed_go_version"
$goenv_install_dir/bin/goenv uninstall --force $installed_go_version
fi
done

# If none Go version is required
if [[ ! $required_go_versions ]]; then
if [[ -z "$required_go_versions" ]]; then
# Remove goenv environment configuration
ynh_print_info --message="Removing of goenv"
ynh_print_info --message="Removing goenv"
ynh_secure_remove --file="$goenv_install_dir"
ynh_secure_remove --file="/etc/profile.d/goenv.sh"
fi
}

ynh_go_try_bash_extension() {
if [ -x src/configure ]; then
src/configure && make -C src || {
ynh_print_info --message="Optional bash extension failed to build, but things will still work normally."
}
fi
}
Loading
Loading