June 2018 - Wine-commits - list.winehq.org

Francois Gouget : testbot: Adjust the reconfig timeout.
by Alexandre Julliard 20 Jun '18

20 Jun '18

Module: tools Branch: master Commit: fea386748c25f865009831611de4c93d4ff3a2f7 URL: https://source.winehq.org/git/tools.git/?a=commit;h=fea386748c25f8650098316… Author: Francois Gouget <fgouget(a)codeweavers.com> Date: Wed Jun 20 02:31:27 2018 +0200 testbot: Adjust the reconfig timeout. $ReconfigTimeout was way overestimated, maybe reflecting a time when either the TestBot or the Wine dependencies were causing too many files to be rebuilt. Note that $BuildTimeout is a bit large too: a patch touching a test module and a core header would typically only require about 2.5 minutes. But in the unlikely event where that patch touched dozens of test modules it may still be too short. So $BuildTimeout is left unchanged. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- testbot/lib/WineTestBot/Config.pm | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/testbot/lib/WineTestBot/Config.pm b/testbot/lib/WineTestBot/Config.pm index b7d633d..66cb813 100644 --- a/testbot/lib/WineTestBot/Config.pm +++ b/testbot/lib/WineTestBot/Config.pm @@ -97,9 +97,10 @@ $SingleTimeout = 2 * 60; # How long to let a regular build run before forcibly shutting it down # (in seconds). $BuildTimeout = 5 * 60; -# How long to let a full recompilation run before forcibly shutting it down -# (in seconds). -$ReconfigTimeout = 45 * 60; +# How long to let a reconfig task run before forcibly shutting it down +# (in seconds). Note that this includes building the native Wine build tools, +# and the 32 and 64 bit test executables. +$ReconfigTimeout = (1 + 2 * 5) * 60; # How much to add to the task timeout to account for file transfers, etc. $TimeoutMargin = 2 * 60; # Maximum amount of traces for a test unit.

1 0

Francois Gouget : testbot/build: Detect patches that impact more than the tests.
by Alexandre Julliard 20 Jun '18

20 Jun '18

Module: tools Branch: master Commit: d39970c93c6dfe07d9795daf9ed1f60c2ec6ea74 URL: https://source.winehq.org/git/tools.git/?a=commit;h=d39970c93c6dfe07d9795da… Author: Francois Gouget <fgouget(a)codeweavers.com> Date: Wed Jun 20 02:31:45 2018 +0200 testbot/build: Detect patches that impact more than the tests. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- testbot/bin/build/Build.pl | 2 +- testbot/lib/WineTestBot/PatchUtils.pm | 101 +++++++++++++++++++++------------- 2 files changed, 64 insertions(+), 39 deletions(-) diff --git a/testbot/bin/build/Build.pl b/testbot/bin/build/Build.pl index 3fe0e14..8318af1 100755 --- a/testbot/bin/build/Build.pl +++ b/testbot/bin/build/Build.pl @@ -246,7 +246,7 @@ exit(1) if (!$Impacts); CountCPUs(); -if (!BuildNative()) +if ($Impacts->{WineBuild} and !BuildNative()) { exit(1); } diff --git a/testbot/lib/WineTestBot/PatchUtils.pm b/testbot/lib/WineTestBot/PatchUtils.pm index 6eb8e3c..f83df60 100644 --- a/testbot/lib/WineTestBot/PatchUtils.pm +++ b/testbot/lib/WineTestBot/PatchUtils.pm @@ -36,6 +36,23 @@ our @EXPORT = qw(GetPatchImpact UpdateWineData); use WineTestBot::Config; +# Patches to these paths don't impact the Wine build. So ignore them. +my $IgnoredPathsRe = join('|', + '\.mailmap$', + 'ANNOUNCE$', + 'AUTHORS$', + 'COPYING\.LIB$', + 'LICENSE\$', + 'LICENSE\.OLD$', + 'MAINTAINERS$', + 'README$', + 'VERSION$', + 'documentation/', + 'tools/c2man\.pl$', + 'tools/winapi/', + 'tools/winemaker/', +); + =pod =over 12 @@ -102,46 +119,54 @@ sub GetTestList() return $TestList; } -sub _AddTest($$$) +sub _HandleFile($$$) { my ($Impacts, $Path, $Change) = @_; - return if ($Path !~ m~^(dlls|programs)/([^/]+)/tests/([^/\s]+)$~); - my ($Root, $Module, $File) = ($1, $2, $3); - - my $Tests = $Impacts->{Tests}; - if (!$Tests->{$Module}) + if ($Path =~ m~^(dlls|programs)/([^/]+)/tests/([^/\s]+)$~) { - my $ExeBase = ($Root eq "programs") ? "${Module}.exe_test" : - "${Module}_test"; - $Tests->{$Module} = { - "Module" => $Module, - "Path" => "$Root/$Module/tests", - "ExeBase" => $ExeBase, - }; - } + my ($Root, $Module, $File) = ($1, $2, $3); + $Impacts->{TestBuild} = 1; - # Assume makefile modifications may break the build but not the tests - if ($File eq "Makefile.in") - { - if ($Change eq "new" or $Change eq "rm") + my $Tests = $Impacts->{Tests}; + if (!$Tests->{$Module}) { - # This adds / removes a directory - $Impacts->{Makefiles} = 1; + my $ExeBase = ($Root eq "programs") ? "${Module}.exe_test" : + "${Module}_test"; + $Tests->{$Module} = { + "Module" => $Module, + "Path" => "$Root/$Module/tests", + "ExeBase" => $ExeBase, + }; } - return; - } - return if ($Impacts->{NoUnits}); - if (!$Tests->{$Module}->{Files}) - { - my $TestList = ( $Impacts->{TestList} ||= GetTestList() ); - foreach my $File (keys %{$TestList->{$Module}}) + # Assume makefile modifications may break the build but not the tests + if ($File eq "Makefile.in") { - $Tests->{$Module}->{Files}->{$File} = 0; # not modified + if ($Change eq "new" or $Change eq "rm") + { + # This adds / removes a directory + $Impacts->{Makefiles} = 1; + } + return; } + return if ($Impacts->{NoUnits}); + + if (!$Tests->{$Module}->{Files}) + { + my $TestList = ( $Impacts->{TestList} ||= GetTestList() ); + foreach my $File (keys %{$TestList->{$Module}}) + { + $Tests->{$Module}->{Files}->{$File} = 0; # not modified + } + } + $Tests->{$Module}->{Files}->{$File} = $Change; + } + else + { + # Figure out if this patch impacts the Wine build + $Impacts->{WineBuild} = 1 if ($Path !~ /^(?:$IgnoredPathsRe)/); } - $Tests->{$Module}->{Files}->{$File} = $Change; } =pod @@ -176,8 +201,8 @@ sub GetPatchImpact($;$$) { foreach my $File (keys %{$PastInfo->{Files}}) { - _AddTest($Impacts, "$PastInfo->{Path}/$File", - $PastInfo->{Files}->{$File} eq "rm" ? "rm" : 0); + _HandleFile($Impacts, "$PastInfo->{Path}/$File", + $PastInfo->{Files}->{$File} eq "rm" ? "rm" : 0); } } } @@ -187,33 +212,33 @@ sub GetPatchImpact($;$$) { if ($Line =~ m=^--- \w+/(?:aclocal\.m4|configure\.ac)$=) { - $Impacts->{Autoconf} = 1; + $Impacts->{WineBuild} = $Impacts->{Autoconf} = 1; } elsif ($Line =~ m=^--- \w+/configure$=) { - $Impacts->{HasConfigure} = 1; + $Impacts->{WineBuild} = $Impacts->{HasConfigure} = 1; } elsif ($Line =~ m=^--- \w+/tools/make_makefiles$=) { - $Impacts->{Makefiles} = 1; + $Impacts->{WineBuild} = $Impacts->{Makefiles} = 1; } elsif ($Line =~ m=^--- /dev/null$=) { $Change = "new"; } - elsif ($Line =~ m~^--- \w+/([^/]+/[^/]+/tests/[^/\s]+)$~) + elsif ($Line =~ m~^--- \w+/([^\s]+)$~) { $Path = $1; } elsif ($Line =~ m~^\+\+\+ /dev/null$~) { - _AddTest($Impacts, $Path, "rm") if (defined $Path); + _HandleFile($Impacts, $Path, "rm") if (defined $Path); $Path = undef; $Change = ""; } - elsif ($Line =~ m~^\+\+\+ \w+/(\w+/[^/]+/tests/[^/\s]+)$~) + elsif ($Line =~ m~^\+\+\+ \w+/([^\s]+)$~) { - _AddTest($Impacts, $1, $Change || "modify"); + _HandleFile($Impacts, $1, $Change || "modify"); $Path = undef; $Change = ""; }

