Francois Gouget : testbot/WinePatchesMLSubmit: Better handle errors and times when the Engine is not running.
Module: tools Branch: master Commit: 2cdc80043ed19e500164d4255a2551bb24089203 URL: http://source.winehq.org/git/tools.git/?a=commit;h=2cdc80043ed19e500164d4255...
Author: Francois Gouget fgouget@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";
-
Alexandre Julliard