Francois Gouget : testbot: Let VM scripts identify the last part of a patchset.

[Alexandre Julliard]

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

Author: Francois Gouget <fgouget(a)codeweavers.com>
Date:   Tue Sep 25 12:07:30 2018 +0200

testbot: Let VM scripts identify the last part of a patchset.

For patchsets (and subsets thereof) the Engine calls GetPatchImpacts()
at least once with only the last part. This allows it to know exactly
what the last part modifies and thus what needs to be retested.
However the VM scripts only have the full patchset so they don't know
what the last part is. Thus they are forced to rerun every test touched
by the full patchset.
So the TestBot now adds a marker before the last part, allowing
GetPatchImpacts() to identify it. Note that it does not use '---' as
that marker may or may not be present depending on how the patch was
formatted for emailing (and can be ambiguous too).

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard(a)winehq.org>

---

 testbot/lib/WineTestBot/PatchUtils.pm       | 76 ++++++++++++++++++++---------
 testbot/lib/WineTestBot/Patches.pm          |  5 +-
 testbot/lib/WineTestBot/PendingPatchSets.pm |  5 ++
 3 files changed, 60 insertions(+), 26 deletions(-)

diff --git a/testbot/lib/WineTestBot/PatchUtils.pm b/testbot/lib/WineTestBot/PatchUtils.pm
index b7307e5..f03cc40 100644
--- a/testbot/lib/WineTestBot/PatchUtils.pm
+++ b/testbot/lib/WineTestBot/PatchUtils.pm
@@ -31,7 +31,8 @@ the Wine builds.
 =cut
 
 use Exporter 'import';
-our @EXPORT = qw(GetPatchImpacts UpdateWineData GetBuildTimeout);
+our @EXPORT = qw(GetPatchImpacts LastPartSeparator UpdateWineData
+                 GetBuildTimeout);
 
 use List::Util qw(min max);
 
@@ -150,6 +151,11 @@ my $IgnoredPathsRe = join('|',
   'tools/winemaker/',
 );
 
