From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Sun, 3 Jul 2022 22:59:32 +0000 (+0000)
Subject: Maintenance: Fix formater.pl application after a failure (#1078)
X-Git-Tag: SQUID_6_0_1~161
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9455268fd2fc1f1cb55c958107c2fef054f3dbe7;p=thirdparty%2Fsquid.git

Maintenance: Fix formater.pl application after a failure (#1078)

Our scripts/source-maintenance.sh relies on formater.pl to create
foo.astylebak -- a fresh copy of the being formatted file foo. The Perl
script is creating that copy, but since formater.pl could be executed
outside the "all source files are staged" protections offered by
source-maintenance.sh, formater.pl never overwrites old copies. Instead,
the Perl script numbers copies: foo.astylebak.1, foo.astylebak.2, ...

Unfortunately, the numbering code was copying into the highest available
index instead of shifting existing copies by one to always name the
fresh copy foo.astylebak, as expected by source-maintenance.sh! Thus, if
formatting failed (e.g., was interrupted by the user) leaving a backup
file on disk, then all subsequent attempts would start to fail after
a branch switch or local edits would make the backup copy stale:

    ERROR: File src/store.cc not formatting well

... with the following files on disk:

    src/store.cc <--- original file that should be backed up
    src/store.cc.astylebak.2 <--- fresh copy just made by formater.pl
    src/store.cc.astylebak.1
    src/store.cc.astylebak.0
    src/store.cc.astylebak <--- stale copy used by source-maintenance.sh

The FILO numbering order was also unusual/surprising to users; logrotate
and Squid itself use FIFO ordering, shifting numbered backup copies.

Implement FIFO ordering required by source-maintenance.sh and expected
by most users. TODO: Adjust source-maintenance.sh to stop relying on
formater.pl-created backup (and to stop guessing its name).
---

diff --git a/scripts/formater.pl b/scripts/formater.pl
index 854a4e535f..7824c8e66f 100755
--- a/scripts/formater.pl
+++ b/scripts/formater.pl
@@ -55,15 +55,10 @@ while($out){
 
     die("Cannot format a non-existent file: $out\n") unless -e $out;
 
-    my $in= "$out.astylebak";
-    my($new_in) = $in;
-    my($i) = 0;
-    while(-e $new_in) {
-        $new_in=$in.".".$i;
-        $i++;
-    }
-    $in=$new_in;
-    rename($out, $in);
+    my $backup = "$out.astylebak";
+    &moveAway($backup);
+    &safeRename($out, $backup);
+    my $in = $backup;
 
     local (*FROM_ASTYLE, *TO_ASTYLE);
     my $pid_style=open2(\*FROM_ASTYLE, \*TO_ASTYLE, $ASTYLE_BIN);
@@ -124,6 +119,37 @@ while($out){
     $out = shift @ARGV;
 }
 
+# renames while ensuring the destination is not clobbered
+sub safeRename
+{
+    my ($from, $to) = @_;
+    die() if -e $to;
+    rename($from, $to) or die("Failed to rename $from to $to: $!, stopped");
+}
+
+# "numbered backup" filename at a given backup depth
+# no ".n" suffix for the freshest/latest (i.e. zero depth) backup
+sub backupFilename
+{
+    my ($basename, $depth) = @_;
+    return $basename unless $depth;
+    return $basename . '.' . $depth;
+}
+
+# Renames the given backup file, moving it out of the way for the new backup.
+# Works recursively to ensure that no backup file is overwritten.
+sub moveAway
+{
+    my ($basename, $depth) = (@_, 0);
+
+    my $filename = &backupFilename($basename, $depth);
+    return unless -e $filename; # nothing to move away
+
+    my $spot = &backupFilename($basename, $depth + 1);
+    &moveAway($basename, $depth + 1); # free the spot if needed
+    &safeRename($filename, $spot); # move into the free spot
+}
+
 sub input_filter{
     my($line)=@_;
     #if we have integer declaration, get it all before processing it..