1 0

Francois Gouget : testbot/WineRun*: Rename the timeout WrapUpAndExit() parameter.
by Alexandre Julliard 20 Jun '18

20 Jun '18

Module: tools Branch: master Commit: 8135e0a60e9ababa222a17d8c9d0f70f4a8ac515 URL: https://source.winehq.org/git/tools.git/?a=commit;h=8135e0a60e9ababa222a17d… Author: Francois Gouget <fgouget(a)codeweavers.com> Date: Wed Jun 20 02:31:57 2018 +0200 testbot/WineRun*: Rename the timeout WrapUpAndExit() parameter. It is a boolean indicating whether the task timed out, not the timeout value in seconds. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- testbot/bin/WineRunBuild.pl | 4 ++-- testbot/bin/WineRunReconfig.pl | 4 ++-- testbot/bin/WineRunTask.pl | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/testbot/bin/WineRunBuild.pl b/testbot/bin/WineRunBuild.pl index eab33bc..1bc6cff 100755 --- a/testbot/bin/WineRunBuild.pl +++ b/testbot/bin/WineRunBuild.pl @@ -196,11 +196,11 @@ sub LogTaskError($) sub WrapUpAndExit($;$$) { - my ($Status, $Retry, $Timeout) = @_; + my ($Status, $Retry, $TimedOut) = @_; my $NewVMStatus = $Status eq 'queued' ? 'offline' : 'dirty'; my $VMResult = $Status eq "boterror" ? "boterror" : $Status eq "queued" ? "error" : - $Timeout ? "timeout" : ""; + $TimedOut ? "timeout" : ""; my $TestFailures; my $Tries = $Task->TestFailures || 0; diff --git a/testbot/bin/WineRunReconfig.pl b/testbot/bin/WineRunReconfig.pl index e7b447e..fe69fc3 100755 --- a/testbot/bin/WineRunReconfig.pl +++ b/testbot/bin/WineRunReconfig.pl @@ -195,12 +195,12 @@ sub LogTaskError($) sub WrapUpAndExit($;$$) { - my ($Status, $Retry, $Timeout) = @_; + my ($Status, $Retry, $TimedOut) = @_; my $NewVMStatus = $Status eq 'queued' ? 'offline' : $Status eq 'completed' ? 'idle' : 'dirty'; my $VMResult = $Status eq "boterror" ? "boterror" : $Status eq "queued" ? "error" : - $Timeout ? "timeout" : ""; + $TimedOut ? "timeout" : ""; my $TestFailures; my $Tries = $Task->TestFailures || 0; diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl index 3b0ccbc..e10656e 100755 --- a/testbot/bin/WineRunTask.pl +++ b/testbot/bin/WineRunTask.pl @@ -226,11 +226,11 @@ sub LogTaskError($) sub WrapUpAndExit($;$$$) { - my ($Status, $TestFailures, $Retry, $Timeout) = @_; + my ($Status, $TestFailures, $Retry, $TimedOut) = @_; my $NewVMStatus = $Status eq 'queued' ? 'offline' : 'dirty'; my $VMResult = $Status eq "boterror" ? "boterror" : $Status eq "queued" ? "error" : - $Timeout ? "timeout" : ""; + $TimedOut ? "timeout" : ""; Debug(Elapsed($Start), " Taking a screenshot\n"); TakeScreenshot($VM, "$TaskDir/screenshot.png");

