Francois Gouget : testbot/WinePatchesMLSubmit: Better handle errors and times when the Engine is not running.

[Alexandre Julliard]

Module: tools
Branch: master
Commit: 2cdc80043ed19e500164d4255a2551bb24089203
URL:    http://source.winehq.org/git/tools.git/?a=commit;h=2cdc80043ed19e500164d4255a2551bb24089203

Author: Francois Gouget <fgouget at codeweavers.com>
Date:   Wed May  7 02:41:26 2014 +0200

testbot/WinePatchesMLSubmit: Better handle errors and times when the Engine is not running.

WinePatchesMLSubmit must return a non-zero exit code in case it could
not deliver the message so the caller gets a chance to handle the
situation gracefully.  Have the Engine scan the staging directory for
pending patches from the mailing list and automatically handle them,
in particular on startup. This makes it possible for
WinePatchesMLSubmit to deliver the patches even if the Engine is not
running which is more robust.  This also fixes a potential source of
staging files leak.

---

 testbot/bin/Engine.pl                    |   77 ++++++++++++++++--------------
 testbot/bin/WinePatchesMLSubmit.pl       |   24 +++++++---
 testbot/lib/WineTestBot/Engine/Notify.pm |    4 +-
 3 files changed, 61 insertions(+), 44 deletions(-)

diff --git a/testbot/bin/Engine.pl b/testbot/bin/Engine.pl
index ea04461..2bb563a 100755
--- a/testbot/bin/Engine.pl
+++ b/testbot/bin/Engine.pl
@@ -446,36 +446,53 @@ sub HandleFoundWinetestUpdate
 
 sub HandleWinePatchMLSubmission
 {
-  # Validate file name
-  if ($_[0] !~ m/([0-9a-fA-F]{32}_wine-patches)/)
+  my $dh;
+  if (!opendir($dh, "$DataDir/staging"))
   {
-    return "0Invalid file name";
+    return "0Unable to open '$DataDir/staging': $!";
   }
-  my $FullMessageFileName = "$DataDir/staging/$1";
 
-  # Create a working dir
-  my $WorkDir = "$DataDir/staging/" . GenerateRandomString(32) . "_work";
-  while (-e $WorkDir)
+  # Read the directory ahead as we'll be adding / removing entries
+  my @Entries = readdir($dh);
+  closedir($dh);
+
+  my @ErrMessages;
+  foreach my $Entry (@Entries)
   {
-    $WorkDir = "$DataDir/staging/" . GenerateRandomString(32) . "_work";
-  }
-  mkdir $WorkDir;
+    # Validate file name
+    next if ($Entry !~ m/^([0-9a-fA-F]{32}_wine-patches)$/);
+    my $FullMessageFileName = "$DataDir/staging/$1";
 
-  # Process the patch
-  my $Parser = new MIME::Parser;
-  $Parser->output_dir($WorkDir);
-  my $Entity = $Parser->parse_open($FullMessageFileName);
-  CreatePatches()->NewPatch($Entity);
+    # Create a work directory
+    my $WorkDir = "$DataDir/staging/" . GenerateRandomString(32) . "_work";
+    while (-e $WorkDir)
+    {
+      $WorkDir = "$DataDir/staging/" . GenerateRandomString(32) . "_work";
+    }
+    mkdir $WorkDir;
 
-  # Clean up
-  if (!rmtree($WorkDir))
-  {
-    # Not a fatal error but log it to help diagnosis
-    LogMsg "Unable to delete '$WorkDir': $!\n";
+    # Process the patch
+    my $Parser = new MIME::Parser;
+    $Parser->output_dir($WorkDir);
+    my $Entity = $Parser->parse_open($FullMessageFileName);
+    my $ErrMessage = CreatePatches()->NewPatch($Entity);
+    push @ErrMessages, $ErrMessage if (defined $ErrMessage);
+
+    # Clean up
+    if (!rmtree($WorkDir))
+    {
+       # Not a fatal error but log it to help diagnosis
+       LogMsg "Unable to delete '$WorkDir': $!\n";
+    }
+    if (!unlink($FullMessageFileName))
+    {
+      # This is more serious because it could cause a patch to be added
+      # again and again. But there is not much we can do.
+      LogMsg "Unable to delete '$FullMessageFileName': $!\n";
+    }
   }
-  unlink($FullMessageFileName);
 
-  return "1OK";
+  return @ErrMessages ? "0". join("; ", @ErrMessages) : "1OK";
 }
 
 sub HandleWinePatchWebSubmission
