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@codeweavers.com --- 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 b7307e527..f03cc40bb 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 038754365..469fedcdc 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 3ac8c5350..286b39314 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>);
-
Francois Gouget