1 0

Francois Gouget : testbot: Retrieve the list of Wine files.
by Alexandre Julliard 20 Jun '18

20 Jun '18

Module: tools Branch: master Commit: d22b2a3976d89277644544947fc0827f7eb25971 URL: https://source.winehq.org/git/tools.git/?a=commit;h=d22b2a3976d892776445449… Author: Francois Gouget <fgouget(a)codeweavers.com> Date: Wed Jun 20 02:32:19 2018 +0200 testbot: Retrieve the list of Wine files. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- testbot/bin/WineRunReconfig.pl | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/testbot/bin/WineRunReconfig.pl b/testbot/bin/WineRunReconfig.pl index fe69fc3..d04b780 100755 --- a/testbot/bin/WineRunReconfig.pl +++ b/testbot/bin/WineRunReconfig.pl @@ -444,6 +444,16 @@ if ($NewStatus eq "completed") { $TAError = "An error occurred while retrieving the test list: ". $TA->GetLastError(); } + + Debug(Elapsed($Start), " Retrieving the list of Wine files '$TaskDir/winefiles.txt'\n"); + if ($TA->GetFile("latest/winefiles.txt", "$TaskDir/winefiles.txt")) + { + copy "$TaskDir/winefiles.txt", "$DataDir/latest/winefiles.txt"; + } + elsif (!defined $TAError) + { + $TAError = "An error occurred while retrieving the list of Wine files: ". $TA->GetLastError(); + } } $TA->Disconnect();

1 0

Francois Gouget : testbot/WineRunTask: Trace the effective timeout, with leeway.
by Alexandre Julliard 20 Jun '18

20 Jun '18

Module: tools Branch: master Commit: 87171dff7e97c8ca8695c1ddbff8b4f0112c930b URL: https://source.winehq.org/git/tools.git/?a=commit;h=87171dff7e97c8ca8695c1d… Author: Francois Gouget <fgouget(a)codeweavers.com> Date: Wed Jun 20 02:33:39 2018 +0200 testbot/WineRunTask: Trace the effective timeout, with leeway. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- testbot/bin/WineRunTask.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl index e10656e..3d8113f 100755 --- a/testbot/bin/WineRunTask.pl +++ b/testbot/bin/WineRunTask.pl @@ -493,7 +493,7 @@ if (!$Pid) my $NewStatus = 'completed'; my ($TaskFailures, $TaskTimedOut, $TAError, $PossibleCrash); -Debug(Elapsed($Start), " Waiting for the script (", $Task->Timeout, "s timeout)\n"); +Debug(Elapsed($Start), " Waiting for the script (", $Timeout, "s timeout)\n"); if (!defined $TA->Wait($Pid, $Timeout, $Keepalive)) { my $ErrMessage = $TA->GetLastError();

1 0

Francois Gouget : testbot: Add section headers and reorder the Utils.pm functions.
by Alexandre Julliard 20 Jun '18

20 Jun '18

Module: tools Branch: master Commit: 1a1fd2c7e62bad68dcd8b7b19974e75c7aa57b5a URL: https://source.winehq.org/git/tools.git/?a=commit;h=1a1fd2c7e62bad68dcd8b7b… Author: Francois Gouget <fgouget(a)codeweavers.com> Date: Wed Jun 20 02:43:35 2018 +0200 testbot: Add section headers and reorder the Utils.pm functions. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- testbot/lib/WineTestBot/Utils.pm | 100 +++++++++++++++++++++------------------ 1 file changed, 54 insertions(+), 46 deletions(-) diff --git a/testbot/lib/WineTestBot/Utils.pm b/testbot/lib/WineTestBot/Utils.pm index 81a2eda..8b0bfeb 100644 --- a/testbot/lib/WineTestBot/Utils.pm +++ b/testbot/lib/WineTestBot/Utils.pm @@ -35,6 +35,10 @@ use Fcntl; use WineTestBot::Config; +# +# Web helpers +# + sub MakeSecureURL($) { my ($URL) = @_; @@ -53,6 +57,56 @@ sub SecureConnection() return defined($ENV{"HTTPS"}) && $ENV{"HTTPS"} eq "on"; } +sub DurationToString($;$) +{ + my ($Secs, $Raw) = @_; + + return "n/a" if (!defined $Secs); + + my @Parts; + if (!$Raw) + { + my $Mins = int($Secs / 60); + $Secs -= 60 * $Mins; + my $Hours = int($Mins / 60); + $Mins -= 60 * $Hours; + my $Days = int($Hours / 24); + $Hours -= 24 * $Days; + push @Parts, "${Days}d" if ($Days); + push @Parts, "${Hours}h" if ($Hours); + push @Parts, "${Mins}m" if ($Mins); + } + if (!@Parts or int($Secs) != 0) + { + push @Parts, (@Parts or int($Secs) == $Secs) ? + int($Secs) ."s" : + sprintf('%.1fs', $Secs); + } + return join(" ", @Parts); +} + +sub BuildEMailRecipient($$) +{ + my ($EMailAddress, $Name) = @_; + + if (! defined($EMailAddress)) + { + return undef; + } + my $Recipient = "<" . $EMailAddress . ">"; + if ($Name) + { + $Recipient .= " ($Name)"; + } + + return $Recipient; +} + + +# +# Temporary file helpers +# + sub GenerateRandomString($) { my ($Len) = @_; @@ -119,50 +173,4 @@ sub CreateNewDir($$) } } -sub DurationToString($;$) -{ - my ($Secs, $Raw) = @_; - - return "n/a" if (!defined $Secs); - - my @Parts; - if (!$Raw) - { - my $Mins = int($Secs / 60); - $Secs -= 60 * $Mins; - my $Hours = int($Mins / 60); - $Mins -= 60 * $Hours; - my $Days = int($Hours / 24); - $Hours -= 24 * $Days; - push @Parts, "${Days}d" if ($Days); - push @Parts, "${Hours}h" if ($Hours); - push @Parts, "${Mins}m" if ($Mins); - } - if (!@Parts or int($Secs) != 0) - { - push @Parts, (@Parts or int($Secs) == $Secs) ? - int($Secs) ."s" : - sprintf('%.1fs', $Secs); - } - return join(" ", @Parts); -} - -sub BuildEMailRecipient($$) -{ - my ($EMailAddress, $Name) = @_; - - if (! defined($EMailAddress)) - { - return undef; - } - my $Recipient = "<" . $EMailAddress . ">"; - if ($Name) - { - $Recipient .= " ($Name)"; - } - - return $Recipient; -} - - 1;

