improve installer script

[laur89]

• replace tabs w/ spaces
• quote vars
• replase dash syntax with that of bash (e.g. [[ "$a" || "$b" ]]) as opposed to [ $a -o $ b ]
• fail wast where command returns non-zero

[laur89]

Is this ok? Unsure if $AUTO_CPUFREQ_FILE" --remove is allowed to exit non-zero in normal operation or not.

[AdnanHodzic]

Removing eval makes sense in this case. But potential problem is that if $AUTO_CPUFREQ_FILE --remove returns a non-zero exit code when the daemon isn't installed, script will prematurely exit with an error, even though the desired final state (daemon removed) has been achieved.

Maybe it would make sense to log it while allowing the removal command to fail silently (which is often acceptable for removal steps), with something like:

if [[ -f "$srv_remove" || -f "$AUTO_CPUFREQ_FILE" ]]; then
    "$AUTO_CPUFREQ_FILE" --remove || echo "\nWARNING: Removal of auto-cpufreq binary failed (exit status $?).\n Continuing ..." >&2
fi

[AdnanHodzic]

Sorry to nitpick, but I think this might confuse some users now, as in case auto-cpufreq daemon is installed and you simply run sudo ./auto-cpufreq-installer --remove it's all clear when it comes to messaging.

But if daemon is not installed and you run sudo ./auto-cpufreq-installer --remove, output can be bit confusing:

auto-cpufreq daemon is not installed.


WARNING: Removal of auto-cpufreq binary failed (exit status 1).
 Continuing...

auto-cpufreq tool and all its supporting files successfully removed

As Removal of auto-cpufreq binary failed ... it didn't fail, it just wasn't there at this point ... and this is indicated as part of previous message, so maybe good to remove:

Continuing...

after all.

Regarding whole block:

  # run uninstall in case of installed daemon
 if [[ -f "$srv_remove" || -f "$AUTO_CPUFREQ_FILE" ]]; then
   "$AUTO_CPUFREQ_FILE" --remove || echo -e "\nWARNING: Removal of auto-cpufreq binary failed (exit status $?).\n Continuing..." >&2
 else
   echo; echo "Couldn't remove the auto-cpufreq daemon, [$srv_remove] does not exist."
 fi

Basically else echo; echo "Couldn't remove the auto-cpufreq daemon, [$srv_remove] does not exist." is confusing and potentially misleading. The if check uses two files ($srv_remove or $AUTO_CPUFREQ_FILE), but the else message only mentions $srv_remove not existing. This message is only accurate if the if statement failed because $srv_remove didn't exist AND $AUTO_CPUFREQ_FILE also didn't exist. The message should ideally reflect the combined condition failure or be more generic.

[AdnanHodzic]

I guess copilot was kind of referring to same thing.

[laur89]

Well that was the very reason that place got flagged -- I'm not familiar with the intricacies of $AUTO_CPUFREQ_FILE itself and would like some guidance/help there.

[AdnanHodzic]

AUTO_CPUFREQ_FILE is simply a auto-cpufreq binary, placed on that location after ./auto-cpufreq-installer --install. After which you can run install or live, etc ...

When it comes to srv_remove and what I see on master branch, this change was introduced with with #695, a refactor attempt I should've payed more attention during review and probably shouldn't have merged.

But before this change, srv_remove pointed to srv_remove="/usr/local/bin/auto-cpufreq-remove"..

I can't remember what was the case here, but I think this was because these were 2 different scripts (before they were merged into single install script with parameters), one for removal and other for install:

• srv_install="/usr/local/bin/auto-cpufreq-install"
• srv_remove="/usr/local/bin/auto-cpufreq-remove"

So I think this variable can even be removed now, as it doesn't point to anything. After auto-cpufreq is installed with auto-cpufreq-installer it'll only have 2 binaries in /usr/local/bin/

• auto-cpufreq
• auto-cpufreq-gtk

[Copilot]

Pull Request Overview

This PR modernizes the auto-cpufreq-installer bash script by improving code quality, consistency, and error handling. The changes focus on adopting bash best practices, improving script robustness, and ensuring proper error propagation throughout the installation and removal processes.

Key changes:

• Standardized quoting of variables and updated syntax to modern bash conventions (e.g., [[ over [)
• Added set -o pipefail and || exit 1 error handling to fail fast on command failures
• Replaced tabs with spaces and corrected variable names (COLOUMNS → COLUMNS)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The error message is misleading. It suggests that $srv_remove doesn't exist, but the condition on line 223 checks for the existence of either $srv_remove OR $AUTO_CPUFREQ_FILE. Consider clarifying: "Couldn't remove the auto-cpufreq daemon, required files do not exist."

Suggested change

	echo; echo "Couldn't remove the auto-cpufreq daemon, [$srv_remove] does not exist."
	echo; echo "Couldn't remove the auto-cpufreq daemon, required files do not exist."

[AdnanHodzic]

@laur89 also please check the Copilot suggestions.