Re: [PATCH 2/3] backup: Create tarball in one pass

[Paul Simmons <mbatranch@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5423 bytes --]

On 12/3/21 07:54, Adolf Belka wrote:
> Hi Michael,
>
> On 02/12/2021 16:48, Michael Tremer wrote:
>> Hello Adolf,
>>
>> Thank you for reviewing my patch :)
>>
>>> On 2 Dec 2021, at 13:42, Adolf Belka <adolf.belka(a)ipfire.org> wrote:
>>>
>>> Hi Michael,
>>>
>>> I split the tar into two stages in May of this year as a fix for Bug
>>> #12626.
>>>
>>> If the tar/gzip is done in one go then any files in the include.user
>>> file that match a pattern in the global exclude will always be
>>> ignored as the --exclude-from has precedence over any file lists at
>>> the end of the command.
>>
>> I should have reached out because it wasn’t quite clear to me why
>> this was split into two operations.
>>
>> I would have considered this the expected behaviour, because what is
>> excluded is excluded. Clearly you expected something else to happen
>> which I would also consider a valid expectation.
> It wasn't me that highlighted that effect but Paul Simmons in Bug
> 11320 - Umbrella bug for backup system, comment 1. I took that and
> created Bug 12626. I have not had any problems myself with the user
> backup as it was previously running.
>>
>>> If this patch is implemented then I would edit the wiki to specify
>>> that user files, wanting to be backed up by adding to the
>>> include.user file, should not be in any of the directories listed in
>>> the global exclude file and must not have a .tmp extension.
>>
>> Is there any use-case where this would become a big problem? Is there
>> anything the user would include that has previously been globally
>> excluded?
> This question would need to be asked to Paul Simmons as he raised the
> bug in the first case. I don't know if he is included in the
> dewvlopment(a)lists.ipfire.org, if not then the simplest would be to add
> his name to this email series and see if we get a response.
> The issues that have been raised of the backup in the iso not working
> and the amount of disk space used with the temporary storage of the
> tar files instead of going direct to gzip would seem to be good
> reasons to revert the change I did anyway and then later, depending on
> the response from Paul Simmons, we can discuss further if a use case
> that needs to be addressed is identified.
>
> Does that seem a reasonable approach to this.
>
> Regards,
> Adolf.
>>
>> -Michael
>>
>>>
>>>
>>> Regards,
>>>
>>> Adolf.
>>>
>>>
>>> On 02/12/2021 13:37, Michael Tremer wrote:
>>>> This patch is changing the behaviour of the backup script so that it
>>>> creates one tarball and compresses it in one go.
>>>>
>>>> This will save storing the original tarball on disk before compressing
>>>> it which on my test system requires significant disk space.
>>>>
>>>> This patch also solves a bug where the backup file included with
>>>> the ISO
>>>> image could not be extracted because it was not gzip-compressed
>>>> when it
>>>> was expected to be.
>>>>
>>>> Signed-off-by: Michael Tremer <michael.tremer(a)ipfire.org>
>>>> ---
>>>>   config/backup/backup.pl | 15 ++++-----------
>>>>   1 file changed, 4 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/config/backup/backup.pl b/config/backup/backup.pl
>>>> index 0b47af2d6..bed5952de 100644
>>>> --- a/config/backup/backup.pl
>>>> +++ b/config/backup/backup.pl
>>>> @@ -58,20 +58,13 @@ make_backup() {
>>>>       done
>>>>         # Backup using global exclude/include definitions
>>>> -    tar cvf "${filename}" \
>>>> +    tar cvfz "${filename}" \
>>>>           --exclude-from="/var/ipfire/backup/exclude" \
>>>> -        $(process_includes "/var/ipfire/backup/include") \
>>>> -        "$@"
>>>> -
>>>> -    # Backup using user exclude/include definitions and append to
>>>> global backup
>>>> -    tar rvf "${filename}" \
>>>>           --exclude-from="/var/ipfire/backup/exclude.user" \
>>>> +        $(process_includes "/var/ipfire/backup/include") \
>>>>           $(process_includes "/var/ipfire/backup/include.user") \
>>>>           "$@"
>>>>   -    # gzip the combined global/user backup and use .ipf suffix
>>>> -    gzip --suffix .ipf "${filename}"
>>>> -
>>>>       return 0
>>>>   }
>>>>   @@ -215,7 +208,7 @@ main() {
>>>>               local filename="${1}"
>>>>                 if [ -z "${filename}" ]; then
>>>> -                filename="/var/ipfire/backup/${NOW}"
>>>> +                filename="/var/ipfire/backup/${NOW}.ipf"
>>>>               fi
>>>>                 make_backup "${filename}" $(find_logfiles)
>>>> @@ -225,7 +218,7 @@ main() {
>>>>               local filename="${1}"
>>>>                 if [ -z "${filename}" ]; then
>>>> -                filename="/var/ipfire/backup/${NOW}"
>>>> +                filename="/var/ipfire/backup/${NOW}.ipf"
>>>>               fi
>>>>                 make_backup "${filename}"
>>>
>>> -- 
>>> Sent from my laptop
>>
>
Howdy, y'all.

I have an alternate solution for my use case.  I've created a pseudo
addon backup file which includes files that were omitted by the original
global include / exclude evaluations.

Thank you,

Paul Simmons