testbot/WinePatchesWebSubmit: Improve robustness, particularly when the Engine is not running.

[Francois Gouget]

Now the Engine will automatically pick up patches from the patches website as part of its periodic 'SafetyNet' check.
However WinePatchesWebSubmit can still be used to avoid the potential 10 minutes delay.
Also, running WinePatchesWebSubmit when the Engine is stopped no longer has the side effect of leaking files in the staging directory.
---

This plugs the leak that lead to the creation of bug 35576:
http://bugs.winehq.org/show_bug.cgi?id=35576

On my system, where the Engine is down 12 hours a day, this was 
ultimately responsible for about 100000 leaked files (but using only 
1GB).

 testbot/bin/Engine.pl                    | 86 +++++++++++++++++++++-----------
 testbot/bin/WinePatchesWebSubmit.pl      | 53 +++-----------------
 testbot/lib/WineTestBot/Engine/Notify.pm |  4 +-
 3 files changed, 63 insertions(+), 80 deletions(-)

diff --git a/testbot/bin/Engine.pl b/testbot/bin/Engine.pl
index 89d0cb2..f8b7e1e 100755
--- a/testbot/bin/Engine.pl
+++ b/testbot/bin/Engine.pl
@@ -478,48 +478,73 @@ sub HandleWinePatchMLSubmission
 
 sub HandleWinePatchWebSubmission
 {
-  # Validate file name
-  if ($_[0] !~ m/([0-9a-fA-F]{32}_patch_\d+)/)
+  my $LatestWebPatchId = 0;
+  my $Patches = CreatePatches();
+  foreach my $Patch (@{$Patches->GetItems()})
   {
-    return "0Invalid file name";
+    my $WebPatchId = $Patch->WebPatchId;
+    if (defined $WebPatchId and $LatestWebPatchId < $WebPatchId)
+    {
+      $LatestWebPatchId = $WebPatchId;
+    }
   }
-  my $FullFileName = "$DataDir/staging/$1";
-  if ($_[1] !~ m/^(\d+)$/)
+
+  # Rescan the directory for robustness in case the patches site has already
+  # expired $LatestWebPatchId+1 (for instance if the WineTestBot has not been
+  # run for a while).
+  my (@ErrMessages, @WebPatchIds);
+  if (opendir(my $dh, "$DataDir/webpatches"))
   {
-    return "0Invalid patch id";
+    foreach my $Entry (readdir($dh))
+    {
+      next if ($Entry !~ /^(\d+)$/);
+      my $WebPatchId = $1;
+      next if ($WebPatchId <= $LatestWebPatchId);
+
+      my $Patches = CreatePatches($Patches);
+      $Patches->AddFilter("WebPatchId", [$WebPatchId]);
+      if (@{$Patches->GetKeys()})
+      {
+        push @ErrMessages, "$WebPatchId already exists and yet the latest patch is $LatestWebPatchId";
+        next;
+      }
+      push @WebPatchIds, $WebPatchId;
+    }
+    close($dh);
   }
-  my $WebPatchId = $1;
-  my $Patches = CreatePatches();
-  $Patches->AddFilter("WebPatchId", [$WebPatchId]);
-  if (@{$Patches->GetKeys()})
+  else
   {
-    LogMsg "Patch $WebPatchId already exists\n";
-    return "1OK";
+    return "0Unable to open '$DataDir/webpatches': $!";
   }
 
-  # Create a working dir
-  my $WorkDir = "$DataDir/staging/" . GenerateRandomString(32) . "_work";
-  while (-e $WorkDir)
+  # Add the patches in increasing WebPatchId order so that next time
+  # $LatestWebPatchId still makes sense in case something goes wrong now
+  foreach my $WebPatchId (sort { $a <=> $b } @WebPatchIds)
   {
-    $WorkDir = "$DataDir/staging/" . GenerateRandomString(32) . "_work";
-  }
-  mkdir $WorkDir;
+    # Create a working dir
+    my $WorkDir = "$DataDir/staging/" . GenerateRandomString(32) . "_work";
+    while (-e $WorkDir)
+    {
+      $WorkDir = "$DataDir/staging/" . GenerateRandomString(32) . "_work";
+    }
+    mkdir $WorkDir;
 
-  # Process the patch
-  my $Parser = new MIME::Parser;
-  $Parser->output_dir($WorkDir);
-  my $Entity = $Parser->parse_open($FullFileName);
-  my $ErrMessage = CreatePatches()->NewPatch($Entity, $WebPatchId);
+    # Process the patch
+    my $Parser = new MIME::Parser;
+    $Parser->output_dir($WorkDir);
+    my $Entity = $Parser->parse_open("$DataDir/webpatches/$WebPatchId");
+    my $ErrMessage = $Patches->NewPatch($Entity, $WebPatchId);
+    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";
+    # Clean up
+    if (!rmtree($WorkDir))
+    {
+      # Not a fatal error but log it to help diagnosis
+      LogMsg "Unable to delete '$WorkDir': $!\n";
+    }
   }
