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

[Francois Gouget]

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.
---

Let me know if there is a problem with this patch.

The potential staging file leak makes this related to bug 35576:
http://bugs.winehq.org/show_bug.cgi?id=35576

However in practice the WineTestBot uses the WinePatchesWebSubmit 
codepath so it should be unaffected by any issue in WinePatchesMLSubmit.
(next I'll send a patch for WinePatchesMLSubmit)

 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 f0570d7..3a7bacb 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 1a1ab6d..9c5d1ba 100644
--- a/testbot/lib/WineTestBot/Engine/Notify.pm
+++ b/testbot/lib/WineTestBot/Engine/Notify.pm
@@ -220,9 +220,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";
-- 
2.0.0.rc0