#3221 PR merged: Overhauled 400_create_include_exclude_files.sh

Labels: enhancement, cleanup, fixed / solved / done

jsmeix opened issue at 2024-05-14 12:13:

Overhauled 400_create_include_exclude_files.sh


  • Description of the changes in this pull request:

In backup/NETFS/default/400_create_include_exclude_files.sh

Now do first backup the mounted filesystems
to backup '/' first so the basic system files
get stored first in the backup and
then backup what is specified in BACKUP_PROG_INCLUDE
see https://github.com/rear/rear/pull/3177#issuecomment-1985926458
and https://github.com/rear/rear/issues/3217#issue-2277985295

Report suspicious cases as LogPrintError
to have the user at least informed.

Remove duplicates but keep the ordering (via unique_unsorted)
to avoid possibly unwanted and unexpected subtle consequences,
see https://github.com/rear/rear/pull/3175#issuecomment-1985382738

Verify that at least '/' is in backup-include.txt
see https://github.com/rear/rear/issues/3217

Redirect stdout into files exactly at the command where needed
instead of more global redirections, cf. "horrible coding style"
in https://github.com/rear/rear/pull/3175#issuecomment-2109554865

In backup/NETFS/default/500_make_backup.sh
unique_unsorted is no longer needed because
backup-include.txt is already without duplicates
because unique_unsorted is now called in
so replaced unique_unsorted calls back to 'cat', cf.

jsmeix commented at 2024-05-15 11:48:

I intend to merge it later today
because this is needed as a precondition for
on SUSE with its default btrfs structure with snapshots

jsmeix commented at 2024-05-15 15:32:

I ignore the CI failures

Build Packages / build (push) Failing after 8m 
testing-farm:fedora-40-x86_64 — Error ... 
testing-farm:fedora-rawhide-x86_64 — Tests failed ...

in particular because other 'testing-farm' checks succeed
so I assume the failing 'testing-farm' checks have reasons
that are not in my code changes.

fishgit commented at 2024-05-24 10:55:

The added verification that at least '/' is in backup-include.txt leads to problems in our setup with borg backup. We have multiple rear conf files to backup data into different borg repositories (basic_system, home, var, ...)

'/' is therefore included in basic_system.conf only. Running rear -C /etc/rear/home.conf mkbackuponly exits with the 'Error "At least the root filesystem must be backed up"' error accordingly.

It would be great if there was an option to disable this check (at least when using borg backup).

jsmeix commented at 2024-05-24 12:58:

thank you so much for your regression report!
It is much appreciated.

I am sorry - I had totally forgotten about the

# rear -C something_else_backup mkbackuponly

possibility - how embarrasing :-(

I (hopefully) fixed that regression right now
via a direct commit to our master code

please test our current GitHub master code if my
is actually a proper fix for your use case
and report here how it behaves for you.

as time permits - please have a look at my
direct commit to our master code
and review if it looks OK to you.
In this particular case I dared to fix the regression
via a direct commit to our master code because
in this case "a proper fix" looked sufficiently simple
and straightforward to me and I wanted to get my regression
fixed myself as fast as reasonably possible.

fishgit commented at 2024-05-24 13:23:

thank you for this instant fix!

I can confirm that your commit solves the problem we had.

jsmeix commented at 2024-05-24 13:36:

thank you for your prompt and positive feedback!

@fishgit @rear/contributors
I wish you all a relaxed and recovering weekend!

At least my weekend is now much more relaxed
with having this regression fixed :-)

[Export of Github issue for rear/rear.]