1 0

Francois Gouget : testbot/web: Reject filenames that are not valid Windows filenames.
by Alexandre Julliard 20 Jun '18

20 Jun '18

Module: tools Branch: master Commit: 759fc680db14c71e66e4ca5afb141c85e0504308 URL: https://source.winehq.org/git/tools.git/?a=commit;h=759fc680db14c71e66e4ca5… Author: Francois Gouget <fgouget(a)codeweavers.com> Date: Wed Jun 20 02:43:45 2018 +0200 testbot/web: Reject filenames that are not valid Windows filenames. IsValidFileName() verifies that the filename is valid on both Windows and Unix. This is necessary to ensure we will be able to upload the file to the build and/or test VMs. IsValidFileName() is defined in the Utils.pm module so it can be reused where necessary. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- testbot/lib/WineTestBot/Utils.pm | 25 ++++++++++++++++++++++++- testbot/web/Submit.pl | 4 ++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/testbot/lib/WineTestBot/Utils.pm b/testbot/lib/WineTestBot/Utils.pm index 8b0bfeb..962e6ff 100644 --- a/testbot/lib/WineTestBot/Utils.pm +++ b/testbot/lib/WineTestBot/Utils.pm @@ -28,7 +28,7 @@ WineTestBot::Utils - Utility functions use Exporter 'import'; our @EXPORT = qw(MakeSecureURL SecureConnection GenerateRandomString OpenNewFile CreateNewFile CreateNewLink CreateNewDir - DurationToString BuildEMailRecipient); + DurationToString BuildEMailRecipient IsValidFileName); use Fcntl; @@ -173,4 +173,27 @@ sub CreateNewDir($$) } } + +# +# Shell helpers +# + +=pod +=over 12 + +=item C<IsValidFileName()> + +Returns true if the filename is valid on Unix and Windows systems. + +This also ensures this is not a trick filename such as '../important/file'. + +=back +=cut + +sub IsValidFileName($) +{ + my ($FileName) = @_; + return $FileName !~ m~[<>:"/\\|?*]~; +} + 1; diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl index c16b99f..afebc72 100644 --- a/testbot/web/Submit.pl +++ b/testbot/web/Submit.pl @@ -514,10 +514,10 @@ sub ValidateAndGetFileName($$) $self->{ErrMessage} = "You must provide a file to test"; return undef; } - if ($FileName =~ m=[/\\]=) + if (!IsValidFileName($FileName)) { $self->{ErrField} = $FieldName; - $self->{ErrMessage} = "The filename is invalid"; + $self->{ErrMessage} = "The filename contains invalid characters"; return undef; } my $PropertyDescriptor = CreateSteps()->GetPropertyDescriptorByName("FileName");

1 0

Francois Gouget : testbot/Build: Don' t use tainting and improve handling of the patch filename.
by Alexandre Julliard 20 Jun '18

20 Jun '18

