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.

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

refactor: MAJOR refactor/beautify of build.sh #632

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

refactor: MAJOR refactor/beautify of build.sh #632

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
225 changes: 186 additions & 39 deletions build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,53 +2,200 @@

set -Eeou pipefail

# color codes
# fold me :) (in vscode vscodium etc)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't really see the point of a "fold me" comment. Even more so a reference to one particular editor. I don't think this is helpful

Frankly I find the region/end region comments more distracting than helpful. Please remove

#region DEFAULTS_AND_COMMAND_LINE_ARGUMENTS
# Parse command-line arguments for flags
verbose=false
UBPORTSDOCSENV=${UBPORTSDOCSENV:-"$HOME/ubportsdocsenv"}
while [[ $# -gt 0 ]]; do
case $1 in
--verbose)
verbose=true
shift
;;
--env)
# If --env flag is passed, set UBPORTSDOCSENV to the next argument value
UBPORTSDOCSENV=${2-$UBPORTSDOCSENV}
shift 2
;;
*)
shift
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's be a little more defensive here and error out instead of silently ignoring invalid options

;;
esac
Copy link
Collaborator

Choose a reason for hiding this comment

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

shift here instead

done
#endregion DEFAULTS_AND_COMMAND_LINE_ARGUMENTS

#region VARIABLES & FUNCTIONS
#region COLOR_CODES
#region COLOR_CODES_CONSTANTS
GREEN='\033[0;32m'
YELLOW='\033[1;33m'
RED='\033[0;31m'
PLAIN='\033[0m'

# build directory
## use environment variable if it is set, or default value
UBPORTSDOCSENV=${UBPORTSDOCSENV-"$HOME/ubportsdocsenv"}
## use commandline parameter if it is given
UBPORTSDOCSENV=${1-$UBPORTSDOCSENV}

# check if virtualenv already exists
if [ -d "$UBPORTSDOCSENV" ]; then
echo -e "${GREEN}Activating virtual build environment found in \"$UBPORTSDOCSENV\".${PLAIN}"
source $UBPORTSDOCSENV/bin/activate
else
echo -e "${YELLOW}Build environment not found in \"$UBPORTSDOCSENV\" ... creating it.${PLAIN}"
echo -e "${YELLOW}Installing pip and virtualenv (using sudo).${PLAIN}"
# Check for package manager
SUCCESS=$GREEN
WARNING=$YELLOW
ERROR=$RED
PLAIN_TEXT=$PLAIN
#endregion COLOR_CODES_CONSTANTS

echo_color(){
local text="$1"
local start_color_code="${2}"
local end_color_code="${3:-$PLAIN}"

echo -e "${start_color_code}${text}${end_color_code}"
}
echo_plain(){
local text="$1"
echo_color "$text" "$PLAIN"
}
echo_success(){
local text="$1"
echo_color "$text" "$GREEN"
}
echo_warning(){
local text="$1"
echo_color "$text" "$YELLOW"
}
echo_error(){
local text="$1"
echo_color "$text" "$RED"
}
#endregion COLOR_CODES

#region INSTALL_DEPENDENCIES
# DEPENDENCIES
APK_PACKAGES="python3 py3-pip py3-virtualenv"
APT_PACKAGES="python3-pip python3-virtualenv"
PACMAN_PACKAGES="python-pip python-virtualenv"

# Function to install packages via apk (with silent installation)
install_via_apk() {
echo_plain "Detected apk"

# Check if packages are already installed
for package in $APK_PACKAGES; do
if ! apk info -e "$package" &> /dev/null; then
echo_warning "Package $package is not installed. Installing..."
# Install silently, redirecting output to /dev/null, but keep errors
if sudo apk add "$package" &> /dev/null; then
echo_plain "Package $package installed successfully."
else
echo_error "Failed to install $package."
fi
else
echo_plain "$package is already installed."
fi
done
}

# Function to install packages via apt (with silent installation)
install_via_apt() {
echo_plain "Detected apt"

# Check if packages are already installed
for package in $APT_PACKAGES; do
if ! dpkg -l | grep -q "$package"; then
echo_warning "Package $package is not installed. Installing..."
# Install silently, redirecting output to /dev/null, but keep errors
if sudo apt install -y "$package" &> /dev/null; then
echo_plain "Package $package installed successfully."
else
echo_error "Failed to install $package."
fi
else
echo_plain "$package is already installed."
fi
done
}

