Bugs related to the management of backups (crashrecovery):
1- The information displayed in "recovery dirs" section by 'psd p' is not always correct.
2- Searching for the most recent backup (between the $BACK_OVFS and $BACKUP directories) will not work on some international versions of 'coreutils'
3- Backups (crashrecovery dirs) are not cleaned as expected in case of multi-profiles.
Detailed information:
Point 1: For example, with multi-profiles in the /home/user/.mozilla/firefox directory, crashrecovery dir created for one of the profiles will be displayed in the "recovery dirs" section of all profiles (Firefox).
Relevant function: parse()
Using the pattern "crashrecovery" (with the 'find' command) for all directories, regardless of the profile name is not sufficient.
On the other hand, the function will also explore all the subdirectories of $DIR to find the "crashrecovery" pattern. This can lead to hazardous deletions if the profile contains a directory with "crashrecovery" in the name...
The following change fixes the problem:
while IFS= read -d '' -r backup; do
CRASHArr=("${CRASHArr[@]}" "$backup")
done < <(find "${DIR%/*}" -maxdepth 1 -type d -name "${item##*/}*crashrecovery*" -print0 | sort -r -z)
Point 2: The following syntax BACKUP_TIME=$(stat "$BACKUP" | grep Change.... used in function ungraceful_state_check() is not compatible with an internationalized version of the 'stat' command ("Change" being translated).
Rather than putting LANG = C; in front of the relevant command lines, the whole code can be simplified :
Point 3 basically comes from the enforce() function.
No loading of env variables from the list of browsers (load_env_for not called)
No initialization loop of the variable $DIR (as is the case in the cleanup() function)
For the cleanup() function, same problem as in point 1 concerning the use of the "crashrecovery" pattern.
Otherwise, this point has already been addressed by the htower : Fix backups cleanup (PR #297)
The main idea is to pass the number of backups to keep as an argument of the cleanup() function.
Finally, this eliminates the need for the enforce() function by calling ' cleanup "$BACKUP_LIMIT" '.
By reshaping the code proposed by htower and adding the modification of the "crashrecovery" pattern processing (with the 'find' command)... It works !
I also observed that the creation of the crashrecovery dir is done by the ungraceful_state_check() function but the enforcement of the limit (max number of recovery directories set by "$BACKUP LIMIT") is only performed when executing a 'sync', more exactly the first invocation of psd resync by systemd psd-resync.service.
This leads to an ambiguous display after invoking 'psd p'.
I think enforce() (rather cleanup "$BACKUP_LIMIT" in the above solution) should be systematically invoked after calling the ungraceful_state_check() function (psd sync|resync|startup|parse)
Other fixes:
In ungraceful_state_check() function:
Message :
echo "Unexpected state detected: we have $BACKUP, but $DIR already exists. Trying move $DIR to $DIR" "$DIR-old-profile-$NOW"
Rather :
echo "Unexpected state detected: we have $BACKUP, but $DIR already exists. Trying move $DIR to $DIR-old-profile-$NOW"
In do_sync() function, forgotten variable :
echo "BACKUP_LIMIT=\"$BACKUP_LIMIT\""
Bugs related to the management of backups (crashrecovery): 1- The information displayed in
"recovery dirs"section by'psd p'is not always correct. 2- Searching for the most recent backup (between the$BACK_OVFSand$BACKUPdirectories) will not work on some international versions of'coreutils'3- Backups (crashrecovery dirs) are not cleaned as expected in case of multi-profiles.Detailed information:
Point 1: For example, with multi-profiles in the
/home/user/.mozilla/firefoxdirectory, crashrecovery dir created for one of the profiles will be displayed in the"recovery dirs"section of all profiles (Firefox).Relevant function:
parse()Using the pattern"crashrecovery"(with the'find'command) for all directories, regardless of the profile name is not sufficient. On the other hand, the function will also explore all the subdirectories of$DIRto find the"crashrecovery"pattern. This can lead to hazardous deletions if the profile contains a directory with"crashrecovery"in the name... The following change fixes the problem:Point 2: The following syntax
BACKUP_TIME=$(stat "$BACKUP" | grep Change....used in functionungraceful_state_check()is not compatible with an internationalized version of the'stat'command ("Change"being translated). Rather than puttingLANG = C;in front of the relevant command lines, the whole code can be simplified :Point 3 basically comes from the
enforce()function.No loading of env variables from the list of browsers (
load_env_fornot called)No initialization loop of the variable
$DIR(as is the case in thecleanup()function)For the
cleanup()function, same problem as in point 1 concerning the use of the"crashrecovery"pattern.Otherwise, this point has already been addressed by the htower : Fix backups cleanup (PR #297) The main idea is to pass the number of backups to keep as an argument of the
cleanup()function. Finally, this eliminates the need for theenforce()function by calling' cleanup "$BACKUP_LIMIT" '.By reshaping the code proposed by htower and adding the modification of the
"crashrecovery"pattern processing (with the'find'command)... It works !I also observed that the creation of the crashrecovery dir is done by the
ungraceful_state_check()function but the enforcement of the limit (max number of recovery directories set by"$BACKUP LIMIT") is only performed when executing a'sync', more exactly the first invocation ofpsd resyncby systemdpsd-resync.service. This leads to an ambiguous display after invoking'psd p'.I think enforce() (rather
cleanup "$BACKUP_LIMIT"in the above solution) should be systematically invoked after calling theungraceful_state_check()function (psd sync|resync|startup|parse)Other fixes: In
ungraceful_state_check()function: Message :Rather :
In
do_sync()function, forgotten variable :echo "BACKUP_LIMIT=\"$BACKUP_LIMIT\""