Module: tools Branch: master Commit: 62342aa288bfaacafb0c5a52f125fae0089e9e1e URL: https://source.winehq.org/git/tools.git/?a=commit;h=62342aa288bfaacafb0c5a5… Author: Francois Gouget <fgouget(a)codeweavers.com> Date: Wed Jun 20 02:43:59 2018 +0200 testbot/Build: Don't use tainting and improve handling of the patch filename. Tainting is moot because the whole purpose of the script is to run arbitrary user-provided code (in patch form). Still validate the patch filename but use the standard IsValidFileName() function (instead of the much stricter regular expression used so far) so patches accepted by Submit.pl don't get rejected at this late stage. However carefully quote the filename when building our shell command so it does not fail if the filename contains a space (for instance). Also note that in practice (so far) the patch filename is always called patch.diff as this is hardcoded in WineRunBuild.pl. So this is all a bit academic. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- testbot/bin/build/Build.pl | 25 ++++++++++++++----------- testbot/lib/WineTestBot/Utils.pm | 26 +++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/testbot/bin/build/Build.pl b/testbot/bin/build/Build.pl index 8318af1..e979955 100755 --- a/testbot/bin/build/Build.pl +++ b/testbot/bin/build/Build.pl @@ -1,10 +1,13 @@ -#!/usr/bin/perl -Tw +#!/usr/bin/perl # -*- Mode: Perl; perl-indent-level: 2; indent-tabs-mode: nil -*- # # Performs the 'build' task in the build machine. Specifically this applies a # conformance test patch, rebuilds the impacted test and retrieves the # resulting 32 and 64 bit binaries. # +# This script does not use tainting (-T) because its whole purpose is to run +# arbitrary user-provided code anyway (in patch form). +# # Copyright 2009 Ge van Geldorp # Copyright 2012-2014, 2017-2018 Francois Gouget # @@ -22,6 +25,7 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA +use warnings; use strict; sub BEGIN @@ -42,6 +46,8 @@ sub BEGIN use WineTestBot::Config; use WineTestBot::PatchUtils; +use WineTestBot::Utils; + my $LogFileName = "$LogDir/Build.log"; @@ -85,7 +91,7 @@ sub ApplyPatch($) InfoMsg "Applying patch\n"; system("( cd $DataDir/wine && set -x && " . - " git apply --verbose $PatchFile && " . + " git apply --verbose ". ShQuote($PatchFile) ." && ". " git add -A " . ") >>$LogFileName 2>&1"); if ($? != 0) @@ -199,18 +205,15 @@ if (! $PatchFile || !$BitIndicators) FatalError "Usage: Build.pl <patchfile> <bits>\n"; } -# Untaint parameters -if ($PatchFile =~ m/^([\w_.\-]+)$/) +# Verify parameters +if (!IsValidFileName($PatchFile)) { - $PatchFile = "$DataDir/staging/$1"; - if (! -r $PatchFile) - { - FatalError "Patch file $PatchFile not readable\n"; - } + FatalError "The patch filename '$PatchFile' contains invalid characters\n"; } -else +$PatchFile = "$DataDir/staging/$PatchFile"; +if (!-r $PatchFile) { - FatalError "Invalid patch file $PatchFile\n"; + FatalError "Patch file '$PatchFile' is not readable\n"; } my ($Run32, $Run64); diff --git a/testbot/lib/WineTestBot/Utils.pm b/testbot/lib/WineTestBot/Utils.pm index 962e6ff..111a565 100644 --- a/testbot/lib/WineTestBot/Utils.pm +++ b/testbot/lib/WineTestBot/Utils.pm @@ -28,7 +28,8 @@ WineTestBot::Utils - Utility functions use Exporter 'import'; our @EXPORT = qw(MakeSecureURL SecureConnection GenerateRandomString OpenNewFile CreateNewFile CreateNewLink CreateNewDir - DurationToString BuildEMailRecipient IsValidFileName); + DurationToString BuildEMailRecipient IsValidFileName + ShQuote); use Fcntl; @@ -196,4 +197,27 @@ sub IsValidFileName($) return $FileName !~ m~[<>:"/\\|?*]~; } +=pod +=over 12 + +=item C<ShQuote()> + +Quotes strings so they can be used in shell commands. + +Note that this implies escaping '$'s and '`'s which may not be appropriate +in another context. + +=back +=cut + +sub ShQuote($) +{ + my ($Str)=@_; + $Str =~ s%\\%\\\\%g; + $Str =~ s%\$%\\\$%g; + $Str =~ s%\"%\\\"%g; + $Str =~ s%\`%\\\`%g; + return "\"$Str\""; +} + 1;

1 0

Francois Gouget : testbot/build: Let the TestBot deal with logging.
by Alexandre Julliard 20 Jun '18

20 Jun '18

