Patches that impact a single module can impact the Wine files list too.
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/lib/WineTestBot/PatchUtils.pm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/testbot/lib/WineTestBot/PatchUtils.pm b/testbot/lib/WineTestBot/PatchUtils.pm index 9e0e68980..602c80b7d 100644 --- a/testbot/lib/WineTestBot/PatchUtils.pm +++ b/testbot/lib/WineTestBot/PatchUtils.pm @@ -279,7 +279,8 @@ sub GetPatchImpacts($;$)
if ($PastImpacts) { - if ($PastImpacts->{WineBuild} or $PastImpacts->{TestBuild}) + if ($PastImpacts->{WineBuild} or $PastImpacts->{ModuleBuild} or + $PastImpacts->{TestBuild}) { # Update the list of Wine files so we correctly recognize patchset parts # that modify new Wine files.
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/lib/WineTestBot/PatchUtils.pm | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/testbot/lib/WineTestBot/PatchUtils.pm b/testbot/lib/WineTestBot/PatchUtils.pm index 602c80b7d..7c57b2c85 100644 --- a/testbot/lib/WineTestBot/PatchUtils.pm +++ b/testbot/lib/WineTestBot/PatchUtils.pm @@ -183,15 +183,10 @@ sub _HandleFile($$$) }; }
- # Assume makefile modifications may break the build but not the tests - if ($File eq "Makefile.in") + if ($File eq "Makefile.in" and $Change ne "modify") { - if ($Change eq "new" or $Change eq "rm") - { - # This adds / removes a directory - $Impacts->{MakeMakefiles} = 1; - } - return; + # This adds / removes a directory + $Impacts->{MakeMakefiles} = 1; }
if (!$Tests->{$Module}->{Files}) @@ -233,6 +228,11 @@ sub _HandleFile($$$) if ($FilePath !~ /^(?:$IgnoredPathsRe)/) { $Impacts->{WineBuild} = 1; + if ($FilePath =~ m~/Makefile.in$~ and $Change ne "modify") + { + # This adds / removes a directory + $Impacts->{MakeMakefiles} = 1; + } } # Else patches to this file don't impact the Wine build. } @@ -358,6 +358,8 @@ sub GetPatchImpacts($;$) { # Skip unmodified files next if (!$TestInfo->{Files}->{$File}); + # Assume makefile modifications may break the build but not the tests + next if ($File eq "Makefile.in");
my $Base = $File; if ($Base !~ s/(?:.c|.spec)$//)
_CreateTestInfo() creates the initial $Impacts->{Tests} structure for GetPachImpacts().
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/lib/WineTestBot/PatchUtils.pm | 64 ++++++++++++++------------- 1 file changed, 33 insertions(+), 31 deletions(-)
diff --git a/testbot/lib/WineTestBot/PatchUtils.pm b/testbot/lib/WineTestBot/PatchUtils.pm index 7c57b2c85..04aea183b 100644 --- a/testbot/lib/WineTestBot/PatchUtils.pm +++ b/testbot/lib/WineTestBot/PatchUtils.pm @@ -150,6 +150,31 @@ my $IgnoredPathsRe = join('|', 'tools/winemaker/', );
+sub _CreateTestInfo($$$) +{ + my ($Impacts, $Root, $Dir) = @_; + + my $Module = ($Root eq "programs") ? "$Dir.exe" : $Dir; + $Impacts->{BuildModules}->{$Module} = 1; + $Impacts->{IsWinePatch} = 1; + + my $Tests = $Impacts->{Tests}; + if (!$Tests->{$Module}) + { + $Tests->{$Module} = { + "Module" => $Module, + "Path" => "$Root/$Dir/tests", + "ExeBase" => "${Module}_test", + }; + foreach my $File (keys %{$_TestList->{$Module}}) + { + $Tests->{$Module}->{Files}->{$File} = 0; # not modified + } + } + + return $Module; +} + sub _HandleFile($$$) { my ($Impacts, $FilePath, $Change) = @_; @@ -168,43 +193,23 @@ sub _HandleFile($$$) if ($FilePath =~ m~^(dlls|programs)/([^/]+)/tests/([^/\s]+)$~) { my ($Root, $Dir, $File) = ($1, $2, $3); - my $Module = ($Root eq "programs") ? "$Dir.exe" : $Dir; - $Impacts->{BuildModules}->{$Module} = 1; - $Impacts->{TestBuild} = 1; - $Impacts->{IsWinePatch} = 1;
- my $Tests = $Impacts->{Tests}; - if (!$Tests->{$Module}) - { - $Tests->{$Module} = { - "Module" => $Module, - "Path" => "$Root/$Dir/tests", - "ExeBase" => "${Module}_test", - }; - } + my $Module = _CreateTestInfo($Impacts, $Root, $Dir); + $Impacts->{TestBuild} = 1; + $Impacts->{Tests}->{$Module}->{Files}->{$File} = $Change;
if ($File eq "Makefile.in" and $Change ne "modify") { # This adds / removes a directory $Impacts->{MakeMakefiles} = 1; } - - if (!$Tests->{$Module}->{Files}) - { - foreach my $File (keys %{$_TestList->{$Module}}) - { - $Tests->{$Module}->{Files}->{$File} = 0; # not modified - } - } - $Tests->{$Module}->{Files}->{$File} = $Change; } elsif ($FilePath =~ m~^(dlls|programs)/([^/]+)/([^/\s]+)$~) { my ($Root, $Dir, $File) = ($1, $2, $3); - my $Module = ($Root eq "programs") ? "$Dir.exe" : $Dir; - $Impacts->{BuildModules}->{$Module} = 1; + + my $Module = _CreateTestInfo($Impacts, $Root, $Dir); $Impacts->{ModuleBuild} = 1; - $Impacts->{IsWinePatch} = 1;
if ($File eq "Makefile.in" and $Change ne "modify") { @@ -300,13 +305,10 @@ sub GetPatchImpacts($;$)
foreach my $PastInfo (values %{$PastImpacts->{Tests}}) { - if ($PastInfo->{Files}) + foreach my $File (keys %{$PastInfo->{Files}}) { - foreach my $File (keys %{$PastInfo->{Files}}) - { - _HandleFile($Impacts, "$PastInfo->{Path}/$File", - $PastInfo->{Files}->{$File} eq "rm" ? "rm" : 0); - } + _HandleFile($Impacts, "$PastInfo->{Path}/$File", + $PastInfo->{Files}->{$File} eq "rm" ? "rm" : 0); } } }
The new names are less ambiguous and less Wine-specific.
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/bin/build/Build.pl | 2 +- testbot/lib/WineTestBot/PatchUtils.pm | 25 ++++++++++++------------- testbot/lib/WineTestBot/Patches.pm | 6 +++--- testbot/web/Submit.pl | 4 ++-- 4 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/testbot/bin/build/Build.pl b/testbot/bin/build/Build.pl index 7a268db5c..1287f6db5 100755 --- a/testbot/bin/build/Build.pl +++ b/testbot/bin/build/Build.pl @@ -222,7 +222,7 @@ if ($DataDir =~ /'/) my $Impacts = ApplyPatch("wine", $PatchFile);
if (!$Impacts or - ($Impacts->{WineBuild} and !BuildNative()) or + ($Impacts->{PatchedRoot} and !BuildNative()) or !BuildTestExecutables($Targets, $Impacts, "exe32") or !BuildTestExecutables($Targets, $Impacts, "exe64")) { diff --git a/testbot/lib/WineTestBot/PatchUtils.pm b/testbot/lib/WineTestBot/PatchUtils.pm index 04aea183b..2bac7be46 100644 --- a/testbot/lib/WineTestBot/PatchUtils.pm +++ b/testbot/lib/WineTestBot/PatchUtils.pm @@ -195,7 +195,7 @@ sub _HandleFile($$$) my ($Root, $Dir, $File) = ($1, $2, $3);
my $Module = _CreateTestInfo($Impacts, $Root, $Dir); - $Impacts->{TestBuild} = 1; + $Impacts->{PatchedTests} = 1; $Impacts->{Tests}->{$Module}->{Files}->{$File} = $Change;
if ($File eq "Makefile.in" and $Change ne "modify") @@ -209,7 +209,7 @@ sub _HandleFile($$$) my ($Root, $Dir, $File) = ($1, $2, $3);
my $Module = _CreateTestInfo($Impacts, $Root, $Dir); - $Impacts->{ModuleBuild} = 1; + $Impacts->{PatchedModules} = 1;
if ($File eq "Makefile.in" and $Change ne "modify") { @@ -227,12 +227,11 @@ sub _HandleFile($$$) $Impacts->{IsWinePatch} = 1; } # Else this file exists in Wine but has a very common name so it may just - # as well belong to another repository. Still update WineBuild in case - # this patch really is for Wine. + # as well belong to another repository.
if ($FilePath !~ /^(?:$IgnoredPathsRe)/) { - $Impacts->{WineBuild} = 1; + $Impacts->{PatchedRoot} = 1; if ($FilePath =~ m~/Makefile.in$~ and $Change ne "modify") { # This adds / removes a directory @@ -245,7 +244,7 @@ sub _HandleFile($$$) { # This may or may not be a Wine patch but the new Makefile.in will be # added to the build by make_makefiles. - $Impacts->{WineBuild} = $Impacts->{MakeMakefiles} = 1; + $Impacts->{PatchedRoot} = $Impacts->{MakeMakefiles} = 1; } } } @@ -274,7 +273,7 @@ sub GetPatchImpacts($;$) # patch. ModuleUnitCount => 0, # Number of patched test units. - UnitCount => 0, + TestUnitCount => 0, # The modules that need a rebuild, even if only for the tests. BuildModules => {}, # Information about 'tests' directories. @@ -284,8 +283,8 @@ sub GetPatchImpacts($;$)
if ($PastImpacts) { - if ($PastImpacts->{WineBuild} or $PastImpacts->{ModuleBuild} or - $PastImpacts->{TestBuild}) + if ($PastImpacts->{PatchedRoot} or $PastImpacts->{PatchedModules} or + $PastImpacts->{PatchedTests}) { # Update the list of Wine files so we correctly recognize patchset parts # that modify new Wine files. @@ -318,11 +317,11 @@ sub GetPatchImpacts($;$) { if ($Line =~ m=^--- \w+/(?:aclocal.m4|configure.ac)$=) { - $Impacts->{WineBuild} = $Impacts->{Autoconf} = 1; + $Impacts->{PatchedRoot} = $Impacts->{Autoconf} = 1; } elsif ($Line =~ m=^--- \w+/tools/make_makefiles$=) { - $Impacts->{WineBuild} = $Impacts->{MakeMakefiles} = 1; + $Impacts->{PatchedRoot} = $Impacts->{MakeMakefiles} = 1; $Impacts->{IsWinePatch} = 1; } elsif ($Line =~ m=^--- /dev/null$=) @@ -401,7 +400,7 @@ sub GetPatchImpacts($;$) }
$TestInfo->{UnitCount} = scalar(keys %{$TestInfo->{Units}}); - $Impacts->{UnitCount} += $TestInfo->{UnitCount}; + $Impacts->{TestUnitCount} += $TestInfo->{UnitCount}; }
return $Impacts; @@ -420,7 +419,7 @@ sub GetBuildTimeout($$) map {$_ =~ /^exe/ ? $ExeCount++ : $WineCount++ } keys %$Builds;
# Set $ModuleCount to 0 if a full rebuild is needed - my $ModuleCount = (!$Impacts or $Impacts->{WineBuild}) ? 0 : + my $ModuleCount = (!$Impacts or $Impacts->{PatchedRoot}) ? 0 : scalar(keys %{$Impacts->{BuildModules}});
my ($ExeTimeout, $WineTimeout) = (0, 0); diff --git a/testbot/lib/WineTestBot/Patches.pm b/testbot/lib/WineTestBot/Patches.pm index 5024ed489..038754365 100644 --- a/testbot/lib/WineTestBot/Patches.pm +++ b/testbot/lib/WineTestBot/Patches.pm @@ -136,8 +136,8 @@ sub Submit($$$) $PastImpacts = GetPatchImpacts($PatchFileName) if ($IsSet); my $Impacts = GetPatchImpacts("$DataDir/patches/" . $self->Id, $PastImpacts);
- if (!$Impacts->{WineBuild} and !$Impacts->{ModuleBuild} and - !$Impacts->{TestBuild}) + if (!$Impacts->{PatchedRoot} and !$Impacts->{PatchedModules} and + !$Impacts->{PatchedTests}) { if ($Impacts->{IsWinePatch}) { @@ -176,7 +176,7 @@ sub Submit($$$) my $BuildVMs = CreateVMs(); $BuildVMs->AddFilter("Type", ["build"]); $BuildVMs->AddFilter("Role", ["base"]); - if ($Impacts->{UnitCount} and !$BuildVMs->IsEmpty()) + if ($Impacts->{TestUnitCount} and !$BuildVMs->IsEmpty()) { # Create the Build Step my $BuildStep = $NewJob->Steps->Add(); diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl index fe101042c..ae4aca3dd 100644 --- a/testbot/web/Submit.pl +++ b/testbot/web/Submit.pl @@ -598,11 +598,11 @@ sub DetermineFileType($$) if ($FileType eq "unknown") { my $Impacts = GetPatchImpacts($FileName); - if ($Impacts->{UnitCount} == 0) + if ($Impacts->{TestUnitCount} == 0) { $ErrMessage = "Patch doesn't affect tests"; } - elsif ($Impacts->{UnitCount} > 1) + elsif ($Impacts->{TestUnitCount} > 1) { $ErrMessage = "Patch contains changes to multiple tests"; }
-
Francois Gouget