From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adolf Belka To: development@lists.ipfire.org Subject: Re: [PATCH] backup.pl: fix Bug12817 - directories missing in backup after include file changed to relat> Date: Mon, 28 Mar 2022 20:22:29 +0200 Message-ID: In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8046654153148103205==" List-Id: --===============8046654153148103205== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Michael, On 28/03/2022 18:40, Michael Tremer wrote: > Hello Adolf, >=20 > So looking at this problem, it seems to be pretty bad. Any backup created w= ith c164 is practically incomplete. Ouch. >=20 > So since c165 is already built and uploaded to the servers, I am not going = to re-open it now. >=20 > I suppose fixing the bug ASAP is what we need to do for now before we are l= ooking deeper into some long-term solution. >=20 > Could you confirm to me that this change is fixing the problem? I am afraid that I can't. Running that ended up with the backup also trying t= o include all backup.ipf files into the backup. It even tried to backup the b= ackup.ipf file from the backup being carried out and came up with the message= that the file had changed while being backed up. That happened when the pushd and popd were first used. Bernhard identified th= at the command was first outputting a whole list of files from the directory = defined ie /. That is why it was changed to pushd / >/dev/null >=20 > diff --git a/config/backup/backup.pl b/config/backup/backup.pl > index a2337cf23..a25c9a355 100644 > --- a/config/backup/backup.pl > +++ b/config/backup/backup.pl > @@ -38,7 +38,7 @@ process_includes() { > for include in $@; do > local file > while read -r file; do > - for file in ${file}; do > + for file in /${file}; do > if [ -e "/${file}" ]; then > echo "${file}" > fi >=20 > It basically does the same as the pushd/popd set, but I find it a little bi= t more elegant :) I ran some bash testing. What I found is that in the above routine the file/p= ath being read from the include file is not having the wildcards expanded so = that the routine ends up, for instance with /var/ipfire/ddns/*.conf as the ac= tual path/filename to check if it exists and of course it does not. Regards, Adolf. >=20 > -Michael >=20 >> On 28 Mar 2022, at 17:24, Adolf Belka wrote: >> >> Hi Michael, >> >> On 28/03/2022 16:30, Adolf Belka wrote: >>> Hi Michael, >>> >>> On 28/03/2022 16:08, Michael Tremer wrote: >>>> Hello Adolf, >>>> >>>>> On 25 Mar 2022, at 12:22, Adolf Belka wrote: >>>>> >>>>> From: Bernhard Bitsch >>>>> >>>>> - The change of the backup include file from absolute paths to relative= paths in CU164 >>>>> commit https://git.ipfire.org/?p=3Dipfire-2.x.git;a=3Dcommit;h=3Dc7e0d7= 3e7cfd7be95db9d0a5f3392b8241813d5b >>>>> resulted in 20 core function directories no longer being backed up. Add= itionally a >>>>> similar effect occurred with some addon backups >>>> >>>> Why is this happening? Is it because tar relied on the shell to expand t= he glowing characters? >>> I don't know. I found the issue, together with someone on the forum. Bern= hard suggested the fix, which I tested, but I don't understand the issues beh= ind the problem. Maybe Bernhard has more input on what might be causing the p= roblem. >>>> >>>> In that case, the file list should be changed that tar knows what files = to pack and what not. I wouldn=E2=80=99t want to rely on globbing unless we h= ave a reason that forces us. >>> Okay, will try and see if I can understand the basis for why the problem = is occurring with the change of names from absolute to relative. >>> >> I have tried to look at the code and see what the problem is. I can unders= tand the essence of what the code is doing but not why it is going wrong, or = at least yet. >> What a comparison of the files that are saved with the ones that are not s= aved shows that the ones not saved are those that would get selected by wildc= ards in the include file. All the ones saved look to be those with explicit f= ilenames with no wild cards. >> >> So something in either the process-includes subroutine or in the tar comma= nd is not working with wildcards. >> >> I will try and see if I can set a small code loop up to see where the prob= lem occurs. >> >> Regards, >> Adolf. >>> Regards, >>> Adolf. >>>> >>>>> - Fix applied here proposed by Bernhard Bitsch and tested on a vm testb= ed system and >>>>> confirmed to fix the problem. Backup of all directories again being don= e. >>>>> >>>>> Fixes: Bug12817 >>>>> Tested-By: Adolf Belka >>>>> Signed-off-by: Adolf Belka >>>>> --- >>>>> config/backup/backup.pl | 22 ++++++++++++---------- >>>>> 1 file changed, 12 insertions(+), 10 deletions(-) >>>>> >>>>> diff --git a/config/backup/backup.pl b/config/backup/backup.pl >>>>> index a2337cf23..c7dbc6cae 100644 >>>>> --- a/config/backup/backup.pl >>>>> +++ b/config/backup/backup.pl >>>>> @@ -35,16 +35,18 @@ list_addons() { >>>>> process_includes() { >>>>> local include >>>>> >>>>> - for include in $@; do >>>>> - local file >>>>> - while read -r file; do >>>>> - for file in ${file}; do >>>>> - if [ -e "/${file}" ]; then >>>>> - echo "${file}" >>>>> - fi >>>>> - done >>>>> - done < "${include}" >>>>> - done | sort -u >>>>> + pushd / >/dev/null >>>>> + for include in $@; do >>>>> + local file >>>>> + while read -r file; do >>>>> + for file in ${file}; do >>>>> + if [ -e "/${file}" ]; then >>>>> + echo "${file}" >>>>> + fi >>>>> + done >>>>> + done < "${include}" >>>>> + done | sort -u >>>>> + popd >/dev/null >>>>> } >>>>> >>>>> make_backup() { >>>>> --=20 >>>>> 2.35.1 >>>>> >>>> >>> >> >> --=20 >> Sent from my laptop >=20 --===============8046654153148103205==--