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

[PATCH 2/2] testbot/build: Add proper command line options to Build.pl and Reconfig.pl.

by Francois Gouget

This makes it much easier to test the scripts directly on the VM. This also makes it possible to test one aspect of the scripts without having to go through the whole process. In particular the --no-rm Reconfig.pl option speeds up testing by avoiding rebuilding from scratch. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/build/Build.pl | 175 +++++++++++++++++++++++----------- testbot/bin/build/Reconfig.pl | 137 +++++++++++++++++++++++--- 2 files changed, 241 insertions(+), 71 deletions(-) diff --git a/testbot/bin/build/Build.pl b/testbot/bin/build/Build.pl index 17ae74ff5..4df107005 100755 --- a/testbot/bin/build/Build.pl +++ b/testbot/bin/build/Build.pl @@ -43,12 +43,19 @@ sub BEGIN } $::BuildEnv = 1; } +my $Name0 = $0; +$Name0 =~ s+^.*/++; + use WineTestBot::Config; use WineTestBot::PatchUtils; use WineTestBot::Utils; +# +# Logging and error handling helpers +# + sub InfoMsg(@) { print @_; @@ -59,12 +66,16 @@ sub LogMsg(@) print "Build: ", @_; } -sub FatalError(@) +sub Error(@) { - print STDERR @_; - exit 1; + print STDERR "$Name0:error: ", @_; } + +# +# Build helpers +# + my $ncpus; sub CountCPUs() { @@ -133,9 +144,11 @@ sub BuildNative() return 1; } -sub BuildTestExecutables($$) +sub BuildTestExecutables($$$) { - my ($Impacts, $Bits) = @_; + my ($Targets, $Impacts, $Bits) = @_; + + return 1 if (!$Targets->{"exe$Bits"}); my (@BuildDirs, @TestExes); foreach my $TestInfo (values %{$Impacts->{Tests}}) @@ -168,62 +181,113 @@ sub BuildTestExecutables($$) return $Success; } + +# +# Setup and command line processing +# + $ENV{PATH} = "/usr/lib/ccache:/usr/bin:/bin"; delete $ENV{ENV}; -my ($PatchFile, $BitIndicators); -if (@ARGV == 2) -{ - ($PatchFile, $BitIndicators) = @ARGV; -} -else -{ - # FIXME Remove support for the legacy parameters - my ($_PatchType, $_BaseName); - ($PatchFile, $_PatchType, $_BaseName, $BitIndicators) = @ARGV; -} -if (! $PatchFile || !$BitIndicators) -{ - FatalError "Usage: Build.pl <patchfile> <bits>\n"; -} - -# Verify parameters -if (!IsValidFileName($PatchFile)) -{ - FatalError "The patch filename '$PatchFile' contains invalid characters\n"; -} -$PatchFile = "$DataDir/staging/$PatchFile"; -if (!-r $PatchFile) -{ - FatalError "Patch file '$PatchFile' is not readable\n"; -} +my %AllTargets; +map { $AllTargets{$_} = 1 } qw(exe32 exe64); -my ($Run32, $Run64); -if ($BitIndicators =~ m/^([\d,]+)$/) +my ($Usage, $PatchFile, $TargetList); +my $IgnoreNext = 0; # FIXME Backward compatibility +while (@ARGV) { - foreach my $BitsValue (split /,/, $1) + my $Arg = shift @ARGV; + if ($Arg =~ /^patch(?:dlls|programs)$/) { - if ($BitsValue eq "32") - { - $Run32 = 1; - } - elsif ($BitsValue eq "64") + $IgnoreNext ||= 1; # Ignore this legacy parameter + } + elsif ($IgnoreNext == 1) + { + $IgnoreNext = 2; # Ignore this legacy parameter + } + elsif ($Arg =~ /^(?:-\?|-h|--help)$/) + { + $Usage = 0; + last; + } + elsif ($Arg =~ /^-/) + { + Error "unknown option '$Arg'\n"; + $Usage = 2; + last; + } + elsif (!defined $PatchFile) + { + if (IsValidFileName($Arg)) { - $Run64 = 1; + $PatchFile = "$DataDir/staging/$Arg"; + if (!-r $PatchFile) + { + Error "patch file '$Arg' is not readable\n"; + $Usage = 2; + } } else { - FatalError "Invalid number of bits $BitsValue\n"; + Error "the patch filename '$Arg' contains invalid characters\n"; + $Usage = 2; + last; } } - if (!$Run32 && !$Run64) + elsif (!defined $TargetList) { - FatalError "Specify at least one of 32 or 64 bits\n"; + $TargetList = $Arg; + } + else + { + Error "unexpected argument '$Arg'\n"; + $Usage = 2; + last; + } +} + +# Check and untaint parameters +my $Targets; +if (!defined $Usage) +{ + if (!defined $PatchFile) + { + Error "you must specify a patch to apply\n"; + $Usage = 2; + } + + $TargetList = join(",", keys %AllTargets) if (!defined $TargetList); + foreach my $Target (split /,/, $TargetList) + { + $Target = "exe$1" if ($Target =~ /^(32|64)$/); + if (!$AllTargets{$Target}) + { + Error "invalid target name $Target\n"; + $Usage = 2; + last; + } + $Targets->{$Target} = 1; } } -else +if (defined $Usage) { - FatalError "Invalid number of bits $BitIndicators\n"; + if ($Usage) + { + Error "try '$Name0 --help' for more information\n"; + exit $Usage; + } + print "Usage: $Name0 [--help] PATCHFILE TARGETS\n"; + print "\n"; + print "Applies the specified patch and rebuilds the Wine test executables.\n"; + print "\n"; + print "Where:\n"; + print " PATCHFILE Is the staging file containing the patch to build.\n"; + print " TARGETS Is a comma-separated list of build targets. By default every\n"; + print " target is run.\n"; + print " - exe32: Rebuild the 32 bit Windows test executables.\n"; + print " - exe64: Rebuild the 64 bit Windows test executables.\n"; + print " --help Shows this usage message.\n"; + exit 0; } if ($DataDir =~ /'/) @@ -232,20 +296,19 @@ if ($DataDir =~ /'/) exit(1); } -my $Impacts = ApplyPatch($PatchFile); -exit(1) if (!$Impacts); + +# +# Run the builds +# CountCPUs(); -if ($Impacts->{WineBuild} and !BuildNative()) -{ - exit(1); -} -if ($Run32 && !BuildTestExecutables($Impacts, 32)) -{ - exit(1); -} -if ($Run64 && !BuildTestExecutables($Impacts, 64)) +my $Impacts = ApplyPatch($PatchFile); + +if (!$Impacts or + ($Impacts->{WineBuild} and !BuildNative()) or + !BuildTestExecutables($Targets, $Impacts, 32) or + !BuildTestExecutables($Targets, $Impacts, 64)) { exit(1); } diff --git a/testbot/bin/build/Reconfig.pl b/testbot/bin/build/Reconfig.pl index 5ecbc2af3..450de0e83 100755 --- a/testbot/bin/build/Reconfig.pl +++ b/testbot/bin/build/Reconfig.pl @@ -38,12 +38,19 @@ sub BEGIN unshift @INC, "$::RootDir/lib"; } $::BuildEnv = 1; - } +} +my $Name0 = $0; +$Name0 =~ s+^.*/++; + use WineTestBot::Config; use WineTestBot::PatchUtils; +# +# Logging and error handling helpers +# + sub InfoMsg(@) { print @_; @@ -54,6 +61,16 @@ sub LogMsg(@) print "Reconfig: ", @_; } +sub Error(@) +{ + print STDERR "$Name0:error: ", @_; +} + + +# +# Build helpers +# + my $ncpus; sub CountCPUs() { @@ -108,8 +125,11 @@ sub BuildTestLauncher() return 1; } -sub GitPull() +sub GitPull($) { + my ($Targets) = @_; + return 1 if (!$Targets->{update}); + InfoMsg "\nUpdating the Wine source\n"; system("cd '$DataDir/wine' && git pull"); if ($? != 0) @@ -128,14 +148,17 @@ sub GitPull() return 1; } -sub BuildNative() +sub BuildNative($$) { + my ($Targets, $NoRm) = @_; + + return 1 if (!$Targets->{native}); mkdir "$DataDir/build-native" if (! -d "$DataDir/build-native"); # Rebuild from scratch to make sure cruft will not accumulate InfoMsg "\nRebuilding native tools\n"; system("cd '$DataDir/build-native' && set -x && ". - "rm -rf * && ". + ($NoRm ? "" : "rm -rf * && ") . "time ../wine/configure --enable-win64 --without-x --without-freetype --disable-winetest && ". "time make -j$ncpus __tooldeps__"); @@ -148,17 +171,18 @@ sub BuildNative() return 1; } -sub BuildCross($) +sub BuildCross($$$) { - my ($Bits) = @_; + my ($Targets, $NoRm, $Bits) = @_; - my $Host = ($Bits == 64 ? "x86_64-w64-mingw32" : "i686-w64-mingw32"); - mkdir "$DataDir/build-mingw$Bits" if (! -d "$DataDir/build-mingw$Bits"); + return 1 if (!$Targets->{"exe$Bits"}); + mkdir "$DataDir/build-mingw$Bits" if (!-d "$DataDir/build-mingw$Bits"); # Rebuild from scratch to make sure cruft will not accumulate InfoMsg "\nRebuilding the $Bits-bit test executables\n"; + my $Host = ($Bits == 64 ? "x86_64-w64-mingw32" : "i686-w64-mingw32"); system("cd '$DataDir/build-mingw$Bits' && set -x && ". - "rm -rf * && ". + ($NoRm ? "" : "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) @@ -170,9 +194,87 @@ sub BuildCross($) return 1; } + +# +# Setup and command line processing +# + $ENV{PATH} = "/usr/lib/ccache:/usr/bin:/bin"; delete $ENV{ENV}; +my %AllTargets; +map { $AllTargets{$_} = 1 } qw(update native exe32 exe64); + +my ($Usage, $TargetList, $NoRm); +while (@ARGV) +{ + my $Arg = shift @ARGV; + if ($Arg eq "--no-rm") + { + $NoRm = 1; + } + elsif ($Arg =~ /^(?:-\?|-h|--help)$/) + { + $Usage = 0; + last; + } + elsif ($Arg =~ /^-/) + { + Error "unknown option '$Arg'\n"; + $Usage = 2; + last; + } + elsif (!defined $TargetList) + { + $TargetList = $Arg; + } + else + { + Error "unexpected argument '$Arg'\n"; + $Usage = 2; + last; + } +} + +# Check and untaint parameters +my $Targets; +if (!defined $Usage) +{ + $TargetList = join(",", keys %AllTargets) if (!defined $TargetList); + foreach my $Target (split /,/, $TargetList) + { + if (!$AllTargets{$Target}) + { + Error "invalid target name $Target\n"; + $Usage = 2; + last; + } + $Targets->{$Target} = 1; + } +} +if (defined $Usage) +{ + if ($Usage) + { + Error "try '$Name0 --help' for more information\n"; + exit $Usage; + } + print "Usage: $Name0 [--no-rm] [--help] [TARGETS]\n"; + print "\n"; + print "Updates Wine to the latest version and recompiles it so the host is ready to build executables for the Windows tests.\n"; + print "\n"; + print "Where:\n"; + print " TARGETS Is a comma-separated list of reconfiguration targets. By default\n"; + print " every target is run.\n"; + print " - update: Update Wine's source code.\n"; + print " - native: Rebuild the native Wine tools.\n"; + print " - exe32: Rebuild the 32 bit Windows test executables.\n"; + print " - exe64: Rebuild the 64 bit Windows test executables.\n"; + print " --no-rm Don't rebuild from scratch.\n"; + print " --help Shows this usage message.\n"; + exit 0; +} + if ($DataDir =~ /'/) { LogMsg "The install path contains invalid characters\n"; @@ -184,14 +286,19 @@ if (! -d "$DataDir/staging" and ! mkdir "$DataDir/staging") exit(1); } + +# +# Run the builds +# + CountCPUs(); -if (!BuildTestAgentd() || - !BuildTestLauncher() || - !GitPull() || - !BuildNative() || - !BuildCross(32) || - !BuildCross(64)) +if (!BuildTestAgentd() or + !BuildTestLauncher() or + !GitPull($Targets) or + !BuildNative($Targets, $NoRm) or + !BuildCross($Targets, $NoRm, 32) or + !BuildCross($Targets, $NoRm, 64)) { exit(1); } -- 2.17.1

