Fix: handle legacy files in NO_SUDO mode

[AhmedYasserrr]

Fix issue 132: handle legacy files in NO_SUDO mode

---

Summary

This pull request improves the installer to properly handle legacy files when --no-sudo mode is used. It prevents permission errors by detecting global legacy files or system-installed Python packages that cannot be removed without root privileges. In such cases, the installer aborts and provides clear instructions to the user on how to resolve the situation.

---

Changes Introduced

• In NO_SUDO=true mode:
  • Detects global legacy binaries and service files.
  • Detects if a system-installed fw-fanctrl Python package exists.
  • Aborts gracefully and shows:
    • A suggested command to remove the files: sudo ./install.sh --remove
    • A suggested command to uninstall any system-wide fw-fanctrl Python package: sudo python -m pip uninstall fw-fanctrl
• Prevents silent failures during local installations and keeps rootless installs safe.

---

Testing Instructions

Scenario	Expected Outcome
Install with --no-sudo and no legacy files	Installer runs successfully
Install with --no-sudo and leftover global files	Installer aborts and tells the user to use sudo ./install.sh --remove
Install with --no-sudo and system-installed fw-fanctrl	Installer aborts and suggests using sudo python -m pip uninstall fw-fanctrl
Install with full sudo permissions	Installer behaves as usual without interruptions

To test:

1. Try installing without sudo when old /usr/local/bin/fw-fanctrl exists.
2. Try installing without sudo when an old system-wide fw-fanctrl Python package is still installed.
3. Test normal installation without --no-sudo to verify backward compatibility.

---

Notes

• This PR only affects users opting for --no-sudo installations.
• Regular users with sudo workflows will not be affected.

[leopoldhub]

Hi,

Thanks for the very well documented problem and PR.

I have reviewed your implementation and suggested some changes.
If you have any questions I will be happy to answer them.

[AhmedYasserrr]

Hi @leopoldhub,
I have found another bug in NO_SUDO mode, and I am taking it slowly so you can confirm with me without creating any new bugs.

I have reverted my last two commits (matching main) and created a new one explaining the bug, which is at line:

actual_installation_path="$(which 'fw-fanctrl' 2>/dev/null)"

Since set -e is enabled, when this command returns a non-zero value, it breaks the script and stops execution.
I have disabled set -e before it and re-enabled it after.

Second Problem

The script continues execution until this line:

./post-install.sh --dest-dir "$DEST_DIR" --sysconf-dir "$SYSCONF_DIR" "$([ "$NO_SUDO" = true ] && echo "--no-sudo")"

It passes this root check in ./post-install.sh:

# Root check
if [ "$EUID" -ne 0 ] && [ "$NO_SUDO" = false ]
then
  echo "This program requires root permissions or use the '--no-sudo' option"
  exit 1
fi

Since it's using &&, the script does not stop when NO_SUDO=true and you're not a root user.
As a result, it proceeds and later fails at this line:

systemctl daemon-reload

This fails because the user is not root..

I believe this root check should be:

# Root check
if [ "$EUID" -ne 0 ] || [ "$NO_SUDO" = true]
then
  echo "This program requires root permissions or use the '--no-sudo' option"
  exit 1
fi

Behavior:

1. If not root, the script exits.
2. If NO_SUDO=true, the script exits.
3. If the user is root and NO_SUDO is false, the script continues.

However, I wanted to check with you before changing anything, since the root check is written the same way in both post-install.sh and pre-uninstall.sh.

[AhmedYasserrr]

Hi @leopoldhub,

Apologies for the repeated check, but I’m still waiting for your input regarding the root check in both post-install.sh and pre-uninstall.sh. I believe it should be updated, as I mentioned earlier. I’m just wondering if it serves any other purpose that I may have missed before editing.

[leopoldhub]

Hi, sorry for the delay.

I think the conditions are working as expected.
The script must either be run as root, or have the no-sudo argument explicitly set.

The no-sudo option is a commitment by the person or system running it to take responsibility for the permissions requirements, not a guarantee that it will work by default on the system.

For example, a user who has set their default permissions to be able to run systemctl, or a packaging script for a unified installation, may want to manage those permissions themselves.

Since systemctl requires root privileges, a "normal" user running the script manually with the no-sudo option will get an error for lack of privileges, and that's expected as there is no other possible solution in this context.
He will either have to run it as root, or disable the pre-uninstallation/post-installation scripts with the appropriate options (--no-pre-uninstall/--no-post-install) and manage the service activation externally.

I hope this was useful, please let me know if it wasn't clear or if you have any further questions.

[leopoldhub]

For the actual_installation_path issue, I will check things out asap

[AhmedYasserrr]

In the install script, if I replace uninstall_legacy with uninstall, users running no-sudo must also use --no-pre-uninstall and --no-post-install to avoid errors.

Since we know that users without sudo will eventually run into issues without --no-pre-uninstall and --no-post-install, can't we activate the flags for them:

'--no-sudo')
    NO_SUDO=true
    SHOULD_POST_INSTALL=false
    SHOULD_PRE_UNINSTALL=false
    ;;

Because we should either prevent the known error or provide a clear error message guiding users to use these flags, I prefer preventing the error.

[leopoldhub]

can't we activate the flags for them

No, as explained in my previous message, users and processes running the script with --no-sudo might still want the pre-install and post-install behaviours.

Also, the philosophy here is to do the maximum (aka attempting a full install) except when explicitly specified.

In your example, we cannot differentiate between a user/process that knows that it will not install everything and a mistake.
There is also no way we can relay efficiently to the user (or process) the info that the script did not complete as expected except by throwing an error.

We can however tell the user that it failed as executing the prev/post scripts and steps to fix it.

[AhmedYasserrr]

@leopoldhub , waiting for your review. It might seem like a lot of changes, but it's actually small changes spread across the last three commits (before the merge commit).

[leopoldhub]

Hey,

Sorry for the delay, I have been busy lately.
I left a single comment on one of your changes, after that it should be good!

[leopoldhub]

Thank you very much for your contribution,
Everything seems fine to me, I will merge this pr!🎉