Module: tools Branch: master Commit: 170fe0ce2b8c0ca1d30f9129a4c1aaf0430588c5 URL: https://source.winehq.org/git/tools.git/?a=commit;h=170fe0ce2b8c0ca1d30f912… Author: Francois Gouget <fgouget(a)codeweavers.com> Date: Wed Jun 20 02:44:15 2018 +0200 testbot/build: Let the TestBot deal with logging. The Engine-side scripts (WineRun*.pl) already redirect stdout and stderr to the appropriate log. So it does not make sense to delete that log and then add new redirects for every command started from the client scripts. Remove FatalError() since it's not used in Reconfig.pl. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- testbot/bin/build/Build.pl | 42 ++++++++++----------------------- testbot/bin/build/Reconfig.pl | 55 +++++++++++++------------------------------ 2 files changed, 29 insertions(+), 68 deletions(-) diff --git a/testbot/bin/build/Build.pl b/testbot/bin/build/Build.pl index e979955..c23bb87 100755 --- a/testbot/bin/build/Build.pl +++ b/testbot/bin/build/Build.pl @@ -49,27 +49,19 @@ use WineTestBot::PatchUtils; use WineTestBot::Utils; -my $LogFileName = "$LogDir/Build.log"; - sub InfoMsg(@) { - my $OldUMask = umask(002); - if (open(my $Log, ">>", $LogFileName)) - { - print $Log @_; - close($Log); - } - umask($OldUMask); + print @_; } sub LogMsg(@) { - InfoMsg "Build: ", @_; + print "Build: ", @_; } sub FatalError(@) { - LogMsg @_; + print STDERR @_; exit 1; } @@ -90,10 +82,9 @@ sub ApplyPatch($) my ($PatchFile) = @_; InfoMsg "Applying patch\n"; - system("( cd $DataDir/wine && set -x && " . - " git apply --verbose ". ShQuote($PatchFile) ." && ". - " git add -A " . - ") >>$LogFileName 2>&1"); + system("cd $DataDir/wine && set -x && ". + "git apply --verbose ". ShQuote($PatchFile) ." && ". + "git add -A"); if ($? != 0) { LogMsg "Patch failed to apply\n"; @@ -104,9 +95,7 @@ sub ApplyPatch($) if ($Impacts->{Makefiles}) { InfoMsg "\nRunning make_makefiles\n"; - system("( cd $DataDir/wine && set -x && " . - " ./tools/make_makefiles " . - ") >>$LogFileName 2>&1"); + system("cd $DataDir/wine && set -x && ./tools/make_makefiles"); if ($? != 0) { LogMsg "make_makefiles failed\n"; @@ -117,9 +106,7 @@ sub ApplyPatch($) if ($Impacts->{Autoconf} && !$Impacts->{HasConfigure}) { InfoMsg "\nRunning autoconf\n"; - system("( cd $DataDir/wine && set -x && " . - " autoconf " . - ") >>$LogFileName 2>&1"); + system("cd $DataDir/wine && set -x && autoconf"); if ($? != 0) { LogMsg "Autoconf failed\n"; @@ -135,9 +122,8 @@ sub BuildNative() mkdir "$DataDir/build-native" if (! -d "$DataDir/build-native"); InfoMsg "\nRebuilding native tools\n"; - system("( cd $DataDir/build-native && set -x && " . - " time make -j$ncpus __tooldeps__ " . - ") >>$LogFileName 2>&1"); + system("cd $DataDir/build-native && set -x && ". + "time make -j$ncpus __tooldeps__"); if ($? != 0) { LogMsg "Rebuild of native tools failed\n"; @@ -161,9 +147,8 @@ sub BuildTestExecutables($$) } InfoMsg "\nBuilding the $Bits-bit test executable(s)\n"; - system("( cd $DataDir/build-mingw$Bits && set -x && " . - " time make -j$ncpus ". join(" ", sort @BuildDirs) . - ") >>$LogFileName 2>&1"); + system("cd $DataDir/build-mingw$Bits && set -x && ". + "time make -j$ncpus ". join(" ", sort @BuildDirs)); if ($? != 0) { LogMsg "Rebuild of $Bits-bit crossbuild failed\n"; @@ -186,9 +171,6 @@ sub BuildTestExecutables($$) $ENV{PATH} = "/usr/lib/ccache:/usr/bin:/bin"; delete $ENV{ENV}; -# Start with a clean logfile -unlink($LogFileName); - my ($PatchFile, $BitIndicators); if (@ARGV == 2) { diff --git a/testbot/bin/build/Reconfig.pl b/testbot/bin/build/Reconfig.pl index c950d49..2da2f54 100755 --- a/testbot/bin/build/Reconfig.pl +++ b/testbot/bin/build/Reconfig.pl @@ -43,28 +43,15 @@ sub BEGIN use WineTestBot::Config; use WineTestBot::PatchUtils; -my $LogFileName = "$LogDir/Reconfig.log"; sub InfoMsg(@) { - my $OldUMask = umask(002); - if (open(my $Log, ">>", $LogFileName)) - { - print $Log @_; - close($Log); - } - umask($OldUMask); + print @_; } sub LogMsg(@) { - InfoMsg "Reconfig: ", @_; -} - -sub FatalError(@) -{ - LogMsg @_; - exit 1; + print "Reconfig: ", @_; } my $ncpus; @@ -86,9 +73,8 @@ sub BuildTestAgentd() if (! -x "$BinDir/build/testagentd") { InfoMsg "\nBuilding the native testagentd\n"; - system("( cd $::RootDir/src/testagentd && set -x && " . - " time make -j$ncpus build " . - ") >>$LogFileName 2>&1"); + system("cd $::RootDir/src/testagentd && set -x && ". + "time make -j$ncpus build"); if ($? != 0) { LogMsg "Build testagentd failed\n"; @@ -97,9 +83,8 @@ sub BuildTestAgentd() } InfoMsg "\nRebuilding the Windows TestAgentd\n"; - system("( cd $::RootDir/src/testagentd && set -x && " . - " time make -j$ncpus iso " . - ") >>$LogFileName 2>&1"); + system("cd $::RootDir/src/testagentd && set -x && ". + "time make -j$ncpus iso"); if ($? != 0) { LogMsg "Build winetestbot.iso failed\n"; @@ -112,9 +97,8 @@ sub BuildTestAgentd() sub BuildTestLauncher() { InfoMsg "\nRebuilding TestLauncher\n"; - system("( cd $::RootDir/src/TestLauncher && set -x && " . - " time make -j$ncpus" . - ") >>$LogFileName 2>&1"); + system("cd $::RootDir/src/TestLauncher && set -x && ". + "time make -j$ncpus"); if ($? != 0) { LogMsg "Build TestLauncher failed\n"; @@ -127,7 +111,7 @@ sub BuildTestLauncher() sub GitPull() { InfoMsg "\nUpdating the Wine source\n"; - system("cd $DataDir/wine && git pull >>$LogFileName 2>&1"); + system("cd $DataDir/wine && git pull"); if ($? != 0) { LogMsg "Git pull failed\n"; @@ -150,11 +134,10 @@ sub BuildNative() # Rebuild from scratch to make sure cruft will not accumulate InfoMsg "\nRebuilding native tools\n"; - system("( cd $DataDir/build-native && set -x && " . - " rm -rf * && " . - " time ../wine/configure --enable-win64 --without-x --without-freetype --disable-winetest && " . - " time make -j$ncpus __tooldeps__ " . - ") >>$LogFileName 2>&1"); + system("cd $DataDir/build-native && set -x && ". + "rm -rf * && ". + "time ../wine/configure --enable-win64 --without-x --without-freetype --disable-winetest && ". + "time make -j$ncpus __tooldeps__"); if ($? != 0) { @@ -174,11 +157,10 @@ sub BuildCross($) # Rebuild from scratch to make sure cruft will not accumulate InfoMsg "\nRebuilding the $Bits-bit test executables\n"; - system("( cd $DataDir/build-mingw$Bits && set -x && " . - " rm -rf * && " . - " time ../wine/configure --host=$Host --with-wine-tools=../build-native --without-x --without-freetype --disable-winetest && " . - " time make -j$ncpus buildtests" . - ") >>$LogFileName 2>&1"); + system("cd $DataDir/build-mingw$Bits && set -x && ". + "rm -rf * && ". + "time ../wine/configure --host=$Host --with-wine-tools=../build-native --without-x --without-freetype --disable-winetest && ". + "time make -j$ncpus buildtests"); if ($? != 0) { LogMsg "Build cross ($Bits bits) failed\n"; @@ -191,9 +173,6 @@ sub BuildCross($) $ENV{PATH} = "/usr/lib/ccache:/usr/bin:/bin"; delete $ENV{ENV}; -# Start with a clean logfile -unlink($LogFileName); - if (! -d "$DataDir/staging" and ! mkdir "$DataDir/staging") { LogMsg "Unable to create '$DataDir/staging': $!\n";