+sub LastPartSeparator()
+{
+  return "===== TestBot: Last patchset part =====\n";
+}
+
 sub _CreateTestInfo($$$)
 {
   my ($Impacts, $Root, $Dir) = @_;
@@ -261,9 +267,9 @@ configure, whether it impacts the tests, etc.
 =back
 =cut
 
-sub GetPatchImpacts($;$)
+sub GetPatchImpacts($)
 {
-  my ($PatchFileName, $PastImpacts) = @_;
+  my ($PatchFileName) = @_;
 
   my $fh;
   return undef if (!open($fh, "<", $PatchFileName));
@@ -281,25 +287,7 @@ sub GetPatchImpacts($;$)
   };
   _LoadWineFiles();
 
-  if ($PastImpacts)
-  {
-    # Update the list of Wine files so we correctly recognize patchset parts
-    # that modify new Wine files.
-    my $WineFiles = $PastImpacts->{WineFiles} || $_WineFiles;
-    map { $Impacts->{WineFiles}->{$_} = 1 } keys %{$WineFiles};
-    map { $Impacts->{WineFiles}->{$_} = 1 } keys %{$PastImpacts->{NewFiles}};
-    map { delete $Impacts->{WineFiles}->{$_} } keys %{$PastImpacts->{DeletedFiles}};
-
-    foreach my $PastInfo (values %{$PastImpacts->{Tests}})
-    {
-      foreach my $File (keys %{$PastInfo->{Files}})
-      {
-        _HandleFile($Impacts, "$PastInfo->{Path}/$File",
-                    $PastInfo->{Files}->{$File} eq "rm" ? "rm" : 0);
-      }
-    }
-  }
-
+  my $PastImpacts;
   my ($Path, $Change);
   while (my $Line = <$fh>)
   {
@@ -332,6 +320,50 @@ sub GetPatchImpacts($;$)
       $Path = undef;
       $Change = "";
     }
+    elsif ($Line eq LastPartSeparator())
+    {
+      # All the diffs so far belongs to previous parts of this patchset.
+      # But:
+      # - Only the last part must be taken into account to determine if a
+      #   rebuild and testing is needed.
+      # - Yet if a rebuild is needed the previous parts' patches will impact
+      #   the scope of the rebuild so that information must be preserved.
+      # So save current impacts in $PastImpacts and reset the current state.
+      $PastImpacts = {};
+
+      # Build a copy of the Wine files list reflecting the current situation.
+      $Impacts->{WineFiles} = { %$_WineFiles } if (!$Impacts->{WineFiles});
+      map { $Impacts->{WineFiles}->{$_} = 1 } keys %{$Impacts->{NewFiles}};
+      map { delete $Impacts->{WineFiles}->{$_} } keys %{$Impacts->{DeletedFiles}};
+      $Impacts->{NewFiles} = {};
+      $Impacts->{DeletedFiles} = {};
+
+      # The modules impacted by previous parts will still need to be built,
+      # but only if the last part justifies a build. So make a backup.
+      $PastImpacts->{BuildModules} = $Impacts->{BuildModules};
+      $Impacts->{BuildModules} = {};
+
+      # Also backup the build-related fields.
+      foreach my $Field ("Autoconf", "MakeMakefiles",
+                         "PatchedRoot", "PatchedModules", "PatchedTests")
+      {
+        $PastImpacts->{$Field} = $Impacts->{$Field};
+        $Impacts->{$Field} = undef;
+      }
+
+      # Reset the status of all test unit files to not modified.
+      foreach my $TestInfo (values %{$Impacts->{Tests}})
+      {
+        foreach my $File (keys %{$TestInfo->{Files}})
+        {
+          if ($TestInfo->{Files}->{$File} ne "rm")
+          {
+            $TestInfo->{Files}->{$File} = 0;
+          }
+        }
+      }
+      $Impacts->{ModuleUnitCount} = $Impacts->{TestUnitCount} = 0;
+    }
     else
     {
       $Path = undef;
diff --git a/testbot/lib/WineTestBot/Patches.pm b/testbot/lib/WineTestBot/Patches.pm
index 0387543..469fedc 100644
--- a/testbot/lib/WineTestBot/Patches.pm
+++ b/testbot/lib/WineTestBot/Patches.pm
@@ -132,10 +132,7 @@ sub Submit($$$)
 {
   my ($self, $PatchFileName, $IsSet) = @_;
 
-  my $PastImpacts;
-  $PastImpacts = GetPatchImpacts($PatchFileName) if ($IsSet);
-  my $Impacts = GetPatchImpacts("$DataDir/patches/" . $self->Id, $PastImpacts);
-
+  my $Impacts = GetPatchImpacts($PatchFileName);
   if (!$Impacts->{PatchedRoot} and !$Impacts->{PatchedModules} and
       !$Impacts->{PatchedTests})
   {
diff --git a/testbot/lib/WineTestBot/PendingPatchSets.pm b/testbot/lib/WineTestBot/PendingPatchSets.pm
index 3ac8c53..286b393 100644
--- a/testbot/lib/WineTestBot/PendingPatchSets.pm
+++ b/testbot/lib/WineTestBot/PendingPatchSets.pm
@@ -42,6 +42,7 @@ use WineTestBot::WineTestBotObjects;
 our @ISA = qw(WineTestBot::WineTestBotItem);
 
 use WineTestBot::Config;
+use WineTestBot::PatchUtils;
 use WineTestBot::Utils;
 
 
@@ -116,6 +117,10 @@ sub SubmitSubset($$$)
       last;
     }
 
+    if ($PartNo == $MaxPart and $PartNo > 1)
+    {
+      print $CombinedFile LastPartSeparator();
+    }
     if (open(my $PartFile, "<" , "$DataDir/patches/" . $Part->Patch->Id))
     {
       print $CombinedFile $_ for (<$PartFile>);