fix: Use Magisk mirror only when it is really possible
Updated the mount script's magisk mirror check, because when using the mount argument via the CLI, the mount script fails with a 'No such file or directory' error.
Haven't used magisk for a few years now, but their docs suggest current implementation is sound.
If it's really the mirror that's not existing, then either
- we should raise it w/ Magisk project, or;
- make the check safer by adding to line
#58something like:[ -e "${'$'}MIRROR" ] || unset MIRROR
In general what should the mirror folder contain? In the docs it says it is used as Partition mirrors, but as for me it is empty..
Shouldn't we check if it is there AND has content?
Currently it is checked as a FILE not a directory too, this code snippet was copied from the revanced-manager's mount script.
It is empty for me as well. Not sure why it is empty though. I guess we can check the contents of mirror folder. If it is empty, unset it. Please move the code block back to its original location at the bottom though.
As a temp fix for my own project, I added a check for that, if it's good maybe I can add it to the code? I check if the mirror folder don't exists or if it is, but it's empty
MAGISKTMP="$(magisk --path)" || MAGISKTMP=/sbin
MIRROR="$MAGISKTMP/.magisk/mirror"
if [ ! -d "$MIRROR" ] || [ -z "$(ls -A "$MIRROR" 2>/dev/null)" ]; then
MIRROR=""
fi
Can't you just check if it is not empty? Because if it doesn't exist, then it will return false anyways
Ohh that's true, then the folder check is not required here, then using
if [ -z "$(ls -A "$MIRROR" 2>/dev/null)" ]; then
should be enough, no?
Or is there a better way to check if it's empty?
is_dir_empty() {
find -L "$1" -mindepth 1 -maxdepth 1 -print -quit | grep -q .
[ $? -eq 1 ]
}
should be enough, no?
I guess so
Should I update the code to use the folder contents check instead of the folder exists check?
yes
Added back check for magisk installation (https://github.com/ReVanced/revanced-library/pull/67/commits/0820352259ff57df9ed6d340cd4bd24ebb12c470), because using this script with other root methods would fail, removing it was my bad.
Also probably would have to fix RV Manager too, as there is no check for magisk installation either, and fix check at Line 138 too. Should I create a PR for that?