rename read_packages_file to pkgapp_packages_required to better suite function

[theofficialgman]

Replacement to #2557

[Botspot]

I mean... if you want to rename the function we could do that.
But please still look at #2557 as it does a lot more than tweak the output of read_packages_file in edge cases.

[theofficialgman]

additionally solves:

• If the user manually tries installing a hidden package-app using manage, the error from apt will be helpful, rather than saying no packages were specified.

[theofficialgman]

additionally solves:

• if both OR packages are available but the second one is installed and not the first, show as installed instead of uninstalled

[theofficialgman]

I do not think we can go ahead with this PR (or the other one) unless this problem is solved

test1(){
> test2(){
> echo hi
> }
> }
gman@gman-jam:~/pi-apps$ test1
gman@gman-jam:~/pi-apps$ test2
hi

f407661
functions are not local and there appears to be no way in bash to make them so. the package_available and package_installed functions get replaced after using the function that re-delares them

[Botspot]

I will check for solutions, but there is always the subshell option

[theofficialgman]

I will check for solutions, but there is always the subshell option

yeah I think that is what we will end up needing to do
define the functions and run refresh_pkgapp_status in a subshell together

[Botspot]

Good catch

pi@raspberrypi:~ $ DIRECTORY=$HOME/pi-apps
pi@raspberrypi:~ $ source ~/pi-apps/api
pi@raspberrypi:~ $ package_available chromium-browser
pi@raspberrypi:~ $ echo $?
0
pi@raspberrypi:~ $ refresh_all_pkgapp_status
Marking Pinta as hidden
Marking Steam Link as hidden
pi@raspberrypi:~ $ package_available chromium-browser
pi@raspberrypi:~ $ echo $?
1

[theofficialgman]

I went with the subshell method. doesn't seem to hurt performance. if you find anything else that you like better feel free to implement

[Botspot]

I thought of saving original values of the functions and setting them back, but a subshell seems more straightforward and safe.

[Botspot]

I'm thinking of doing a squash-merge unless you want to do another force push to cut down on commit count.

[theofficialgman]

I'll rewrite the history tomorrow so it makes more sense and merge it (with no actual code changes besides maybe adding -qq since this doesn't have it).

I'd rather leave this tonight so I can sleep on it and then check over again tomorrow.

[theofficialgman]

