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 854a4e535fb84702912b61163bfe8d38e8a9d4ad..7824c8e66f9d251ef271afb1964b45846b0a4bad 100755 (executable)
--- 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..