# Function to install packages via pacman (with silent installation)
install_via_pacman() {
echo_plain "Detected pacman"

# Check if packages are already installed
for package in $PACMAN_PACKAGES; do
if ! pacman -Qi "$package" &> /dev/null; then
echo_warning "Package $package is not installed. Installing..."
# Install silently, redirecting output to /dev/null, but keep errors
if sudo pacman -S --noconfirm "$package" &> /dev/null; then
echo_plain "Package $package installed successfully."
else
echo_error "Failed to install $package."
fi
else
echo_plain "$package is already installed."
fi
done
}


# Function to install dependencies for most Linux systems
install_dependencies_for_most_linux_systems() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we just call this install_dependencies please

echo_success "Checking dependencies"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be plain?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe the detected xyz could be the success?!


if command -v apk &> /dev/null; then
echo "Detected apk"
sudo apk add python3 py3-pip py3-virtualenv
install_via_apk
elif command -v apt &> /dev/null; then
echo "Detected apt"
sudo apt install python3-pip python3-virtualenv
install_via_apt
elif command -v pacman &> /dev/null; then
echo "Detected pacman"
sudo pacman -S --noconfirm python-pip python-virtualenv
install_via_pacman
else
echo "Unsupported package manager. Please install the packages and create the virtualenv manually."
echo_error "Unsupported package manager. Please install the packages and create the virtualenv manually."
Copy link
Collaborator

Choose a reason for hiding this comment

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

The way it's implemented right now with exit 1, it's misleading to say users should install dependecies, since even if they did, it would still error out. Better to be factual and just say "Failed to detect package manager"

If the exit would be removed, the script could continue. In that case this should be a warning about package manager detection.

In no case does it make sense to instruct about virtualenv, since this would be a separate subsequent step.

exit 1
fi
echo -e "${YELLOW}Creating and activating a virtual build environment in \"$UBPORTSDOCSENV\".${PLAIN}"
virtualenv $UBPORTSDOCSENV
source $UBPORTSDOCSENV/bin/activate
echo -e "${YELLOW}Installing build tools and prerequisites (using pip).${PLAIN}"
pip3 install -r requirements.txt
fi

echo -e "${GREEN}Building...${PLAIN}"
rm -rf _build/
if [ "$(uname)" == "Linux" ]; then
sphinx-build -Wa . _build/html -j `nproc --all` || exit $?
else
sphinx-build -Wa . _build/html -j `sysctl -n hw.ncpu` || exit $?
fi
sphinx-build -Wab rediraffecheckdiff . _build/html

echo "Open _build/html/index.html in your browser to navigate through the docs"
}
#endregion INSTALL_DEPENDENCIES

#region PREPARE_VIRTUALENV
prepare_virtualenv_for_python(){
# check if virtualenv already exists
if [ -d "$UBPORTSDOCSENV" ]; then
echo_success "Activating virtual build environment found in \"$UBPORTSDOCSENV\"."

source $UBPORTSDOCSENV/bin/activate
else
echo_warning "Build environment not found in \"$UBPORTSDOCSENV\" ... creating it."
echo_warning "Creating and activating a virtual build environment in \"$UBPORTSDOCSENV\"."

virtualenv $UBPORTSDOCSENV
source $UBPORTSDOCSENV/bin/activate

echo_warning "Installing build tools and prerequisites (using pip)."

pip3 install -r requirements.txt
fi
}
#endregion PREPARE_VIRTUALENV

#region BUILD
wrapper_build_documentation(){
rm -rf _build/
if [ "$(uname)" == "Linux" ]; then
sphinx-build -Wa . _build/html -j `nproc --all` || exit $?
else
sphinx-build -Wa . _build/html -j `sysctl -n hw.ncpu` || exit $?
fi
sphinx-build -Wab rediraffecheckdiff . _build/html
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this also || exit ?

That should also allow you to put a echo_success after

}
build_documentation(){
echo_success "Building..."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Plain


if [ "$verbose" = true ]; then
wrapper_build_documentation
else
wrapper_build_documentation >/dev/null 2>&1
fi

echo_plain "Open _build/html/index.html in your browser to navigate through the docs"
}
#endregion BUILD

#endregion

install_dependencies_for_most_linux_systems
prepare_virtualenv_for_python
build_documentation