@@ -613,19 +630,6 @@ sub SafetyNet
   CheckJobs();
   ScheduleJobs();
 
-  if (opendir STAGING, "$DataDir/staging")
-  {
-    my @DirEntries = readdir STAGING;
-    closedir STAGING;
-    foreach my $DirEntry (@DirEntries)
-    {
-      if ($DirEntry =~ m/[0-9a-fA-F]{32}_wine-patches/)
-      {
-        HandleWinePatchMLSubmission($DirEntry);
-      }
-    }
-  }
-
   my $Set = WineTestBot::PendingPatchSets::CreatePendingPatchSets();
   my $ErrMessage = $Set->CheckForCompleteSet();
   if (defined($ErrMessage))
@@ -737,6 +741,9 @@ sub main
 
   Cleanup();
 
+  # Check for patches that arrived while the server was off.
+  HandleWinePatchMLSubmission();
+
   my $SockName = "$DataDir/socket/engine";
   my $uaddr = sockaddr_un($SockName);
   my $proto = getprotobyname('tcp');
diff --git a/testbot/bin/WinePatchesMLSubmit.pl b/testbot/bin/WinePatchesMLSubmit.pl
index ffc65cb..e904bbc 100755
--- a/testbot/bin/WinePatchesMLSubmit.pl
+++ b/testbot/bin/WinePatchesMLSubmit.pl
@@ -41,16 +41,28 @@ use File::Copy;
 use WineTestBot::Config;
 use WineTestBot::Utils;
 use WineTestBot::Engine::Notify;
+use WineTestBot::Log;
 
 # Store the message in the staging dir
-my $FileNameRandomPart = GenerateRandomString(32);
-while (-e ("$DataDir/staging/${FileNameRandomPart}_wine-patches"))
+my $FileName = GenerateRandomString(32) . "_wine-patches";
+while (-e ("$DataDir/staging/$FileName"))
 {
-  $FileNameRandomPart = GenerateRandomString(32);
+  $FileName = GenerateRandomString(32);
 }
-copy(\*STDIN, "$DataDir/staging/${FileNameRandomPart}_wine-patches");
+if (!copy(\*STDIN, "$DataDir/staging/$FileName"))
+{
+  LogMsg "Unable to copy the email to '$FileName': $!\n";
+  exit 1;
+}
+
 
-# Let the engine handle it
-WinePatchMLSubmission("${FileNameRandomPart}_wine-patches");
+# Notify the Engine of the new message
+my $ErrMessage = WinePatchMLSubmission();
+if (defined $ErrMessage)
+{
+  # The Engine will pick up the email later so return success anyway.
+  # But still log the issue so it can be checked out.
+  LogMsg "$ErrMessage\n"
+}
 
 exit 0;
diff --git a/testbot/lib/WineTestBot/Engine/Notify.pm b/testbot/lib/WineTestBot/Engine/Notify.pm
index f376337..09191ee 100644
--- a/testbot/lib/WineTestBot/Engine/Notify.pm
+++ b/testbot/lib/WineTestBot/Engine/Notify.pm
@@ -218,9 +218,7 @@ sub FoundWinetestUpdate
 
 sub WinePatchMLSubmission
 {
-  my $FileName = $_[0];
-
-  my $Reply = SendCmdReceiveReply("winepatchmlsubmission $FileName\n");
+  my $Reply = SendCmdReceiveReply("winepatchmlsubmission\n");
   if (length($Reply) < 1)
   {
     return "Unrecognized reply received from engine";