-  unlink($FullFileName);
 
-  return defined($ErrMessage) ? "0" . $ErrMessage : "1OK";
+  return @ErrMessages ? "0". join("; ", @ErrMessages) : "1OK";
 }
 
 sub HandleGetScreenshot
@@ -609,6 +634,7 @@ sub SafetyNet
 {
   CheckJobs();
   ScheduleJobs();
+  HandleWinePatchWebSubmission();
 
   my $Set = WineTestBot::PendingPatchSets::CreatePendingPatchSets();
   my $ErrMessage = $Set->CheckForCompleteSet();
diff --git a/testbot/bin/WinePatchesWebSubmit.pl b/testbot/bin/WinePatchesWebSubmit.pl
index d5b032a..15a5fd5 100755
--- a/testbot/bin/WinePatchesWebSubmit.pl
+++ b/testbot/bin/WinePatchesWebSubmit.pl
@@ -37,56 +37,15 @@ sub BEGIN
   }
 }
 
-use File::Copy;
-use WineTestBot::Config;
 use WineTestBot::Log;
-use WineTestBot::Utils;
-use WineTestBot::Patches;
 use WineTestBot::Engine::Notify;
 
-my ($MaxCount) = @ARGV;
-if (defined $MaxCount)
+# Just tell the Engine to rescan the webpatches directory
+my $ErrMessage = WinePatchWebSubmission();
+if (defined $ErrMessage)
 {
-  $MaxCount =~ m/^(\d+)$/;
-  $MaxCount = $1;
+  LogMsg "$ErrMessage\n";
+  exit(1);
 }
 
-my $LastWebPatchId = 0;
-foreach my $Patch (@{CreatePatches()->GetItems()})
-{
-  my $WebPatchId = $Patch->WebPatchId;
-  if (defined $WebPatchId and $LastWebPatchId < $WebPatchId)
-  {
-    $LastWebPatchId = $WebPatchId;
-  }
-}
-
-while (1)
-{
-  $LastWebPatchId++;
-  my $NewPatch = "$DataDir/webpatches/$LastWebPatchId";
-  last if (!-f $NewPatch);
-
-  my $FileNameRandomPart = GenerateRandomString(32);
-  while (-e "$DataDir/staging/${FileNameRandomPart}_patch_$LastWebPatchId")
-  {
-    $FileNameRandomPart = GenerateRandomString(32);
-  }
-  my $StagingFileName = "$DataDir/staging/${FileNameRandomPart}_patch_$LastWebPatchId";
-
-  if (!copy($NewPatch, $StagingFileName))
-  {
-    LogMsg "Unable to copy '$NewPatch' to '$StagingFileName': $!\n";
-    exit 1;
-  }
-
-  WinePatchWebSubmission("${FileNameRandomPart}_patch_$LastWebPatchId", $LastWebPatchId);
-  LogMsg "Added wine-patches patch $LastWebPatchId\n";
-  if (defined $MaxCount)
-  {
-    $MaxCount--;
-    last if ($MaxCount == 0);
-  }
-}
-
-exit;
+exit(0);
diff --git a/testbot/lib/WineTestBot/Engine/Notify.pm b/testbot/lib/WineTestBot/Engine/Notify.pm
index 9b802e1..a241c81 100644
--- a/testbot/lib/WineTestBot/Engine/Notify.pm
+++ b/testbot/lib/WineTestBot/Engine/Notify.pm
@@ -216,9 +216,7 @@ sub WinePatchMLSubmission
 
 sub WinePatchWebSubmission
 {
-  my ($FileName, $WebPatchId) = @_;
-
-  my $Reply = SendCmdReceiveReply("winepatchwebsubmission $FileName $WebPatchId\n");
+  my $Reply = SendCmdReceiveReply("winepatchwebsubmission\n");
   if (length($Reply) < 1)
   {
     return "Unrecognized reply received from engine";
-- 
2.0.0.rc0