June 21, 2018

[PATCH 1/2] testbot/build: Quote the paths in the shell commands.

by Francois Gouget

Also check that $DataDir does not contain single quotes, thus ensuring our simple quoting scheme works. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- A more absolutist approach would be to use ShQuote() everywhere which would remove the single quote restriction. But that would compromise readability. So this solution seems like a good compromise. Also this is mostly cosmetic in the first place since the administrator would presumably not put spaces, stars, dollar signs or single quotes in the $DataDir path. Still I much prefer having proper quoting in my shell scripts. testbot/bin/build/Build.pl | 16 +++++++++++----- testbot/bin/build/Reconfig.pl | 17 +++++++++++------ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/testbot/bin/build/Build.pl b/testbot/bin/build/Build.pl index c23bb8780..17ae74ff5 100755 --- a/testbot/bin/build/Build.pl +++ b/testbot/bin/build/Build.pl @@ -82,7 +82,7 @@ sub ApplyPatch($) my ($PatchFile) = @_; InfoMsg "Applying patch\n"; - system("cd $DataDir/wine && set -x && ". + system("cd '$DataDir/wine' && set -x && ". "git apply --verbose ". ShQuote($PatchFile) ." && ". "git add -A"); if ($? != 0) @@ -95,7 +95,7 @@ sub ApplyPatch($) if ($Impacts->{Makefiles}) { InfoMsg "\nRunning make_makefiles\n"; - system("cd $DataDir/wine && set -x && ./tools/make_makefiles"); + system("cd '$DataDir/wine' && set -x && ./tools/make_makefiles"); if ($? != 0) { LogMsg "make_makefiles failed\n"; @@ -106,7 +106,7 @@ sub ApplyPatch($) if ($Impacts->{Autoconf} && !$Impacts->{HasConfigure}) { InfoMsg "\nRunning autoconf\n"; - system("cd $DataDir/wine && set -x && autoconf"); + system("cd '$DataDir/wine' && set -x && autoconf"); if ($? != 0) { LogMsg "Autoconf failed\n"; @@ -122,7 +122,7 @@ sub BuildNative() mkdir "$DataDir/build-native" if (! -d "$DataDir/build-native"); InfoMsg "\nRebuilding native tools\n"; - system("cd $DataDir/build-native && set -x && ". + system("cd '$DataDir/build-native' && set -x && ". "time make -j$ncpus __tooldeps__"); if ($? != 0) { @@ -147,7 +147,7 @@ sub BuildTestExecutables($$) } InfoMsg "\nBuilding the $Bits-bit test executable(s)\n"; - system("cd $DataDir/build-mingw$Bits && set -x && ". + system("cd '$DataDir/build-mingw$Bits' && set -x && ". "time make -j$ncpus ". join(" ", sort @BuildDirs)); if ($? != 0) { @@ -226,6 +226,12 @@ else FatalError "Invalid number of bits $BitIndicators\n"; } +if ($DataDir =~ /'/) +{ + LogMsg "The install path contains invalid characters\n"; + exit(1); +} + my $Impacts = ApplyPatch($PatchFile); exit(1) if (!$Impacts); diff --git a/testbot/bin/build/Reconfig.pl b/testbot/bin/build/Reconfig.pl index 2da2f54a4..5ecbc2af3 100755 --- a/testbot/bin/build/Reconfig.pl +++ b/testbot/bin/build/Reconfig.pl @@ -73,7 +73,7 @@ sub BuildTestAgentd() if (! -x "$BinDir/build/testagentd") { InfoMsg "\nBuilding the native testagentd\n"; - system("cd $::RootDir/src/testagentd && set -x && ". + system("cd '$::RootDir/src/testagentd' && set -x && ". "time make -j$ncpus build"); if ($? != 0) { @@ -83,7 +83,7 @@ sub BuildTestAgentd() } InfoMsg "\nRebuilding the Windows TestAgentd\n"; - system("cd $::RootDir/src/testagentd && set -x && ". + system("cd '$::RootDir/src/testagentd' && set -x && ". "time make -j$ncpus iso"); if ($? != 0) { @@ -97,7 +97,7 @@ sub BuildTestAgentd() sub BuildTestLauncher() { InfoMsg "\nRebuilding TestLauncher\n"; - system("cd $::RootDir/src/TestLauncher && set -x && ". + system("cd '$::RootDir/src/TestLauncher' && set -x && ". "time make -j$ncpus"); if ($? != 0) { @@ -111,7 +111,7 @@ sub BuildTestLauncher() sub GitPull() { InfoMsg "\nUpdating the Wine source\n"; - system("cd $DataDir/wine && git pull"); + system("cd '$DataDir/wine' && git pull"); if ($? != 0) { LogMsg "Git pull failed\n"; @@ -134,7 +134,7 @@ sub BuildNative() # Rebuild from scratch to make sure cruft will not accumulate InfoMsg "\nRebuilding native tools\n"; - system("cd $DataDir/build-native && set -x && ". + 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__"); @@ -157,7 +157,7 @@ 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 && ". + 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"); @@ -173,6 +173,11 @@ sub BuildCross($) $ENV{PATH} = "/usr/lib/ccache:/usr/bin:/bin"; delete $ENV{ENV}; +if ($DataDir =~ /'/) +{ + LogMsg "The install path contains invalid characters\n"; + exit(1); +} if (! -d "$DataDir/staging" and ! mkdir "$DataDir/staging") { LogMsg "Unable to create '$DataDir/staging': $!\n"; -- 2.17.1

June 21, 2018

[PATCH 3/3] testbot/WineRunTask: Rename the Windows test reports.

by Francois Gouget

Keep 'log' for the task log messages (such as the compilation log) and store the actual (WineTest) test report in '.report' files. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- A coming patch will put the output of the batch file in the log file (after I have checked that the TestBot VMs have the version of TestAgentd where redirections work correctly). testbot/bin/WineRunTask.pl | 16 ++++++++-------- testbot/web/JobDetails.pl | 9 ++++++--- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl index 7f92b8858..3f5a3ee8d 100755 --- a/testbot/bin/WineRunTask.pl +++ b/testbot/bin/WineRunTask.pl @@ -195,7 +195,7 @@ my $OldUMask = umask(002); my $TaskDir = $Task->CreateDir(); umask($OldUMask); my $VM = $Task->VM; -my $RptFileName = $VM->Name . ".rpt"; +my $RptFileName = $Step->FileType .".report"; my $Start = Time(); @@ -292,13 +292,13 @@ sub WrapUpAndExit($;$$$) if ($Step->Type eq 'suite' and $Status eq 'completed' and !$TimedOut) { # Keep the old report if the new one is missing - if (-f "$TaskDir/log" and !-z "$TaskDir/log") + if (-f "$TaskDir/$RptFileName" and !-z "$TaskDir/$RptFileName") { # Update the reference VM suite results for WineSendLog.pl my $LatestBaseName = join("", "$DataDir/latest/", $Task->VM->Name, "_", $Step->FileType eq "exe64" ? "64" : "32"); unlink("$LatestBaseName.log"); - link("$TaskDir/log", "$LatestBaseName.log"); + link("$TaskDir/$RptFileName", "$LatestBaseName.log"); unlink("$LatestBaseName.err"); if (-f "$TaskDir/err" and !-z "$TaskDir/err") { @@ -518,11 +518,11 @@ if (!defined $TA->Wait($Pid, $Timeout, $Keepalive)) } my $TimedOut; -Debug(Elapsed($Start), " Retrieving the report file to 'log'\n"); -if ($TA->GetFile($RptFileName, "$TaskDir/log")) +Debug(Elapsed($Start), " Retrieving the report file to '$RptFileName'\n"); +if ($TA->GetFile($RptFileName, "$TaskDir/$RptFileName")) { - chmod 0664, "$TaskDir/log"; - if (open(my $LogFile, "<", "$TaskDir/log")) + chmod 0664, "$TaskDir/$RptFileName"; + if (open(my $LogFile, "<", "$TaskDir/$RptFileName")) { # There is more than one test unit when running the full test suite so keep # track of the current one. Note that for the TestBot we don't count or @@ -788,7 +788,7 @@ if ($TA->GetFile($RptFileName, "$TaskDir/log")) else { $NewStatus = 'boterror'; - Error "Unable to open 'log' for reading: $!\n"; + Error "Unable to open '$RptFileName' for reading: $!\n"; LogTaskError("Unable to open the log file for reading: $!\n"); } } diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl index 3add52b4c..a1a846ff9 100644 --- a/testbot/web/JobDetails.pl +++ b/testbot/web/JobDetails.pl @@ -280,9 +280,12 @@ sub GetHtmlLine($$$) return (undef, $Line); } +my @MILogFiles = qw(exe32.report exe64.report log log.old); my %MILogLabels = ( - "log" => "task log", - "log.old" => "old logs", + "exe32.report" => "32 bit Windows report", + "exe64.report" => "64 bit Windows report", + "log" => "task log", + "log.old" => "old logs", ); sub InitMoreInfo($) @@ -298,7 +301,7 @@ sub InitMoreInfo($) my $Value = $self->GetParam("f$Key"); my $TaskDir = $StepTask->GetTaskDir(); - foreach my $Log ("log", "log.old") + foreach my $Log (@MILogFiles) { if (!-f "$TaskDir/$Log" or -z "$TaskDir/$Log") { -- 2.17.1

June 21, 2018

[PATCH 2/3] testbot/WineRunTask: Preserve the old WineTest report in case of timeout.

by Francois Gouget

If the task timed out the new report is likely quite incomplete and thus would be a poor substitute for the existing reference WineTest report. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- 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 ed190348b..7f92b8858 100755 --- a/testbot/bin/WineRunTask.pl +++ b/testbot/bin/WineRunTask.pl @@ -289,7 +289,7 @@ sub WrapUpAndExit($;$$$) $VM->Save(); } - if ($Status eq 'completed' and $Step->Type eq 'suite') + if ($Step->Type eq 'suite' and $Status eq 'completed' and !$TimedOut) { # Keep the old report if the new one is missing if (-f "$TaskDir/log" and !-z "$TaskDir/log") -- 2.17.1

June 21, 2018

[PATCH 1/3] testbot/WineRunTask: Keep the old reference report if the new one is missing.

by Francois Gouget

The WineTest results usually don't change too much from one run to the next so it's better to have a slightly out of date report than nothing at all. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/WineRunTask.pl | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl index c4ca578e2..ed190348b 100755 --- a/testbot/bin/WineRunTask.pl +++ b/testbot/bin/WineRunTask.pl @@ -291,13 +291,20 @@ sub WrapUpAndExit($;$$$) if ($Status eq 'completed' and $Step->Type eq 'suite') { - # Update the reference VM suite results for WineSendLog.pl - my $LatestBaseName = join("", "$DataDir/latest/", $Task->VM->Name, "_", - $Step->FileType eq "exe64" ? "64" : "32"); - unlink("$LatestBaseName.log"); - link("$TaskDir/log", "$LatestBaseName.log") if (-f "$TaskDir/log"); - unlink("$LatestBaseName.err"); - link("$TaskDir/err", "$LatestBaseName.err") if (-f "$TaskDir/err"); + # Keep the old report if the new one is missing + if (-f "$TaskDir/log" and !-z "$TaskDir/log") + { + # Update the reference VM suite results for WineSendLog.pl + my $LatestBaseName = join("", "$DataDir/latest/", $Task->VM->Name, "_", + $Step->FileType eq "exe64" ? "64" : "32"); + unlink("$LatestBaseName.log"); + link("$TaskDir/log", "$LatestBaseName.log"); + unlink("$LatestBaseName.err"); + if (-f "$TaskDir/err" and !-z "$TaskDir/err") + { + link("$TaskDir/err", "$LatestBaseName.err"); + } + } } my $Result = $VM->Name .": ". $VM->Status ." Status: $Status Failures: ". (defined $TestFailures ? $TestFailures : "unset"); -- 2.17.1

June 21, 2018

[PATCH] testbot/WineRun*: Don't duplicate errors and factorize the log parsing.

by Francois Gouget

Move the task log parsing functions to LogUtils.pm so the WineRun*.pl scripts can share them. Also stop copying the 'Build: *' result lines from the log to the err file. It's up to the GUI to present all the relevant lines when only showing the log summary (which it does now). Finally, there is nothing for the script to do if the patch fails to apply. So even if we did not get the full log or the task timed out somehow, the status of the task should be 'badpatch' and the other errors don't matter. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/WineRunBuild.pl | 49 ++++++++---------- testbot/bin/WineRunReconfig.pl | 49 ++++++------------ testbot/lib/WineTestBot/LogUtils.pm | 77 +++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 64 deletions(-) create mode 100644 testbot/lib/WineTestBot/LogUtils.pm diff --git a/testbot/bin/WineRunBuild.pl b/testbot/bin/WineRunBuild.pl index 1bc6cffbc..60af32c73 100755 --- a/testbot/bin/WineRunBuild.pl +++ b/testbot/bin/WineRunBuild.pl @@ -46,6 +46,7 @@ use WineTestBot::Jobs; use WineTestBot::PatchUtils; use WineTestBot::VMs; use WineTestBot::Log; +use WineTestBot::LogUtils; use WineTestBot::Engine::Notify; @@ -391,39 +392,29 @@ if (!defined $TA->Wait($Pid, $Task->Timeout, 60)) Debug(Elapsed($Start), " Retrieving 'Build.log'\n"); if ($TA->GetFile("Build.log", "$TaskDir/log")) { - if (open(my $LogFile, "<", "$TaskDir/log")) + my $Result = ParseTaskLog("$TaskDir/log", "Build"); + if ($Result eq "ok") { - # Collect and analyze the 'Build:' status line(s) - $ErrMessage ||= ""; - foreach my $Line (<$LogFile>) - { - chomp($Line); - next if ($Line !~ /^Build: (.*)$/); - if ($1 eq "ok") - { - # We must have gotten the full log and the build did succeed. - # So forget any prior error. - $NewStatus = "completed"; - $TAError = $ErrMessage = undef; - } - else - { - $NewStatus = ($1 eq "Patch failed to apply") ? "badpatch" : "badbuild"; - # Collect all the build errors (32 bit, 64 bit, etc) - $ErrMessage .= "$1\n"; - } - } - close($LogFile); - - if (!defined $NewStatus) - { - $NewStatus = "badbuild"; - $ErrMessage = "Missing build status line\n"; - } + # We must have gotten the full log and the build did succeed. + # So forget any prior error. + $NewStatus = "completed"; + $TAError = $ErrMessage = undef; + } + elsif ($Result eq "badpatch") + { + # This too is conclusive enough to ignore other errors. + $NewStatus = "badpatch"; + $TAError = $ErrMessage = undef; + } + elsif ($Result =~ s/^nolog://) + { + FatalError("$Result\n", "retry"); } else { - FatalError("Unable to open the build log for reading: $!\n", "retry"); + # If the result line is missing we probably already have an error message + # that explains why. + $NewStatus = "badbuild"; } } elsif (!defined $TAError) diff --git a/testbot/bin/WineRunReconfig.pl b/testbot/bin/WineRunReconfig.pl index d04b78061..2950799d5 100755 --- a/testbot/bin/WineRunReconfig.pl +++ b/testbot/bin/WineRunReconfig.pl @@ -45,6 +45,7 @@ use WineTestBot::Config; use WineTestBot::Jobs; use WineTestBot::VMs; use WineTestBot::Log; +use WineTestBot::LogUtils; use WineTestBot::Engine::Notify; @@ -373,45 +374,23 @@ if (!defined $TA->Wait($Pid, $Task->Timeout, 60)) Debug(Elapsed($Start), " Retrieving 'Reconfig.log'\n"); if ($TA->GetFile("Reconfig.log", "$TaskDir/log")) { - if (open(my $LogFile, "<", "$TaskDir/log")) + my $Result = ParseTaskLog("$TaskDir/log", "Reconfig"); + if ($Result eq "ok") { - # Collect and analyze the 'Reconfig:' status line(s). - my $LogErrors; - foreach my $Line (<$LogFile>) - { - chomp($Line); - next if ($Line !~ /^Reconfig: (.*)$/); - # Add the error message or an empty string for 'ok' - $LogErrors = ($LogErrors || "") . ($1 ne "ok" ? "$1\n" : ""); - } - close($LogFile); - - if (!defined $LogErrors) - { - if (!defined $ErrMessage) - { - $NewStatus = "badbuild"; - $ErrMessage = "Missing reconfig status line\n"; - } - # otherwise $ErrMessage probably already explains why the reconfig - # status line is missing - } - elsif ($LogErrors eq "") - { - # We must have gotten the full log and the build did succeed. - # So forget any prior error. - $NewStatus = "completed"; - $TAError = $ErrMessage = undef; - } - else - { - $NewStatus = "badbuild"; - $ErrMessage = $LogErrors . ($ErrMessage || ""); - } + # We must have gotten the full log and the build did succeed. + # So forget any prior error. + $NewStatus = "completed"; + $TAError = $ErrMessage = undef; + } + elsif ($Result =~ s/^nolog://) + { + FatalError("$Result\n", "retry"); } else { - FatalError("Unable to open the build log for reading: $!\n"); + # We should not have badpatch errors and if the result line is missing we + # probably already have an error message that explains why. + $NewStatus = "badbuild"; } } elsif (!defined $TAError) diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm new file mode 100644 index 000000000..3e40fbc35 --- /dev/null +++ b/testbot/lib/WineTestBot/LogUtils.pm @@ -0,0 +1,77 @@ +# -*- Mode: Perl; perl-indent-level: 2; indent-tabs-mode: nil -*- +# Copyright 2018 Francois Gouget +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# 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 strict; + +package WineTestBot::LogUtils; + +=head1 NAME + +WineTestBot::LogUtils - Provides functions to parse task logs + +=cut + + +use Exporter 'import'; +our @EXPORT = qw(ParseTaskLog); + + +# +# Task log parser +# + +=pod +=over 12 + +=item C<ParseTaskLog()> + +Returns ok if the task was successful and an error code otherwise. + +=back +=cut + +sub ParseTaskLog($$) +{ + my ($FileName, $ResultPrefix) = @_; + + if (open(my $LogFile, "<", $FileName)) + { + my $Result; + foreach my $Line (<$LogFile>) + { + chomp $Line; + if ($Line =~ /^$ResultPrefix: ok$/) + { + $Result ||= "ok"; + } + elsif ($Line =~ /^$ResultPrefix: Patch failed to apply$/) + { + $Result = "badpatch"; + last; # Should be the last and most specific message + } + elsif ($Line =~ /^$ResultPrefix: /) + { + $Result = "failed"; + } + } + close($LogFile); + return $Result || "missing"; + } + return "nolog:Unable to open the task log for reading: $!"; +} + +1; -- 2.17.1

June 21, 2018

[PATCH] testbot: VMs::FilterHypervisors() is not used anymore so remove it.

by Francois Gouget

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/lib/WineTestBot/VMs.pm | 7 ------- 1 file changed, 7 deletions(-) diff --git a/testbot/lib/WineTestBot/VMs.pm b/testbot/lib/WineTestBot/VMs.pm index a29eebbba..2f494cb81 100644 --- a/testbot/lib/WineTestBot/VMs.pm +++ b/testbot/lib/WineTestBot/VMs.pm @@ -730,11 +730,4 @@ sub FilterEnabledStatus($) $self->AddFilter("Status", ["dirty", "reverting", "sleeping", "idle", "running", "off", "offline"]); } -sub FilterHypervisors($$) -{ - my ($self, $Hypervisors) = @_; - - $self->AddFilter("VirtURI", $Hypervisors); -} - 1; -- 2.17.1

June 21, 2018

[PATCH] testbot/web: Detect and highlight some more Git errors.

by Francois Gouget

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/web/JobDetails.pl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl index 8cc354e84..410d18691 100644 --- a/testbot/web/JobDetails.pl +++ b/testbot/web/JobDetails.pl @@ -253,9 +253,12 @@ sub GetHtmlLine($$$) $Line =~ / done $258$/ or $Line =~ /: unhandled exception [0-9a-fA-F]{8} at / or $Line =~ /^Unhandled exception: / or + # Git errors + $Line =~ /^CONFLICT / or + $Line =~ /^error: patch failed:/ or + $Line =~ /^error: corrupt patch / or # Build errors $Line =~ /: error: / or - $Line =~ /^error: patch failed:/ or $Line =~ /^Makefile:[0-9]+: recipe for target .* failed$/ or $Line =~ /^(?:Build|Reconfig|Task): (?!ok)/ or # Typical perl errors -- 2.17.1

June 21, 2018

[PATCH 2/2] testbot/web: Prettify the log / error separator.

by Francois Gouget

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/web/JobDetails.pl | 7 ++++++- testbot/web/WineTestBot.css | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl index 1536e87cd..8cc354e84 100644 --- a/testbot/web/JobDetails.pl +++ b/testbot/web/JobDetails.pl @@ -495,7 +495,12 @@ sub GenerateBody($) chomp $Line; if ($ErrFirst) { - print "<hr>\n" if (!$LogFirst); + if (!$LogFirst) + { + print "<div class='HrTitle'>". + ($ErrName eq "err" ? "Other errors" : "Old errors") . + "<div class='HrLine'></div></div>\n"; + } print "<pre><code>"; $ErrFirst = 0; } diff --git a/testbot/web/WineTestBot.css b/testbot/web/WineTestBot.css index 28ac1b1c2..593c38f44 100644 --- a/testbot/web/WineTestBot.css +++ b/testbot/web/WineTestBot.css @@ -317,6 +317,20 @@ h2 padding-bottom: 0.1em; } +.HrTitle { + margin-top: 0.5em; + display: flex; + align-items: center; + color: #a50d0d; +} + +.HrLine { + border-top: 1px solid grey; + flex-grow: 1; + margin: 0 10px; + border-color: #a50d0d; +} + details { font-style: italic; -- 2.17.1

June 21, 2018

[PATCH 1/2] testbot/web: Better present the logs and task errors.

by Francois Gouget

Add a horizontal ruler so the log / test report is always clearly separated from the task error messages. Avoid duplicating the code that shows the task errors. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/web/JobDetails.pl | 152 +++++++++++++++++--------------------- 1 file changed, 67 insertions(+), 85 deletions(-) diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl index 3add52b4c..1536e87cd 100644 --- a/testbot/web/JobDetails.pl +++ b/testbot/web/JobDetails.pl @@ -408,127 +408,109 @@ sub GenerateBody($) my $LogName = $MoreInfo->{Full} || $MoreInfo->{Logs}->[0] || "log"; my $ErrName = $LogName eq "log.old" ? "err.old" : "err"; - if (open LOGFILE, "<", "$TaskDir/$LogName") + my ($EmptyDiag, $LogFirst); + if (open(my $LogFile, "<", "$TaskDir/$LogName")) { - my $HasLogEntries = !1; - my $First = 1; + my $HasLogEntries; my $CurrentDll = ""; my $PrintedDll = ""; - my $Line; - while (defined($Line = <LOGFILE>)) + $LogFirst = 1; + foreach my $Line (<$LogFile>) { $HasLogEntries = 1; - chomp($Line); + chomp $Line; if ($Line =~ m/^([^:]+):[^ ]+ start [^ ]+ -\s*$/) { $CurrentDll = $1; } my ($Highlight, $Plain) = $self->GetHtmlLine($MoreInfo->{Full}, $Line); - if ($MoreInfo->{Full} || defined $Highlight) + next if (!$MoreInfo->{Full} and !defined $Highlight); + + if ($PrintedDll ne $CurrentDll && !$MoreInfo->{Full}) + { + print "</code></pre>" if (!$LogFirst); + print "<div class='LogDllName'>$CurrentDll:</div><pre><code>"; + $PrintedDll = $CurrentDll; + $LogFirst = 0; + } + elsif ($LogFirst) + { + print "<pre><code>"; + $LogFirst = 0; + } + if (!$MoreInfo->{Full} && $Line =~ m/^[^:]+:([^:]*)(?::[0-9a-f]+)? done $258$/) + { + my $Unit = $1 ne "" ? "$1: " : ""; + print "${Unit}Timeout\n"; + } + else { - if ($PrintedDll ne $CurrentDll && !$MoreInfo->{Full}) - { - if ($First) - { - $First = !1; - } - else - { - print "</code></pre>"; - } - print "<div class='LogDllName'>$CurrentDll:</div><pre><code>"; - $PrintedDll = $CurrentDll; - } - elsif ($First) - { - print "<pre><code>"; - $First = !1; - } - if (!$MoreInfo->{Full} && $Line =~ m/^[^:]+:([^:]*)(?::[0-9a-f]+)? done $258$/) - { - my $Unit = $1 ne "" ? "$1: " : ""; - print "${Unit}Timeout\n"; - } - else - { - print(($Highlight || $Plain), "\n"); - } + print(($Highlight || $Plain), "\n"); } } - close LOGFILE; + close($LogFile); - if (open ERRFILE, "<", "$TaskDir/$ErrName") + if (!$LogFirst) { - $CurrentDll = "*err*"; - while (defined($Line = <ERRFILE>)) + print "</code></pre>\n"; + } + elsif ($HasLogEntries) + { + # Here we know we did not show the full log since it was not empty, + # and yet we did not show anything to the user. But don't claim there + # is no failure if the error log is not empty. + if (-z "$TaskDir/$ErrName") { - $HasLogEntries = 1; - chomp($Line); - if ($PrintedDll ne $CurrentDll) - { - if ($First) - { - $First = !1; - } - else - { - print "</code></pre>\n"; - } - print "<br><pre><code>"; - $PrintedDll = $CurrentDll; - } - print $self->escapeHTML($Line), "\n"; + print "No ". ($StepTask->Type eq "single" || + $StepTask->Type eq "suite" ? "test" : "build") . + " failures found"; + $LogFirst = 0; } - close ERRFILE; } - - if (! $First) + elsif ($StepTask->Status eq "canceled") { - print "</code></pre>\n"; + $EmptyDiag = "<p>No log, task was canceled</p>\n"; + } + elsif ($StepTask->Status eq "skipped") + { + $EmptyDiag = "<p>No log, task skipped</p>\n"; } else { - print $HasLogEntries ? "No " . - ($StepTask->Type eq "single" || - $StepTask->Type eq "suite" ? "test" : "build") . - " failures found" : "Empty log"; + print "Empty log"; + $LogFirst = 0; } } - elsif (open ERRFILE, "<", "$TaskDir/$ErrName") + else + { + print "No log". ($StepTask->Status =~ /^(?:queued|running)$/ ? " yet" : ""); + $LogFirst = 0; + } + + if (open(my $ErrFile, "<", "$TaskDir/$ErrName")) { - my $HasErrEntries = !1; - my $Line; - while (defined($Line = <ERRFILE>)) + my $ErrFirst = 1; + foreach my $Line (<$ErrFile>) { - chomp($Line); - if (! $HasErrEntries) + chomp $Line; + if ($ErrFirst) { + print "<hr>\n" if (!$LogFirst); print "<pre><code>"; - $HasErrEntries = 1; + $ErrFirst = 0; } print $self->escapeHTML($Line), "\n"; } - if ($HasErrEntries) + close($ErrFile); + + if (!$ErrFirst) { print "</code></pre>\n"; } - else + elsif (defined $EmptyDiag) { - print "Empty log"; + print $EmptyDiag; } - close ERRFILE; - } - elsif ($StepTask->Status eq "canceled") - { - print "<p>No log, task was canceled</p>\n"; - } - elsif ($StepTask->Status eq "skipped") - { - print "<p>No log, task skipped</p>\n"; - } - else - { - print "<p>No log available yet</p>\n"; } } print "</div>\n"; -- 2.17.1

June 21, 2018