The old reference report filename format was '<vmname>_<reportname>' which means there could only be one per report. Each job should only have one report of a given type per VM so add it to the reference filename. Also the use of an underscore as the separator made it hard to split the components apart as both <vmname> and <reportname> can contain underscores such as 'w1064v1809_fr' and 'win32_fr_FR.report'. Dashes are a better choice since they are not allowed in either the VM name or the report name. So the new refeence filename format is '<vmname>-job<jobid>-<reportname>'.
Note: * After applying this patch run UpdateTaskLogs to move and rename the existing reference reports. * When downgrading to run code before this patch it's best to run UpdateTaskLogs --delete before downgrading and then UpdateTaskLogs after the downgrade.
Signed-off-by: Francois Gouget fgouget@codeweavers.com ---
To simplify one can just apply both patches in the series and then just run UpdateTaskLogs once to upgrade the existing reports and logs.
It may be a good idea to run it as follows to help with debugging in case something goes wrong:
cd testbot/var && ../bin/UpdateTaskLogs --debug 2>&1 | tee UpdateTaskLogs.log
testbot/bin/Janitor.pl | 3 +- testbot/bin/UpdateTaskLogs | 113 +++++++++++++++++++++++--- testbot/bin/WineSendLog.pl | 2 +- testbot/lib/WineTestBot/LogUtils.pm | 5 +- testbot/lib/WineTestBot/StepsTasks.pm | 2 +- testbot/lib/WineTestBot/Tasks.pm | 2 +- testbot/web/JobDetails.pl | 4 +- 7 files changed, 110 insertions(+), 21 deletions(-)
diff --git a/testbot/bin/Janitor.pl b/testbot/bin/Janitor.pl index bf3989a712..dd13966018 100755 --- a/testbot/bin/Janitor.pl +++ b/testbot/bin/Janitor.pl @@ -314,7 +314,8 @@ if (opendir(my $dh, "$DataDir/latest")) my $Age = int((-M $FileName) + 0.5); my $TTL = $JobPurgeDays ? $JobPurgeDays - $Age : undef;
- if ($Entry =~ /^([a-zA-Z0-9_]+)_(?:exe|win|wow)(?:32|64)[a-zA-Z0-9_]*.report(?:.err)?$/) + # Keep the regexp in sync with WineTestBot::Task::GetRefReportName() + if ($Entry =~ /^([a-zA-Z0-9_]+)-job[0-9]+-[a-zA-Z0-9_]+.report(?:.err)?$/) { # Keep the reference WineTest reports for all VMs even if they are # retired or scheduled for deletion. diff --git a/testbot/bin/UpdateTaskLogs b/testbot/bin/UpdateTaskLogs index 887061dab2..11dd6d31b9 100755 --- a/testbot/bin/UpdateTaskLogs +++ b/testbot/bin/UpdateTaskLogs @@ -261,18 +261,66 @@ sub DoUpdateLatestReport($$$) return $Rc; }
+sub MoveRefReport($$;$) +{ + my ($RefDir, $RefName, $NewDir) = @_; + + my $RefPath = "$RefDir/$RefName"; + return Delete("$RefPath.err", "orphaned") if (!-f $RefPath); + + $NewDir ||= $RefDir; + my $NewName = $RefName; + if ($RefName =~ /^([a-zA-Z0-9_]+)_((?:exe|win|wow)(?:32|64)[a-zA-Z0-9_]*.report(?:.err)?)$/) + { + # We don't know which job generated this reference log. + # So use a fixed, arbitrary JobId. + $NewName = "$1-job000000-$2"; + } + + my $NewPath = "$NewDir/$NewName"; + return 0 if ($RefPath eq $NewPath); + + my $Rc = 0; + my $TaskKey = Path2TaskKey($NewDir); + if (-f $NewPath and -M $NewPath <= -M $RefPath) + { + # A WineTest job has probably completed after the upgrade already + $Rc += Delete($RefPath); + $Rc += Delete("$RefPath.err"); + return $Rc; + } + + if (-f $NewPath) + { + Error "Could not move '$RefName' because '$NewName' already exists and is older!\n"; + return 1; + } + + my $RelRefDir = ($RefDir eq $NewDir) ? "" : "../"; + Debug("$TaskKey: $RelRefDir$RefName* -> $NewName\n"); + foreach my $Suffix ("", ".err") + { + next if (!-f "$RefPath$Suffix"); + next if (rename("$RefPath$Suffix", "$NewPath$Suffix")); + Error "Could not move '$RefName$Suffix' to '$NewName$Suffix' for $TaskKey: $!\n"; + $Rc = 1; + } + + return $Rc; +} + sub ProcessTaskLogs($$$) { my ($Step, $Task, $CollectOnly) = @_;
my $Rc = 0; - my $StepDir = $Step->GetDir(); + my $ReportNames; my $TaskDir = $Task->GetDir();
- if (($OptDelete or $OptRebuild) and !$CollectOnly) + if (!$CollectOnly) { - # Save / delete the task's reference reports... all of them, - # even if they were not supposed to exist (e.g. failed builds). + # Upgrade the naming scheme of the task's old reference reports, + # delete those that are not meant to exist. my ($ErrMessage, $ReportNames, $_TaskMissions) = $Task->GetReportNames(); if ($ErrMessage) { @@ -280,9 +328,30 @@ sub ProcessTaskLogs($$$) $Rc = 1; } foreach my $ReportName (@$ReportNames) + { + my $StepReportName = $Task->VM->Name ."_$ReportName"; + my $StepReportPath = $Step->GetFullFileName($StepReportName); + if (-f "$TaskDir/$ReportName") + { + $Rc += MoveRefReport(dirname($StepReportPath), $StepReportName, $TaskDir); + # The old WineTest reference reports for Windows tasks may end up being + # duplicated: one copy in the build step and another in the step for + # the task. MoveRefReport() moved the latter, now ensure Delete() can + # remove the former. + $StepReportPath = $Step->GetFullFileName($StepReportName); + } + $Rc += Delete($StepReportPath, "unneeded"); + } + } + + if (($OptDelete or $OptRebuild) and !$CollectOnly) + { + # Save / delete the task's reference reports... all of them, + # even if they were not supposed to exist (e.g. failed builds). + foreach my $ReportName (@$ReportNames) { my $RefReportName = $Task->GetRefReportName($ReportName); - my $RefReportPath = "$StepDir/$RefReportName"; + my $RefReportPath = "$TaskDir/$RefReportName";
if (-f $RefReportPath or -f "$RefReportPath.err") { @@ -290,7 +359,7 @@ sub ProcessTaskLogs($$$) { # (Re)Build the .err file before adding the reference report to # latest/. - my $ErrMessage = BuildErrFile($StepDir, $RefReportName, 1, 0); + my $ErrMessage = BuildErrFile($TaskDir, $RefReportName, 1, 0); if (defined $ErrMessage) { Error "$ErrMessage\n"; @@ -315,7 +384,7 @@ sub ProcessTaskLogs($$$) { next if ($ReportName !~ /.report$/); my $RefReportName = $Task->GetRefReportName($ReportName); - next if (-f "$StepDir/$RefReportName"); + next if (-f "$TaskDir/$RefReportName");
my $LatestReportPath = $LatestReports{$RefReportName}; if (!defined $LatestReportPath) @@ -328,9 +397,9 @@ sub ProcessTaskLogs($$$) Debug(TaskKeyStr($Task) .": Snapshotting $RefReportName from ". Path2TaskKey($LatestReportPath) ."\n"); foreach my $Suffix ("", ".err") { - unlink "$StepDir/$RefReportName$Suffix"; + unlink "$TaskDir/$RefReportName$Suffix"; if (-f "$LatestReportPath$Suffix" and - !link("$LatestReportPath$Suffix", "$StepDir/$RefReportName$Suffix")) + !link("$LatestReportPath$Suffix", "$TaskDir/$RefReportName$Suffix")) { Error "Could not link '$RefReportName$Suffix': $!\n"; $Rc = 1; @@ -390,18 +459,38 @@ sub ProcessLatestReports() my $Rc = 0; my $LatestGlob = "$DataDir/latest/*.report";
- # Perform cleanups and updates + # Delete or rename the old reference reports foreach my $LatestReportPath (glob("$LatestGlob $LatestGlob.err")) { my $RefReportName = basename($LatestReportPath); - # Keep the regexp in sync with WineTestBot::Task::GetRefReportName() next if ($RefReportName !~ /^([a-zA-Z0-9_]+_[a-zA-Z0-9_]+.report)(?:.err)?$/); $RefReportName = $1; # untaint $LatestReportPath = "$DataDir/latest/$RefReportName";
if ($OptDelete or $OptRebuild) { - # Delete the reports that should be deleted / rebuilt + $Rc += Delete($LatestReportPath); + $Rc += Delete("$LatestReportPath.err"); + } + else + { + # MoveRefReport() also renames .err files + $Rc += MoveRefReport("$DataDir/latest", $RefReportName); + } + } + + # Then perform cleanups and rebuild missing files + foreach my $LatestReportPath (glob("$LatestGlob $LatestGlob.err")) + { + my $RefReportName = basename($LatestReportPath); + # Keep the regexp in sync with WineTestBot::Task::GetRefReportName() + next if ($RefReportName !~ /^([a-zA-Z0-9_]+-job[0-9]+-[a-zA-Z0-9_]+.report)(?:.err)?$/); + $RefReportName = $1; # untaint + $LatestReportPath = "$DataDir/latest/$RefReportName"; + + if ($OptDelete or $OptRebuild) + { + # These can be rebuilt from the task reports $Rc += Delete($LatestReportPath); $Rc += Delete("$LatestReportPath.err"); } diff --git a/testbot/bin/WineSendLog.pl b/testbot/bin/WineSendLog.pl index 44d7255e71..e0cd585b11 100755 --- a/testbot/bin/WineSendLog.pl +++ b/testbot/bin/WineSendLog.pl @@ -293,7 +293,7 @@ EOF next if (!$LogInfo->{ErrCount});
my $AllNew; - my $RefReportPath = $StepTask->GetFullFileName($StepTask->GetRefReportName($LogName)); + my $RefReportPath = "$TaskDir/". $StepTask->GetRefReportName($LogName); TagNewErrors($RefReportPath, $LogInfo); if ($LogInfo->{NoRef}) { diff --git a/testbot/lib/WineTestBot/LogUtils.pm b/testbot/lib/WineTestBot/LogUtils.pm index 5b567bc23b..36f367497b 100644 --- a/testbot/lib/WineTestBot/LogUtils.pm +++ b/testbot/lib/WineTestBot/LogUtils.pm @@ -1070,11 +1070,10 @@ sub SnapshotLatestReport($$) { next if (!-f "$DataDir/latest/$RefReportName$Suffix");
- # FIXME: The reference reports are stored at the step level! if (!link("$DataDir/latest/$RefReportName$Suffix", - "$TaskDir/../$RefReportName$Suffix")) + "$TaskDir/$RefReportName$Suffix")) { - push @ErrMessages, "Could not create the '../$RefReportName$Suffix' link: $!"; + push @ErrMessages, "Could not create the '$RefReportName$Suffix' link: $!"; } }
diff --git a/testbot/lib/WineTestBot/StepsTasks.pm b/testbot/lib/WineTestBot/StepsTasks.pm index bad0e24375..f65e6a6327 100644 --- a/testbot/lib/WineTestBot/StepsTasks.pm +++ b/testbot/lib/WineTestBot/StepsTasks.pm @@ -71,7 +71,7 @@ sub GetTaskDir($) sub GetRefReportName($$) { my ($self, $ReportName) = @_; - return $self->VM->Name ."_$ReportName"; + return $self->VM->Name ."-job000000-$ReportName"; }
sub GetTitle($) diff --git a/testbot/lib/WineTestBot/Tasks.pm b/testbot/lib/WineTestBot/Tasks.pm index 1a46f0d57c..7b9ac9b816 100644 --- a/testbot/lib/WineTestBot/Tasks.pm +++ b/testbot/lib/WineTestBot/Tasks.pm @@ -141,7 +141,7 @@ sub GetReportNames($) sub GetRefReportName($$) { my ($self, $ReportName) = @_; - return $self->VM->Name ."_$ReportName"; + return $self->VM->Name ."-job000000-$ReportName"; }
sub _SetupTask($$) diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl index f1ffd498ed..3dce91595d 100644 --- a/testbot/web/JobDetails.pl +++ b/testbot/web/JobDetails.pl @@ -494,7 +494,7 @@ EOF if ($LogInfo->{ErrCount}) { # Identify new errors in test reports - my $RefReportPath = $StepTask->GetFullFileName($StepTask->GetRefReportName($MoreInfo->{Full})); + my $RefReportPath = "$TaskDir/". $StepTask->GetRefReportName($MoreInfo->{Full}); TagNewErrors($RefReportPath, $LogInfo); } } @@ -563,7 +563,7 @@ EOF if ($LogName =~ /.report$/) { # For test reports try to identify the new errors - my $RefReportPath = $StepTask->GetFullFileName($StepTask->GetRefReportName($LogName)); + my $RefReportPath = "$TaskDir/". $StepTask->GetRefReportName($LogName); TagNewErrors($RefReportPath, $LogInfo); }