Francois Gouget : testbot: Improve Job file storage.

Alexandre Julliard julliard at winehq.org
Mon Jun 4 15:43:21 CDT 2018


Module: tools
Branch: master
Commit: 2e906aa00c0b334fd0f7b1206ff5dc04a2fbd294
URL:    https://source.winehq.org/git/tools.git/?a=commit;h=2e906aa00c0b334fd0f7b1206ff5dc04a2fbd294

Author: Francois Gouget <fgouget at 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 at codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard at 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}) .




More information about the wine-cvs mailing list