Module: tools Branch: master Commit: 2e906aa00c0b334fd0f7b1206ff5dc04a2fbd294 URL: https://source.winehq.org/git/tools.git/?a=commit;h=2e906aa00c0b334fd0f7b120...
Author: Francois Gouget fgouget@codeweavers.com Date: Mon Jun 4 12:20:34 2018 +0200
testbot: Improve Job file storage.
So far files needed by a Step were stored in that Step's directory. This means the files produced by a Step are stored in the directory of the Step that needs them. Since multiple Steps may need the same file this can result in duplication (and also in retrieving the same file multiple times). This duplication is also needed for staging files if they are needed by more than one Step. This patch changes file storage so that files produced by a Step are stored in that Step's directory. Conversely files needed by a Step are found in the PreviousNo Step's directory, or in the Job's directory if PreviousNo is not set. This avoids duplication both when retrieving files from a VM and for staged files. To simplify the code Step::GetFullFileName() converts Step::FileName to an absolute path pointing to the Step's file. To ensure backward compatibility with existing Jobs it supports both the old and the new schemes.
Signed-off-by: Francois Gouget fgouget@codeweavers.com Signed-off-by: Alexandre Julliard julliard@winehq.org
---
testbot/bin/WineRunBuild.pl | 13 ++++++------- testbot/bin/WineRunTask.pl | 5 ++--- testbot/lib/WineTestBot/Steps.pm | 25 +++++++++++++++++++++++-- testbot/lib/WineTestBot/StepsTasks.pm | 18 ++++++++++++++++++ testbot/web/GetFile.pl | 2 +- testbot/web/JobDetails.pl | 2 +- 6 files changed, 51 insertions(+), 14 deletions(-)
diff --git a/testbot/bin/WineRunBuild.pl b/testbot/bin/WineRunBuild.pl index 27b8be0..7a474e7 100755 --- a/testbot/bin/WineRunBuild.pl +++ b/testbot/bin/WineRunBuild.pl @@ -354,11 +354,10 @@ if (!defined $BaseName) # Run the build #
-my $StepDir = $Step->GetDir(); -my $FileName = $Step->FileName; +my $FileName = $Step->GetFullFileName(); my $TA = $VM->GetAgent(); -Debug(Elapsed($Start), " Sending '$StepDir/$FileName'\n"); -if (!$TA->SendFile("$StepDir/$FileName", "staging/patch.diff", 0)) +Debug(Elapsed($Start), " Sending '$FileName'\n"); +if (!$TA->SendFile($FileName, "staging/patch.diff", 0)) { FatalTAError($TA, "Could not copy the patch to the VM"); } @@ -461,6 +460,7 @@ FatalTAError(undef, $TAError) if (defined $TAError); # Don't try copying the test executables if the build step failed if ($NewStatus eq "completed") { + my $StepDir = $Step->CreateDir(); foreach my $OtherStep (@{$Job->Steps->GetItems()}) { next if ($OtherStep->No == $StepNo); @@ -480,12 +480,11 @@ if ($NewStatus eq "completed") }
Debug(Elapsed($Start), " Retrieving '$OtherFileName'\n"); - my $OtherStepDir = $OtherStep->CreateDir(); - if (!$TA->GetFile($TestExecutable, "$OtherStepDir/$OtherFileName")) + if (!$TA->GetFile($TestExecutable, "$StepDir/$OtherFileName")) { FatalTAError($TA, "Could not retrieve '$OtherFileName'"); } - chmod 0664, "$OtherStepDir/$OtherFileName"; + chmod 0664, "$StepDir/$OtherFileName"; } } $TA->Disconnect(); diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl index 231f34e..d757417 100755 --- a/testbot/bin/WineRunTask.pl +++ b/testbot/bin/WineRunTask.pl @@ -388,10 +388,9 @@ if ($FileType ne "exe32" && $FileType ne "exe64") { FatalError("Unexpected file type $FileType found\n"); } -my $StepDir = $Step->GetDir(); my $FileName = $Step->FileName; -Debug(Elapsed($Start), " Sending '$StepDir/$FileName'\n"); -if (!$TA->SendFile("$StepDir/$FileName", $FileName, 0)) +Debug(Elapsed($Start), " Sending '". $Step->GetFullFileName() ."'\n"); +if (!$TA->SendFile($Step->GetFullFileName(), $FileName, 0)) { FatalTAError($TA, "Could not copy the test executable to the VM"); } diff --git a/testbot/lib/WineTestBot/Steps.pm b/testbot/lib/WineTestBot/Steps.pm index 4dc4569..9774194 100644 --- a/testbot/lib/WineTestBot/Steps.pm +++ b/testbot/lib/WineTestBot/Steps.pm @@ -57,6 +57,12 @@ the Step was running, and to skipped if it was queued.
=back
+If FileName is set it identifies a file that the Step needs for its operation. +That file will either be in the directory of the PreviousNo Step that produced +it, or in the directory of the job if it was provided by the user. +Conversely a Step's Task(s) may produce one or more files. These are all stored +in that Step's directory and may be used by one or more Steps. + Note that the PreviousNo relation will prevent the deletion of the target Step. It is the responsibility of the caller to delete the Steps in a suitable order, or to reset their PreviousNo fields beforehand. @@ -136,6 +142,21 @@ sub RmTree($) rmtree($Dir); }
+sub GetFullFileName($) +{ + my ($self) = @_; + + my ($JobId, $StepNo) = @{$self->GetMasterKey()}; + # FIXME: Remove legacy support once no such job remains (so after + # $JobPurgeDays). + my $LegacyPath = "$DataDir/jobs/$JobId/$StepNo/" . $self->FileName; + return $LegacyPath if (-f $LegacyPath); + + my $Path = "$DataDir/jobs/$JobId/"; + $Path .= $self->PreviousNo ."/" if ($self->PreviousNo); + return $Path . $self->FileName; +} + sub HandleStaging($$) { my ($self) = @_; @@ -150,13 +171,13 @@ sub HandleStaging($$) return "Can't split staging filename"; } my $BaseName = $1; + $self->FileName($BaseName); my $StagingFileName = "$DataDir/staging/$FileName"; - if (!move($StagingFileName, "$StepDir/$BaseName")) + if (!move($StagingFileName, $self->GetFullFileName())) { return "Could not move the staging file: $!"; }
- $self->FileName($BaseName); $self->InStaging(!1); my ($ErrProperty, $ErrMessage) = $self->Save();
diff --git a/testbot/lib/WineTestBot/StepsTasks.pm b/testbot/lib/WineTestBot/StepsTasks.pm index db0cc06..b9f70d7 100644 --- a/testbot/lib/WineTestBot/StepsTasks.pm +++ b/testbot/lib/WineTestBot/StepsTasks.pm @@ -39,6 +39,22 @@ sub GetStepDir($) return "$DataDir/jobs/$JobId/". $self->StepNo; }
+# See WineTestBot::Step::GetFullFileName() +sub GetFullFileName($) +{ + my ($self) = @_; + + my ($JobId, $_StepTaskId) = @{$self->GetMasterKey()}; + # FIXME: Remove legacy support once no such job remains (so after + # $JobPurgeDays). + my $LegacyPath = "$DataDir/jobs/$JobId/". $self->StepNo ."/". $self->FileName; + return $LegacyPath if (-f $LegacyPath); + + my $Path = "$DataDir/jobs/$JobId/"; + $Path .= $self->PreviousNo ."/" if ($self->PreviousNo); + return $Path . $self->FileName; +} + sub GetTaskDir($) { my ($self) = @_; @@ -121,6 +137,7 @@ sub _initialize($$) my $StepTask = $self->CreateItem(); $StepTask->Id(100 * $Step->No + $Task->No); $StepTask->StepNo($Step->No); + $StepTask->PreviousNo($Step->PreviousNo); $StepTask->TaskNo($Task->No); $StepTask->Type($Step->Type); $StepTask->Status($Task->Status); @@ -166,6 +183,7 @@ sub CreateItem($) my @PropertyDescriptors = ( CreateBasicPropertyDescriptor("Id", "Id", 1, 1, "N", 4), CreateBasicPropertyDescriptor("StepNo", "Step no", !1, 1, "N", 2), + CreateBasicPropertyDescriptor("PreviousNo", "Previous step", !1, !1, "N", 2), CreateBasicPropertyDescriptor("TaskNo", "Task no", !1, 1, "N", 2), CreateBasicPropertyDescriptor("Type", "Step type", !1, 1, "A", 32), CreateBasicPropertyDescriptor("Status", "Status", !1, 1, "A", 32), diff --git a/testbot/web/GetFile.pl b/testbot/web/GetFile.pl index a178cca..7e274a8 100644 --- a/testbot/web/GetFile.pl +++ b/testbot/web/GetFile.pl @@ -52,7 +52,7 @@ sub GetFile($$$) return !1; }
- my $FileName = $Step->GetDir() ."/". $Step->FileName; + my $FileName = $Step->GetFullFileName(); if (! sysopen(FILE, $FileName, O_RDONLY)) { return !1; diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl index 4d9a77a..6bf4a12 100644 --- a/testbot/web/JobDetails.pl +++ b/testbot/web/JobDetails.pl @@ -485,7 +485,7 @@ sub GenerateDataCell($$$$$) } elsif ($PropertyName eq "FileName") { - my $FileName = $StepTask->GetStepDir() ."/". $StepTask->FileName; + my $FileName = $StepTask->GetFullFileName(); if (-r $FileName) { my $URI = "/GetFile.pl?JobKey=" . uri_escape($self->{JobId}) .