1 0

Francois Gouget : testbot/web: Allow showing multiple job screenshots and logs.
by Alexandre Julliard 20 Jun '18

20 Jun '18

Module: tools Branch: master Commit: b0e2d37d418a7fdb038f4f0f7c6fc0fc168a00a7 URL: https://source.winehq.org/git/tools.git/?a=commit;h=b0e2d37d418a7fdb038f4f0… Author: Francois Gouget <fgouget(a)codeweavers.com> Date: Wed Jun 20 02:46:51 2018 +0200 testbot/web: Allow showing multiple job screenshots and logs. It's now possible to have the job details page show multiple screenshots and multiple logs. Once a screenshot / log is shown, the link can be clicked again to hide it. The code can also more easily be extended to handle more log files. The code inserting the links is simpler too. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- testbot/bin/WineRunTask.pl | 2 +- testbot/web/JobDetails.pl | 156 ++++++++++++++++++++++++++++----------------- 2 files changed, 97 insertions(+), 61 deletions(-) diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl index 3d8113f..c4ca578 100755 --- a/testbot/bin/WineRunTask.pl +++ b/testbot/bin/WineRunTask.pl @@ -448,7 +448,7 @@ elsif ($Step->Type eq "suite") if (defined($WebHostName)) { my $StepTask = 100 * $StepNo + $TaskNo; - $Script .= "-u \"http://$WebHostName/JobDetails.pl?Key=$JobId&scrshot_$StepTask=1#k$StepTask\""; + $Script .= "-u \"http://$WebHostName/JobDetails.pl?Key=$JobId&s$StepTask=1#k$StepTask\""; } my $Info = $VM->Description ? $VM->Description : ""; if ($VM->Details) diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl index 70d8b59..f829407 100644 --- a/testbot/web/JobDetails.pl +++ b/testbot/web/JobDetails.pl @@ -228,12 +228,95 @@ sub GeneratePage($) $self->SUPER::GeneratePage(); } +my %MILogLabels = ( + "log" => "task log", + "log.old" => "old logs", +); + +sub InitMoreInfo($) +{ + my ($self) = @_; + + my $More = $self->{More} = {}; + my $Keys = $self->SortKeys(undef, $self->{Collection}->GetKeys()); + foreach my $Key (@$Keys) + { + my $StepTask = $self->{Collection}->GetItem($Key); + $More->{$Key}->{Screenshot} = $self->GetParam("s$Key"); + + my $Value = $self->GetParam("f$Key"); + my $TaskDir = $StepTask->GetTaskDir(); + foreach my $Log ("log", "log.old") + { + if (!-f "$TaskDir/$Log" or -z "$TaskDir/$Log") + { + my $Err = $Log; + next if ($Err !~ s/^log/err/ or !-f "$TaskDir/$Err" or -z "$TaskDir/$Err"); + } + push @{$More->{$Key}->{Logs}}, $Log; + + $More->{$Key}->{Full} = $Log if (uri_escape($Log) eq $Value); + } + $More->{$Key}->{Full} ||= ""; + } +} + +sub GenerateMoreInfoLink($$$;$) +{ + my ($self, $LinkKey, $Label, $Set, $Value) = @_; + + my $Url = $ENV{"SCRIPT_NAME"} ."?Key=". uri_escape($self->{JobId}); + + my $Action = "Show"; + foreach my $Key (sort keys %{$self->{More}}) + { + my $MoreInfo = $self->{More}->{$Key}; + if ($Key eq $LinkKey and $Set eq "Screenshot") + { + if (!$MoreInfo->{Screenshot}) + { + $Url .= "&s$Key=1"; + } + else + { + $Action = "Hide"; + } + } + else + { + $Url .= "&s$Key=1" if ($MoreInfo->{Screenshot}); + } + + if ($Key eq $LinkKey and $Set eq "Full") + { + if ($MoreInfo->{Full} ne $Value) + { + $Url .= "&f$Key=". uri_escape($Value); + } + else + { + $Action = "Hide"; + } + } + else + { + $Url .= "&f$Key=". uri_escape($MoreInfo->{Full}) if ($MoreInfo->{Full}); + } + } + $Url .= "#k" . uri_escape($LinkKey); + + print "<div class='TaskMoreInfoLink'><a href='", + $self->CGI->escapeHTML($Url), "'>$Action $Label</a></div>\n"; +} + sub GenerateBody($) { my ($self) = @_; $self->SUPER::GenerateBody(); + $self->InitMoreInfo(); + print "<div class='Content'>\n"; my $Keys = $self->SortKeys(undef, $self->{Collection}->GetKeys()); foreach my $Key (@$Keys) @@ -249,18 +332,11 @@ sub GenerateBody($) $self->CGI->escapeHTML($VM->Details || "No details!"), "</details>\n"; - my $FullLogParamName = "log_$Key"; - my $FullLog = $self->GetParam($FullLogParamName); - $FullLog = "" if ($FullLog !~ /^[12]$/); - - my $ScreenshotParamName = "scrshot_$Key"; - my $Screenshot = $self->GetParam($ScreenshotParamName); - $Screenshot = "" if ($Screenshot ne "1"); - + my $MoreInfo = $self->{More}->{$Key}; print "<div class='TaskMoreInfoLinks'>\n"; if (-r "$TaskDir/screenshot.png") { - if ($Screenshot) + if ($MoreInfo->{Screenshot}) { my $URI = "/Screenshot.pl?JobKey=" . uri_escape($self->{JobId}) . "&StepKey=" . uri_escape($StepTask->StepNo) . @@ -268,59 +344,19 @@ sub GenerateBody($) print "<div class='Screenshot'><img src='" . $self->CGI->escapeHTML($URI) . "' alt='Screenshot' /></div>\n"; } - else - { - my $URI = $ENV{"SCRIPT_NAME"} . "?Key=" . uri_escape($self->{JobId}) . - "&$ScreenshotParamName=1"; - $URI .= "&$FullLogParamName=$FullLog"; - $URI .= "#k" . uri_escape($Key); - print "<div class='TaskMoreInfoLink'><a href='" . - $self->CGI->escapeHTML($URI) . - "'>Show final screenshot</a></div>"; - print "\n"; - } + $self->GenerateMoreInfoLink($Key, "final screenshot", "Screenshot"); } - my $LogName = "$TaskDir/log"; - my $ErrName = "$TaskDir/err"; - if (-r $LogName and $FullLog != "1") - { - my $URI = $ENV{"SCRIPT_NAME"} . "?Key=" . uri_escape($self->{JobId}) . - "&$FullLogParamName=1"; - $URI .= "&$ScreenshotParamName=$Screenshot"; - $URI .= "#k" . uri_escape($Key); - print "<div class='TaskMoreInfoLink'><a href='" . - $self->CGI->escapeHTML($URI) . - "'>Show full log</a></div>\n"; - } - if ((-r $LogName or -r $ErrName) and $FullLog == "2") + foreach my $Log (@{$MoreInfo->{Logs}}) { - my $URI = $ENV{"SCRIPT_NAME"} . "?Key=" . uri_escape($self->{JobId}); - $URI .= "&$ScreenshotParamName=$Screenshot"; - $URI .= "#k" . uri_escape($Key); - print "<div class='TaskMoreInfoLink'><a href='" . - $self->CGI->escapeHTML($URI) . - "'>Show latest log</a></div>\n"; - } - if ((-r "$LogName.old" or -r "$ErrName.old") and $FullLog != "2") - { - my $URI = $ENV{"SCRIPT_NAME"} . "?Key=" . uri_escape($self->{JobId}) . - "&$FullLogParamName=2"; - $URI .= "&$ScreenshotParamName=$Screenshot"; - $URI .= "#k" . uri_escape($Key); - print "<div class='TaskMoreInfoLink'><a href='" . - $self->CGI->escapeHTML($URI) . - "'>Show old logs</a></div>\n"; + $self->GenerateMoreInfoLink($Key, $MILogLabels{$Log}, "Full", $Log); } print "</div>\n"; - if ($FullLog eq "2") - { - $LogName .= ".old"; - $ErrName .= ".old"; - } + my $LogName = $MoreInfo->{Full} || $MoreInfo->{Logs}->[0] || "log"; + my $ErrName = $LogName eq "log.old" ? "err.old" : "err"; - if (open LOGFILE, "<$LogName") + if (open LOGFILE, "<", "$TaskDir/$LogName") { my $HasLogEntries = !1; my $First = 1; @@ -335,13 +371,13 @@ sub GenerateBody($) { $CurrentDll = $1; } - if ($FullLog || + if ($MoreInfo->{Full} || $Line =~ m/: Test (?:failed|succeeded inside todo block): / || $Line =~ m/Fatal: test '[^']+' does not exist/ || $Line =~ m/ done $258$/ || $Line =~ m/: unhandled exception [0-9a-fA-F]{8} at /) { - if ($PrintedDll ne $CurrentDll && ! $FullLog) + if ($PrintedDll ne $CurrentDll && !$MoreInfo->{Full}) { if ($First) { @@ -359,7 +395,7 @@ sub GenerateBody($) print "<pre><code>"; $First = !1; } - if (! $FullLog && $Line =~ m/^[^:]+:([^:]*)(?::[0-9a-f]+)? done $258$/) + if (!$MoreInfo->{Full} && $Line =~ m/^[^:]+:([^:]*)(?::[0-9a-f]+)? done $258$/) { my $Unit = $1 ne "" ? "$1: " : ""; print "${Unit}Timeout\n"; @@ -372,7 +408,7 @@ sub GenerateBody($) } close LOGFILE; - if (open ERRFILE, "<$ErrName") + if (open ERRFILE, "<", "$TaskDir/$ErrName") { $CurrentDll = "*err*"; while (defined($Line = <ERRFILE>)) @@ -409,7 +445,7 @@ sub GenerateBody($) " failures found" : "Empty log"; } } - elsif (open ERRFILE, "<$ErrName") + elsif (open ERRFILE, "<", "$TaskDir/$ErrName") { my $HasErrEntries = !1; my $Line;

1 0