autom4te: replace output file atomically (#110305)

In 2003, Joey Hess reported the following bug against Debian's
autoconf package (see http://bugs.debian.org/221483):

    I noticed that if I ctrl-c autoconf, it can leave a partially
    written, executable configure script. I was lucky enough to
    get a configure script that exited with a shell parse error,
    but if I had been unlucky, it might have exited 0 without
    doing all the tests I expected it to do.  That would have
    sucked to ship to users.

    There are many ways to update a file in a way that is not
    prone to these problems, and I suggest that autoconf adopt
    one of them.

Ben Pfaff wrote a patch to make autom4te replace the output file
atomically; Debian has carried it since 2006.  He submitted it
to autoconf upstream in 2008 but it never went anywhere.

I (Zack) have dusted off the patch and made some minor improvements:
using File::Temp (with DIR set to the directory of the output file)
instead of a predictable temporary file name, and using
Autom4te::FileUtils::update_file instead of File::Copy::move.

I do not attempt to test the fix (the test would be inherently racey)
nor do I have autom4te delete the temp file if it crashes while the
file is being written (there is no way to do this with 100%
reliability and it strikes me as likely to cause more problems than it
solves).

Fixes our bug #110305.

* bin/autom4te.in (handle_output): When $output is to a regular or
  nonexistent file, write to a temporary file in the same directory
  and then rename it over $output after completion.

diff --git a/bin/autom4te.in b/bin/autom4te.in
index 20ecca9ee171978032146f8d5ba2cb854da46c47..9aa0d1f1b50d4f3af9bb045482f69e01e0740ad5 100644 (file)
--- a/bin/autom4te.in
+++ b/bin/autom4te.in
@@ -546,21 +546,38 @@ sub handle_output ($$)
     foreach (sort keys %forbidden);
   verb "allowed   tokens: $allowed";
 
-  # Read the (cached) raw M4 output, produce the actual result.  We
-  # have to use the 2nd arg to have Autom4te::XFile honor the third, but then
-  # stdout is to be handled by hand :(.  Don't use fdopen as it means
-  # we will close STDOUT, which we already do in END.
-  my $out = new Autom4te::XFile;
+  # Read the (cached) raw M4 output, produce the actual result.
+  # If we are writing to a regular file, replace it atomically.
+  my $atomic_replace = 0;
+  my $out;
   if ($output eq '-')
     {
-      $out->open (">$output");
+      # Don't just make $out be STDOUT, because then we would close STDOUT,
+      # which we already do in END.
+      $out = new Autom4te::XFile ('>&STDOUT');
+    }
+  elsif (-e $output && ! -f $output)
+    {
+      $out = new Autom4te::XFile ($output, '>');
     }
   else
     {
-      $out->open ($output, O_CREAT | O_WRONLY | O_TRUNC, oct ($mode));
+      my (undef, $outdir, undef) = fileparse ($output);
+
+      use File::Temp ();
+      $out = new File::Temp (UNLINK => 0, DIR => $outdir);
+      fatal "cannot create a file in $outdir: $!"
+        unless $out;
+
+      # File::Temp doesn't give us access to 3-arg open(2), unfortunately.
+      # In older Perls, implicit conversion of a File::Temp to its filename
+      # cannot be relied upon.
+      chmod (oct ($mode) & ~(umask), $out->filename)
+        or fatal "setting mode of " . $out->filename . ": $!";
+
+      $atomic_replace = 1;
     }
-  fatal "cannot create $output: $!"
-    unless $out;
+
   my $in = new Autom4te::XFile ($ocache . $req->id, "<");
 
   my %prohibited;
@@ -585,7 +602,8 @@ sub handle_output ($$)
       foreach (split (/\W+/))
        {
          $prohibited{$_} = $.
-           if !/^$/ && /$forbidden/o && !/$allowed/o && ! exists $prohibited{$_};
+           if !/^$/ && /$forbidden/o && !/$allowed/o
+               && ! exists $prohibited{$_};
        }
 
       # Performed *last*: the empty quadrigraph.
@@ -595,6 +613,8 @@ sub handle_output ($$)
     }
 
   $out->close();
+  update_file ($out->filename, $output, $force)
+    if $atomic_replace;
 
   # If no forbidden words, we're done.
   return