also I just want to make it clear. I liked most of the changes from your PR (which is why I integrated them here, anything you made I'll make sure has a coauthor on it). There were just some things I had issues with.

Clearly our best work is when its done together or iterating on each other.

[Botspot]

also I just want to make it clear. I liked most of the changes from your PR (which is why I integrated them here, anything you made I'll make sure has a coauthor on it). There were just some things I had issues with.

Understand.

Clearly our best work is when its done together or iterating on each other.

For sure. Pi-Apps would just be another PiKISS if it was not for teamwork.

[Botspot]

• if both OR packages are available but the second one is installed and not the first, show as installed instead of uninstalled

I just thought of a scenario where your solution to this is likely insufficient.

• Imagine I am on a system where both chromium and chromium-browser are available.
• The packages file contains chromium-browser | chromium as usual.
• chromium is installed.
• pi-apps needs to update Chromium (it should never need to do this - this is theoretical)
• Pi-Apps uninstalls chromium because pkgapp_packages_required told apt to purge chromium.
• Pi-Apps installs back chromium-browser because pkgapp_packages_required now uses the first available package in the OR list as neither one are installed at the time of its second execution.

[theofficialgman]

• if both OR packages are available but the second one is installed and not the first, show as installed instead of uninstalled

I just thought of a scenario where your solution to this is likely insufficient.

• Imagine I am on a system where both chromium and chromium-browser are available.
• The packages file contains chromium-browser | chromium as usual.
• chromium is installed.
• pi-apps needs to update Chromium (it should never need to do this - this is theoretical)
• Pi-Apps uninstalls chromium because pkgapp_packages_required told apt to purge chromium.
• Pi-Apps installs back chromium-browser because pkgapp_packages_required now uses the first available package in the OR list as neither one are installed at the time of its second execution.

your scenario hinges on this pi-apps needs to update Chromium which as you say cannot happen. it is easy to break many of the functions of pi-apps with output or actions that cannot occur.

if it is desirable that updates to package apps retain the previously installed packages (assuming those packages are still in the new packages file), then that should be handled in the updater for package apps, not the pkgapp_packages_required call. that means modifying update_app which is ends up being called for updating apps via all methods (gui, manage, cli, etc):

pi-apps/updater

Lines 436 to 488 in 2c32783

	update_app() { #first arg is app name
	local app="$1"
	[ -z "$app" ] && error "update_app(): no app specified!"
	status "Updating \e[1m${app}\e[0m\e[96m..."
	echo
	#Set terminal title
	echo -ne "\e]0;Updating ${app}\a"
	# check if update app folder exists before doing anything
	# it can happen that executing the updater from the pi-apps GUI the update folder is missing
	# if the user has no internet or internet issues the update folder will be removed due to the failed git pull and then wait in the loop for a new git clone
	# the GUI updater executes check_repo with the "fast" option so it skips updating the pi-apps update folder and obtains any previously determined updatable apps and files
	# if we do not check for this then apps will be removed, the older version moved to trash, and then the new version will fail to copy over since it does not exist
	if [ -d "${DIRECTORY}/update/pi-apps/apps/${app}" ]; then
	local installback=no
	if will_reinstall "$app";then
	installback=yes
	status "$app's install script has been updated. Reinstalling $app..."
	#uninstall it
	"${DIRECTORY}/manage" uninstall "$app" update #report to the app uninstall script that this is an uninstall for the purpose of updating by passing "update"
	#fix edge case: if app is installed but uninstall script doesn't exist somehow, then pretend app was uninstalled so that the reinstall later will happen noninteractively
	if [ "$(app_status "$app")" == installed ];then
	echo 'uninstalled' > "${DIRECTORY}/data/status/${app}"
	fi
	fi
	no_status=true refresh_app "$app"
	failed=false
	if [ "$installback" == 'yes' ];then
	#install the app again
	"${DIRECTORY}/manage" install "$app" update #report to the app install script that this is an install for the purpose of updating by passing "update"
	if [ $? != 0 ]; then
	failed=true
	else
	# click update link only if app is already installed and the update succeeded
	shlink_link "$app" update &
	fi
	fi
	else
	failed=true
	fi
	if [ "$failed" == 'true' ]; then
	echo -e "\e[91mFailed to update ${app}.\e[0m"
	return 1
	else
	status_green "${app} was updated successfully."
	return 0
	fi
	}

alternatively, you could modify manage uninstall to not uninstall package apps when triggered as part of an update (reminder that the 3rd argument update is passed to manage when triggered as part of an update). instead of uninstalling, mark the packages as automatically installed (apt-mark auto) (instead of manually installed which is their current status). then, during the manage install, the packages will still be installed (but will get autoremoved during any apt autoremove command). that will result in pkgapp_packages_required returning the desired chromium output since it is still installed. manage install can continue as normal and (optionally) you could add an apt autoremove (or add --autoremove as a flag during the apt install) to remove any packages that were marked as automatically installed during manage uninstall that were not marked as manually installed during manage install (eg: like would be desirable for a package that got removed from the packages list during the update)

pi-apps/manage

Lines 596 to 742 in 2c32783

	elif [ "$1" == 'install' ] || [ "$1" == 'uninstall' ];then
	#for this operation, a program name must be specified.
	app="$2"
	if [ -z "$app" ];then
	error "For this operation, you must specify which app to operate on."
	elif [ ! -d "${DIRECTORY}/apps/$app" ];then
	error "${DIRECTORY}/apps/$app does not exist!"
	fi
	if [ "$1" == install ];then
	action=install
	else
	action=uninstall
	fi
	if [ "$action" == install ];then
	#check for internet connection
	errors="$(command wget --spider https://github.com 2>&1)"
	if [ $? != 0 ];then
	error "No internet connection! (github.com failed to respond)\nErrors:\n$errors"
	fi
	fi
	#ensure not a disabled app
	if [ "$action" == install ] && [ "$(app_status "${app}")" == 'disabled' ];then
	warning "Not installing the $app app. IT IS DISABLED."
	exit 0
	fi
	#determine path for log file to be created
	logfile="${DIRECTORY}/logs/${action}-incomplete-${app}.log"
	if [ -f "$logfile" ] || [ -f "$(echo "$logfile" | sed 's+-incomplete-+-success-+g')" ] || [ -f "$(echo "$logfile" | sed 's+-incomplete-+-fail-+g')" ];then
	#append a number to logfile's file-extension if the original filename already exists
	i=1
	while true;do
	#if variable $i is 2, then example newlogfile value: /path/to/install-Discord.log2
	newlogfile="$logfile$i"
	if [ ! -f "$newlogfile" ] && [ ! -f "$(echo "$newlogfile" | sed 's+/-incomplete-+-success-+g')" ] && [ ! -f "$(echo "$newlogfile" | sed 's+-incomplete-+-fail-+g')" ];then
	logfile="${newlogfile}"
	break
	fi
	i=$((i+1))
	done
	fi
	#display warning if hardware and os is unsupported
	if [ "$supported" == no ];then
	warning "YOUR SYSTEM IS UNSUPPORTED:\n$unsupported_message" 2>&1 | tee -a "$logfile"
	sleep 1
	echo -e "\e[103m\e[30mThe ability to send error reports has been disabled.\e[39m\e[49m" | tee -a "$logfile"
	sleep 1
	echo -e "\e[103m\e[30mWaiting 10 seconds... (To cancel, press Ctrl+C or close this terminal)\e[39m\e[49m" | tee -a "$logfile"
	sleep 10
	fi
	#if this app has scripts, determine which script to run
	if [ "$(app_type "$app")" == standard ];then
	if [ "$action" == install ];then
	scriptname="$(script_name_cpu "$app")" #will be install, install-32, or install-64
	if [ -z "$scriptname" ];then
	error "It appears $app does not have an install-${arch} script suitable for your ${arch}-bit OS." | tee -a "$logfile"
	exit 1
	fi
	else #uninstall mode
	scriptname=uninstall
	fi
	appscript=("${DIRECTORY}/apps/${app}/${scriptname}")
	chmod u+x "$appscript" &>/dev/null
	#if this app just lists a package-name, set the appscript to install that package
	else #package-app: directly use apt to install what is mentioned in the packages file
	packages_to_install=$(pkgapp_packages_required "$app")
	[ -z "$packages_to_install" ] && error "It appears $app does not have any packages that can be installed on your system."
	appscript=(bash -c -o pipefail "apt_lock_wait ; sudo -E apt $(echo "$action" | sed 's/uninstall/purge --autoremove/g') -yf $packages_to_install 2>&1 | less_apt")
	#fix edge case: new will_reinstall function avoids a reinstall if packages to install do not change.
	#unfortunately updater script does not source the new api so chromium is being reinstalled after we added "| chromium" to the packages file.
	#fix it here because new manage script sources the new api
	if [ "$3" == "update" ] && ! will_reinstall "$app" ;then
	#manage was told to reinstall app, but it does not actually need to. Do nothing
	appscript=(bash -c -o pipefail "status 'Not reinstalling $app as no changes would be made.\nYou should only see this message once. Exiting.'")
	fi
	fi
	#print to terminal
	status "${action^}ing \e[1m${app}\e[0m\e[96m..." | tee -a "$logfile"
	echo
	cd $HOME
	if [ "$3" == "update" ]; then
	script_input="update"
	else
	script_input=""
	fi
	nice "${appscript[@]}" "$script_input" &> >(tee -a "$logfile")
	exitcode="${PIPESTATUS[0]}"
	#if app succeeded
	if [ $exitcode == 0 ];then
	#Contribute to app install/uninstall count as long as parent processes is NOT updater (during an app-reinstall)
	#See: https://askubuntu.com/a/1012236
	if [ "$(cat /proc/$PPID/comm)" != "updater" ] && [ "$3" != "update" ];then
	shlink_link "$app" "$action" &
	fi
	status_green "\n${action^}ed ${app} successfully." | tee -a "$logfile"
	echo "${action}ed" > "${DIRECTORY}/data/status/${app}"
	format_logfile "$logfile" #remove escape sequences from logfile
	mv "$logfile" "$(echo "$logfile" | sed 's+-incomplete-+-success-+g')" #rename logfile to indicate it was successful
	else #if app failed to install/uninstall
	#remove dummy deb if app failed to install (to avoid dummy debs being left on a users install for broken apps)
	package_name="$(app_to_pkgname "$app")"
	if [[ ${action} == "install" ]] && [[ "$(app_type "$app")" == standard ]] && package_installed "$package_name"; then
	#Run purge_packages
	echo $'\n'"Running purge_packages..." >> "$logfile"
	purge_packages 2>&1 | tee -a "$logfile" >/dev/null
	fi
	unset package_name
	echo -e "\n\e[91mFailed to ${action} ${app}!\e[39m
	\e[40m\e[93m\e[5m◢◣\e[25m\e[39m\e[49m\e[93mNeed help? Copy the \e[1mENTIRE\e[0m\e[49m\e[93m terminal output or take a screenshot.
	Please ask on Github: \e[94m\e[4mhttps://github.com/Botspot/pi-apps/issues/new/choose\e[24m\e[93m
	Or on Discord: \e[94m\e[4mhttps://discord.gg/RXSTvaUvuu\e[0m" | tee -a "$logfile"
	#set the app's status to 'corrupted' if the diagnostics determine the error is NOT internet or system or package, AND if the app is a script-type app
	if ! [[ "$(log_diagnose "$logfile" | head -n1)" =~ ^(system|internet|package)$ ]] && [ "$(app_type "$app")" == standard ];then
	echo "corrupted" > "${DIRECTORY}/data/status/${app}"
	fi
	format_logfile "$logfile" #remove escape sequences from logfile
	mv "$logfile" "$(echo "$logfile" | sed 's+-incomplete-+-fail-+g')" #rename logfile to indicate it was unsuccessful
	fi
	#if the app is a package, set its status to whatever dpkg thinks it is
	if [ "$(app_type "$app")" == package ];then
	refresh_pkgapp_status "$app"
	fi
	#exit the manage script with the same exit-code of the app's script
	exit $exitcode