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.

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

Provide better output on toltecctl uninstall #692

Open
wants to merge 36 commits into
base: testing
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
f0c8013
Provide better output on toltecctl uninstall
Eeems May 10, 2023
71c5462
Update version
Eeems May 10, 2023
46ab86b
Remove extra whitespace left by the github editor
Eeems May 10, 2023
99fb2f6
Make variable local
Eeems May 10, 2023
c3dba52
No longer use a subshell
Eeems May 10, 2023
1649a09
Merge branch 'testing' into Eeems-patch-10
Eeems May 10, 2023
f10380a
Merge branch 'testing' into Eeems-patch-10
Eeems Aug 2, 2023
25372bd
Merge branch 'testing' into Eeems-patch-10
Eeems Aug 2, 2023
f45e08b
Merge branch 'testing' into Eeems-patch-10
Eeems Aug 8, 2023
03d090e
Merge branch 'testing' into Eeems-patch-10
Eeems Aug 15, 2023
90d6117
Merge branch 'testing' into Eeems-patch-10
Eeems Oct 3, 2023
ecee049
Merge branch 'testing' into Eeems-patch-10
Eeems Nov 27, 2023
325cd4d
Fix toltec-bootstrap version
Eeems Nov 27, 2023
8937f96
Merge branch 'testing' into Eeems-patch-10
Eeems Dec 6, 2023
038d359
Merge branch 'testing' into Eeems-patch-10
Eeems May 8, 2024
b7e1753
Merge branch 'testing' into Eeems-patch-10
Eeems May 31, 2024
44228c5
Merge branch 'testing' into Eeems-patch-10
Eeems May 31, 2024
0e024aa
Merge branch 'testing' into Eeems-patch-10
Eeems Jun 2, 2024
94af3c3
Add missing dependency to launcherctl
Eeems Jun 2, 2024
08c582d
Merge branch 'testing' into Eeems-patch-10
Eeems Jun 2, 2024
ea59852
Bump toltec-bootstrap version
Eeems Jun 2, 2024
fcedcd4
Force launcherctl to uninstall before xochitl
Eeems Jun 2, 2024
2645de9
Make uninstall way more fault tolerant, and report better errors
Eeems Jun 2, 2024
a11076d
Fix error line output
Eeems Jun 2, 2024
9c24c05
Better handle failed uninstalls
Eeems Jun 2, 2024
c96df00
Add extra log line
Eeems Jun 2, 2024
30663b7
Fix entware-rc depends
Eeems Jun 2, 2024
dcf6d73
fix final exit
Eeems Jun 2, 2024
52b100e
Merge branch 'testing' into Eeems-patch-10
Eeems Jun 3, 2024
96720a3
Merge branch 'testing' into Eeems-patch-10
Eeems Jun 6, 2024
8deff1c
Update toltecctl
Eeems Sep 7, 2024
7604217
Merge branch 'testing' into Eeems-patch-10
Eeems Sep 7, 2024
54d5dcb
Update toltecctl
Eeems Sep 7, 2024
3cd792e
Merge branch 'testing' into Eeems-patch-10
Eeems Sep 12, 2024
3e5c0a1
Merge branch 'testing' into Eeems-patch-10
Eeems Sep 18, 2024
118c9a1
Merge branch 'testing' into Eeems-patch-10
Eeems Oct 12, 2024
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
2 changes: 1 addition & 1 deletion package/launcherctl/launcherctl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ help() {
}
data_dir="/opt/share/launcherctl"
launchers() {
/opt/bin/find "$data_dir" -type f -perm '-u+x' | xargs -rn1 basename
/opt/libexec/find-gnu "$data_dir" -type f -perm '-u+x' | xargs -rn1 basename
}
active_launchers() {
launchers | while read -r launcher; do
Expand Down
3 changes: 2 additions & 1 deletion package/launcherctl/package
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
pkgnames=(launcherctl)
pkgdesc="Manage your installed launcher"
url=https://toltec-dev.org/
pkgver=0.0.1-2
pkgver=0.0.2-2
timestamp=2023-12-18T03:32Z
section="launcher"
maintainer="Eeems <[email protected]>"
license=MIT
installdepends=('findutils' 'xochitl')

source=(
launcherctl
Expand Down
2 changes: 1 addition & 1 deletion package/toltec-bootstrap/package
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
pkgnames=(toltec-bootstrap)
pkgdesc="Manage your Toltec install"
url=https://toltec-dev.org/
pkgver=0.4.4-1
pkgver=0.4.5-1
timestamp=2024-05-31T19:13Z
section="utils"
maintainer="Eeems <[email protected]>"
Expand Down
148 changes: 122 additions & 26 deletions package/toltec-bootstrap/toltecctl
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

set -euo pipefail

# Path to this script
toltecctl_path=$(realpath "$0")

# Path where Toltec resides (will be mounted to $toltec_dest)
toltec_src=/home/root/.entware

Expand All @@ -15,7 +18,7 @@ toltec_share=/home/root/.local/share/toltec
# Path to static opkg build
opkg_path=/home/root/.local/bin/opkg
# Path to opkg install status file
opkg_status="/opt/lib/opkg/status"
opkg_status="$toltec_dest/lib/opkg/status"

# Path to Opkg configuration
opkg_conf="$toltec_src"/etc/opkg.conf
Expand Down Expand Up @@ -123,7 +126,7 @@ get-release-version() {
# 3 - unable to install standalone wget
check-version() {
local wget
if [[ "$(install-state)" != "yes" ]] || [[ "$(command -v wget)" != "/opt/bin/wget" ]]; then
if [[ "$(install-state)" != "yes" ]] || [[ "$(command -v wget)" != "$toltec_dest/bin/wget" ]]; then
if ! install-standalone-wget; then
return 2
fi
Expand Down Expand Up @@ -197,9 +200,9 @@ add-bind-mount() {
unit_name="$(basename "$unit_path")"

if [[ -e $unit_path ]]; then
echo "Bind mount configuration for '$2' already exists, updating"
log INFO "Bind mount configuration for '$2' already exists, updating"
else
echo "Mounting '$1' over '$2'"
log INFO "Mounting '$1' over '$2'"
fi

cat > "$unit_path" << UNIT
Expand Down Expand Up @@ -234,13 +237,14 @@ remove-bind-mount() {
unit_name="$(basename "$unit_path")"

if [[ ! -e $unit_path ]]; then
echo "No existing bind mount for '$1'"
log INFO "No existing bind mount for '$1'"
return
fi

echo "Removing mount over '$1'"
log INFO "Removing mount over '$1'"
systemctl disable "$unit_name"
systemctl stop "$unit_name"
# Don't error, as umount -l will handle the failure
systemctl stop "$unit_name" || true
if mountpoint -q "$1"; then
umount -l "$1"
fi
Expand Down Expand Up @@ -460,7 +464,7 @@ reinstall-root() {
if [[ -v "on_root_packages[$pkgname]" ]]; then
reinstall_packages[$pkgname]=1
fi
done < <(gunzip -c /opt/var/opkg-lists/* | grep "^Package:" | awk '{print $2}')
done < <(gunzip -c $toltec_dest/var/opkg-lists/* | grep "^Package:" | awk '{print $2}')
Eeems marked this conversation as resolved.
Show resolved Hide resolved

# Workaround: Checking the size of an empty array when the nounset option
# is active may throw an error on some Bash versions, so we disable it
Expand All @@ -480,6 +484,7 @@ clean-path() {
sed -i "/^$bashrc_start_marker\$/,/^$bashrc_end_marker\$/d" "$bashrc_path"
sed -i "/^$bashrc_old_start_marker\$/!b;n;d" "$bashrc_path"
sed -i "/^$bashrc_old_start_marker\$/d" "$bashrc_path"
# TODO - rewrite this to use $toltec_dest instead of /opt
sed -i '/^\(export \)\?PATH="\?\.*\/opt\/bin:\/opt\/sbin.*"\?$/d' "$bashrc_path"
fi
}
Expand All @@ -492,7 +497,7 @@ set-path() {
clean-path
cat >> "$bashrc_path" << SHELL
$bashrc_start_marker
PATH="/opt/bin:/opt/sbin:/home/root/.local/bin:\$PATH"
PATH="$toltec_dest/bin:$toltec_dest/sbin:/home/root/.local/bin:\$PATH"
$bashrc_end_marker
SHELL

Expand Down Expand Up @@ -541,9 +546,9 @@ generate-opkg-conf() {
# then run \`toltecctl generate-opkg-conf\` to regenerate this file

dest root /
dest ram /opt/tmp
lists_dir ext /opt/var/opkg-lists
option tmp_dir /opt/tmp
dest ram $toltec_dest/tmp
Eeems marked this conversation as resolved.
Show resolved Hide resolved
lists_dir ext $toltec_dest/var/opkg-lists
option tmp_dir $toltec_dest/tmp

CONF

Expand Down Expand Up @@ -672,23 +677,23 @@ CONF

# Re-enable Toltec install after system update
reenable() {
log INFO "Mounting /opt"
log INFO "Mounting $toltec_dest"
add-bind-mount "$toltec_src" "$toltec_dest"
switch-branch "$(get-branch)"
log INFO "Generating /opt/etc/opkg.conf"
log INFO "Generating $toltec_dest/etc/opkg.conf"
generate-opkg-conf || true
log INFO "Opkg update"
opkg update
log INFO "Reinsalling base packages"
reinstall-base
log INFO "Reinstalling packages with files on the root partition"
reinstall-root
if [ -d /opt/share/toltec/reenable.d ]; then
find /opt/share/toltec/reenable.d -maxdepth 1 -mindepth 1 -print0 \
if [ -d $toltec_dest/share/toltec/reenable.d ]; then
find $toltec_dest/share/toltec/reenable.d -maxdepth 1 -mindepth 1 -print0 \
| xargs -0rn1 basename \
| while read -r pkg; do
local script
script="/opt/lib/opkg/info/${pkg}.postinst"
script="$toltec_dest/lib/opkg/info/${pkg}.postinst"
if [ -f "$script" ]; then
log INFO "Reconfiguring ${pkg}"
"$script" configure || true
Expand Down Expand Up @@ -720,7 +725,7 @@ list-installed-ordered() {
# Install standalone opkg binary
install-standalone-opkg() {
local wget
if [[ "$(install-state)" != "yes" ]] || [[ "$(command -v wget)" != "/opt/bin/wget" ]]; then
if [[ "$(install-state)" != "yes" ]] || [[ "$(command -v wget)" != "$toltec_dest/bin/wget" ]]; then
if ! install-standalone-wget; then
return 2
fi
Expand Down Expand Up @@ -775,31 +780,122 @@ install-standalone-wget() {

# Remove Toltec completely
uninstall() {
if ! [ -d "$toltec_src" ] && ! [ -d "$toltec_dest" ]; then
log INFO "Toltec does not appear to be installed"
exit 0
fi

# Fetch standalone opkg used to uninstall packages
if ! install-standalone-opkg; then
return 1
fi
local clean=true
local success=true
local complete=false

broken-state() {
echo " Your system may be in a broken state. Please review the logs"
echo " before rebooting your device. If you need assistance, you can"
echo " get help on the community discord channel."
}

handle-error() {
local error_code=$?
log ERROR "line $1: $BASH_COMMAND"
log ERROR "Uninstall did not complete properly"
broken-state
exit $error_code
}
set -o functrace
trap 'handle-error ${LINENO}' ERR
handle-exit() {
if ! $completed && $success && $clean; then
Copy link

Choose a reason for hiding this comment

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

completed vs complete, wrong variable names

Copy link
Member Author

Choose a reason for hiding this comment

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

You seem to be reviewing an outdated version, this is $complete for me when I look

Copy link

Choose a reason for hiding this comment

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

Ok, I'm reviewing patches in order, not a full diff. I'm more used to a workflow where a PR is force updated instead of incremental patches and a squash afterwards.

Copy link

@Havner Havner Sep 7, 2024

Choose a reason for hiding this comment

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

Also I think there is operator order issue here:

$ export VAR1=true
$ export VAR2=true
$ export VAR3=true
$ if ! $VAR1 && $VAR2 && $VAR3; then echo failed; fi
$ export VAR2=false
$ if ! $VAR1 && $VAR2 && $VAR3; then echo failed; fi
$ if ! ($VAR1 && $VAR2 && $VAR3); then echo failed; fi
failed

Copy link
Member Author

Choose a reason for hiding this comment

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

Since they are declared as local it could be that they aren't working as expected. Although when I tested this originally it was working as I expected.

Copy link

Choose a reason for hiding this comment

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

I don't think local makes any difference here, ! has higher priority than && hence it applies only to VAR1 and from the look of it you wanted to apply it to the whole expression (V1 && V2 && V3)

Copy link
Member Author

Choose a reason for hiding this comment

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

I would have wrapped it in ( and ) if I was trying to apply it to all of them, I'm checking if not complete, and success and clean. success/clean are true by default and get changed to false when something else handles reporting an error.

Copy link

Choose a reason for hiding this comment

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

Ah, ok then. From reading the code it looked to me like you are looking for a failure scenario (e.g. at least one of those vars is false).

Copy link
Member Author

Choose a reason for hiding this comment

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

In this case, I'm only wanting to handle the error if the error handler is triggered and we did not complete the uninstall, and nothing set success or clean to false due to their own error handling.

It is less than ideal, thus the extra comments on the variable declarations. I couldn't really come up with a cleaner way to handle this with the limitations bash has on error handling.

Copy link

Choose a reason for hiding this comment

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

I would have to analyze this fully to check the logic completely. I'm too tired now.

What I did find though is that you make a dep between clean and success so checking both in this line might be at least redundant.

https://github.com/toltec-dev/toltec/blob/Eeems-patch-10/package/toltec-bootstrap/toltecctl#L885

But I'm not able to check the logic fully now.

log ERROR "Uninstall did not complete properly"
broken-state
exit 1
fi
}
trap 'handle-exit' EXIT

if [[ "$toltecctl_path" != *_backup ]]; then
log INFO "Creating backup of toltecctl at ${toltecctl_path}_backup"
rm -f "${toltecctl_path}_backup"
cp "$toltecctl_path" "${toltecctl_path}_backup"
fi

if ! [ -f "$toltec_dest"/etc/opkg.conf ]; then
if [ -f "$toltec_src"/etc/opkg.conf ] && ! mountpoint -q "$toltec_dest"; then
log INFO "Mounting $toltec_dest"
add-bind-mount "$toltec_src" "$toltec_dest"
fi
log ERROR "Current install seems to be partially removed, unable to automatically removed."
broken-state
complete=true
exit 1
fi

# Remove installed packages in reverse dependency order
list-installed-ordered | while read -r pkgname; do
"$opkg_path" remove --force-removal-of-essential-packages --force-depends --force-remove "$pkgname"
done
while read -r pkgname; do
if ! "$opkg_path" remove --force-depends "$pkgname"; then
echo "Failed to remove $pkgname, forcing removal..."
"$opkg_path" remove --force-removal-of-essential-packages --force-depends --force-remove "$pkgname"
clean=false
fi
done << EOF
$(list-installed-ordered)
EOF

systemctl daemon-reload
if ! systemctl daemon-reload; then
log WARN "Failed to reload systemd state"
fi
rm -f "$opkg_path"

# Remove mount point
set +e # Don't exit on error, next check will validate if it worked
remove-bind-mount "$toltec_dest"
rmdir "$toltec_dest"
set -e

case "$(install-state)" in
no) ;;
*)
log ERROR "$toltec_dest bind mount still is active"
success=false
;;
esac

if ! rmdir "$toltec_dest"; then
log ERROR "Failed to remove mount point"
success=false
fi

# Unset PATH
clean-path

# Remove Toltec data
rm -r "$toltec_src"
if ! rm -r "$toltec_src"; then
log ERROR "Failed to remove toltec data directory"
success=false
fi

# Re-enable xochitl if needed
systemctl enable xochitl
if ! systemctl enable xochitl; then
log ERROR "Failed to enable the user interface."
success=false
fi
if ! $clean; then
log ERROR "There were errors removing some of the toltec packages."
success=false
fi
if ! $success; then
log ERROR "Uninstall failed"
broken-state
complete=true
exit 1
fi
if [[ "$toltecctl_path" != *_backup ]]; then
rm -f "$toltecctl_path"
fi
rm -f "${toltecctl_path}_backup"
complete=true
}

# The current toltec install state
Expand Down
Loading