Also check that $DataDir does not contain single quotes, thus ensuring our simple quoting scheme works.
Signed-off-by: Francois Gouget fgouget@codeweavers.com ---
A more absolutist approach would be to use ShQuote() everywhere which would remove the single quote restriction. But that would compromise readability. So this solution seems like a good compromise.
Also this is mostly cosmetic in the first place since the administrator would presumably not put spaces, stars, dollar signs or single quotes in the $DataDir path. Still I much prefer having proper quoting in my shell scripts.
testbot/bin/build/Build.pl | 16 +++++++++++----- testbot/bin/build/Reconfig.pl | 17 +++++++++++------ 2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/testbot/bin/build/Build.pl b/testbot/bin/build/Build.pl index c23bb8780..17ae74ff5 100755 --- a/testbot/bin/build/Build.pl +++ b/testbot/bin/build/Build.pl @@ -82,7 +82,7 @@ sub ApplyPatch($) my ($PatchFile) = @_;
InfoMsg "Applying patch\n"; - system("cd $DataDir/wine && set -x && ". + system("cd '$DataDir/wine' && set -x && ". "git apply --verbose ". ShQuote($PatchFile) ." && ". "git add -A"); if ($? != 0) @@ -95,7 +95,7 @@ sub ApplyPatch($) if ($Impacts->{Makefiles}) { InfoMsg "\nRunning make_makefiles\n"; - system("cd $DataDir/wine && set -x && ./tools/make_makefiles"); + system("cd '$DataDir/wine' && set -x && ./tools/make_makefiles"); if ($? != 0) { LogMsg "make_makefiles failed\n"; @@ -106,7 +106,7 @@ sub ApplyPatch($) if ($Impacts->{Autoconf} && !$Impacts->{HasConfigure}) { InfoMsg "\nRunning autoconf\n"; - system("cd $DataDir/wine && set -x && autoconf"); + system("cd '$DataDir/wine' && set -x && autoconf"); if ($? != 0) { LogMsg "Autoconf failed\n"; @@ -122,7 +122,7 @@ sub BuildNative() mkdir "$DataDir/build-native" if (! -d "$DataDir/build-native");
InfoMsg "\nRebuilding native tools\n"; - system("cd $DataDir/build-native && set -x && ". + system("cd '$DataDir/build-native' && set -x && ". "time make -j$ncpus __tooldeps__"); if ($? != 0) { @@ -147,7 +147,7 @@ sub BuildTestExecutables($$) }
InfoMsg "\nBuilding the $Bits-bit test executable(s)\n"; - system("cd $DataDir/build-mingw$Bits && set -x && ". + system("cd '$DataDir/build-mingw$Bits' && set -x && ". "time make -j$ncpus ". join(" ", sort @BuildDirs)); if ($? != 0) { @@ -226,6 +226,12 @@ else FatalError "Invalid number of bits $BitIndicators\n"; }
+if ($DataDir =~ /'/) +{ + LogMsg "The install path contains invalid characters\n"; + exit(1); +} + my $Impacts = ApplyPatch($PatchFile); exit(1) if (!$Impacts);
diff --git a/testbot/bin/build/Reconfig.pl b/testbot/bin/build/Reconfig.pl index 2da2f54a4..5ecbc2af3 100755 --- a/testbot/bin/build/Reconfig.pl +++ b/testbot/bin/build/Reconfig.pl @@ -73,7 +73,7 @@ sub BuildTestAgentd() if (! -x "$BinDir/build/testagentd") { InfoMsg "\nBuilding the native testagentd\n"; - system("cd $::RootDir/src/testagentd && set -x && ". + system("cd '$::RootDir/src/testagentd' && set -x && ". "time make -j$ncpus build"); if ($? != 0) { @@ -83,7 +83,7 @@ sub BuildTestAgentd() }
InfoMsg "\nRebuilding the Windows TestAgentd\n"; - system("cd $::RootDir/src/testagentd && set -x && ". + system("cd '$::RootDir/src/testagentd' && set -x && ". "time make -j$ncpus iso"); if ($? != 0) { @@ -97,7 +97,7 @@ sub BuildTestAgentd() sub BuildTestLauncher() { InfoMsg "\nRebuilding TestLauncher\n"; - system("cd $::RootDir/src/TestLauncher && set -x && ". + system("cd '$::RootDir/src/TestLauncher' && set -x && ". "time make -j$ncpus"); if ($? != 0) { @@ -111,7 +111,7 @@ sub BuildTestLauncher() sub GitPull() { InfoMsg "\nUpdating the Wine source\n"; - system("cd $DataDir/wine && git pull"); + system("cd '$DataDir/wine' && git pull"); if ($? != 0) { LogMsg "Git pull failed\n"; @@ -134,7 +134,7 @@ sub BuildNative()
# Rebuild from scratch to make sure cruft will not accumulate InfoMsg "\nRebuilding native tools\n"; - system("cd $DataDir/build-native && set -x && ". + system("cd '$DataDir/build-native' && set -x && ". "rm -rf * && ". "time ../wine/configure --enable-win64 --without-x --without-freetype --disable-winetest && ". "time make -j$ncpus __tooldeps__"); @@ -157,7 +157,7 @@ sub BuildCross($)
# Rebuild from scratch to make sure cruft will not accumulate InfoMsg "\nRebuilding the $Bits-bit test executables\n"; - system("cd $DataDir/build-mingw$Bits && set -x && ". + system("cd '$DataDir/build-mingw$Bits' && set -x && ". "rm -rf * && ". "time ../wine/configure --host=$Host --with-wine-tools=../build-native --without-x --without-freetype --disable-winetest && ". "time make -j$ncpus buildtests"); @@ -173,6 +173,11 @@ sub BuildCross($) $ENV{PATH} = "/usr/lib/ccache:/usr/bin:/bin"; delete $ENV{ENV};
+if ($DataDir =~ /'/) +{ + LogMsg "The install path contains invalid characters\n"; + exit(1); +} if (! -d "$DataDir/staging" and ! mkdir "$DataDir/staging") { LogMsg "Unable to create '$DataDir/staging': $!\n";
This makes it much easier to test the scripts directly on the VM. This also makes it possible to test one aspect of the scripts without having to go through the whole process. In particular the --no-rm Reconfig.pl option speeds up testing by avoiding rebuilding from scratch.
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/bin/build/Build.pl | 175 +++++++++++++++++++++++----------- testbot/bin/build/Reconfig.pl | 137 +++++++++++++++++++++++--- 2 files changed, 241 insertions(+), 71 deletions(-)
diff --git a/testbot/bin/build/Build.pl b/testbot/bin/build/Build.pl index 17ae74ff5..4df107005 100755 --- a/testbot/bin/build/Build.pl +++ b/testbot/bin/build/Build.pl @@ -43,12 +43,19 @@ sub BEGIN } $::BuildEnv = 1; } +my $Name0 = $0; +$Name0 =~ s+^.*/++; +
use WineTestBot::Config; use WineTestBot::PatchUtils; use WineTestBot::Utils;
+# +# Logging and error handling helpers +# + sub InfoMsg(@) { print @_; @@ -59,12 +66,16 @@ sub LogMsg(@) print "Build: ", @_; }
-sub FatalError(@) +sub Error(@) { - print STDERR @_; - exit 1; + print STDERR "$Name0:error: ", @_; }
+ +# +# Build helpers +# + my $ncpus; sub CountCPUs() { @@ -133,9 +144,11 @@ sub BuildNative() return 1; }
-sub BuildTestExecutables($$) +sub BuildTestExecutables($$$) { - my ($Impacts, $Bits) = @_; + my ($Targets, $Impacts, $Bits) = @_; + + return 1 if (!$Targets->{"exe$Bits"});
my (@BuildDirs, @TestExes); foreach my $TestInfo (values %{$Impacts->{Tests}}) @@ -168,62 +181,113 @@ sub BuildTestExecutables($$) return $Success; }
+ +# +# Setup and command line processing +# + $ENV{PATH} = "/usr/lib/ccache:/usr/bin:/bin"; delete $ENV{ENV};
-my ($PatchFile, $BitIndicators); -if (@ARGV == 2) -{ - ($PatchFile, $BitIndicators) = @ARGV; -} -else -{ - # FIXME Remove support for the legacy parameters - my ($_PatchType, $_BaseName); - ($PatchFile, $_PatchType, $_BaseName, $BitIndicators) = @ARGV; -} -if (! $PatchFile || !$BitIndicators) -{ - FatalError "Usage: Build.pl <patchfile> <bits>\n"; -} - -# Verify parameters -if (!IsValidFileName($PatchFile)) -{ - FatalError "The patch filename '$PatchFile' contains invalid characters\n"; -} -$PatchFile = "$DataDir/staging/$PatchFile"; -if (!-r $PatchFile) -{ - FatalError "Patch file '$PatchFile' is not readable\n"; -} +my %AllTargets; +map { $AllTargets{$_} = 1 } qw(exe32 exe64);
-my ($Run32, $Run64); -if ($BitIndicators =~ m/^([\d,]+)$/) +my ($Usage, $PatchFile, $TargetList); +my $IgnoreNext = 0; # FIXME Backward compatibility +while (@ARGV) { - foreach my $BitsValue (split /,/, $1) + my $Arg = shift @ARGV; + if ($Arg =~ /^patch(?:dlls|programs)$/) { - if ($BitsValue eq "32") - { - $Run32 = 1; - } - elsif ($BitsValue eq "64") + $IgnoreNext ||= 1; # Ignore this legacy parameter + } + elsif ($IgnoreNext == 1) + { + $IgnoreNext = 2; # Ignore this legacy parameter + } + elsif ($Arg =~ /^(?:-?|-h|--help)$/) + { + $Usage = 0; + last; + } + elsif ($Arg =~ /^-/) + { + Error "unknown option '$Arg'\n"; + $Usage = 2; + last; + } + elsif (!defined $PatchFile) + { + if (IsValidFileName($Arg)) { - $Run64 = 1; + $PatchFile = "$DataDir/staging/$Arg"; + if (!-r $PatchFile) + { + Error "patch file '$Arg' is not readable\n"; + $Usage = 2; + } } else { - FatalError "Invalid number of bits $BitsValue\n"; + Error "the patch filename '$Arg' contains invalid characters\n"; + $Usage = 2; + last; } } - if (!$Run32 && !$Run64) + elsif (!defined $TargetList) { - FatalError "Specify at least one of 32 or 64 bits\n"; + $TargetList = $Arg; + } + else + { + Error "unexpected argument '$Arg'\n"; + $Usage = 2; + last; + } +} + +# Check and untaint parameters +my $Targets; +if (!defined $Usage) +{ + if (!defined $PatchFile) + { + Error "you must specify a patch to apply\n"; + $Usage = 2; + } + + $TargetList = join(",", keys %AllTargets) if (!defined $TargetList); + foreach my $Target (split /,/, $TargetList) + { + $Target = "exe$1" if ($Target =~ /^(32|64)$/); + if (!$AllTargets{$Target}) + { + Error "invalid target name $Target\n"; + $Usage = 2; + last; + } + $Targets->{$Target} = 1; } } -else +if (defined $Usage) { - FatalError "Invalid number of bits $BitIndicators\n"; + if ($Usage) + { + Error "try '$Name0 --help' for more information\n"; + exit $Usage; + } + print "Usage: $Name0 [--help] PATCHFILE TARGETS\n"; + print "\n"; + print "Applies the specified patch and rebuilds the Wine test executables.\n"; + print "\n"; + print "Where:\n"; + print " PATCHFILE Is the staging file containing the patch to build.\n"; + print " TARGETS Is a comma-separated list of build targets. By default every\n"; + print " target is run.\n"; + print " - exe32: Rebuild the 32 bit Windows test executables.\n"; + print " - exe64: Rebuild the 64 bit Windows test executables.\n"; + print " --help Shows this usage message.\n"; + exit 0; }
if ($DataDir =~ /'/) @@ -232,20 +296,19 @@ if ($DataDir =~ /'/) exit(1); }
-my $Impacts = ApplyPatch($PatchFile); -exit(1) if (!$Impacts); + +# +# Run the builds +#
CountCPUs();
-if ($Impacts->{WineBuild} and !BuildNative()) -{ - exit(1); -} -if ($Run32 && !BuildTestExecutables($Impacts, 32)) -{ - exit(1); -} -if ($Run64 && !BuildTestExecutables($Impacts, 64)) +my $Impacts = ApplyPatch($PatchFile); + +if (!$Impacts or + ($Impacts->{WineBuild} and !BuildNative()) or + !BuildTestExecutables($Targets, $Impacts, 32) or + !BuildTestExecutables($Targets, $Impacts, 64)) { exit(1); } diff --git a/testbot/bin/build/Reconfig.pl b/testbot/bin/build/Reconfig.pl index 5ecbc2af3..450de0e83 100755 --- a/testbot/bin/build/Reconfig.pl +++ b/testbot/bin/build/Reconfig.pl @@ -38,12 +38,19 @@ sub BEGIN unshift @INC, "$::RootDir/lib"; } $::BuildEnv = 1; - } +} +my $Name0 = $0; +$Name0 =~ s+^.*/++; +
use WineTestBot::Config; use WineTestBot::PatchUtils;
+# +# Logging and error handling helpers +# + sub InfoMsg(@) { print @_; @@ -54,6 +61,16 @@ sub LogMsg(@) print "Reconfig: ", @_; }
+sub Error(@) +{ + print STDERR "$Name0:error: ", @_; +} + + +# +# Build helpers +# + my $ncpus; sub CountCPUs() { @@ -108,8 +125,11 @@ sub BuildTestLauncher() return 1; }
-sub GitPull() +sub GitPull($) { + my ($Targets) = @_; + return 1 if (!$Targets->{update}); + InfoMsg "\nUpdating the Wine source\n"; system("cd '$DataDir/wine' && git pull"); if ($? != 0) @@ -128,14 +148,17 @@ sub GitPull() return 1; }
-sub BuildNative() +sub BuildNative($$) { + my ($Targets, $NoRm) = @_; + + return 1 if (!$Targets->{native}); mkdir "$DataDir/build-native" if (! -d "$DataDir/build-native");
# Rebuild from scratch to make sure cruft will not accumulate InfoMsg "\nRebuilding native tools\n"; system("cd '$DataDir/build-native' && set -x && ". - "rm -rf * && ". + ($NoRm ? "" : "rm -rf * && ") . "time ../wine/configure --enable-win64 --without-x --without-freetype --disable-winetest && ". "time make -j$ncpus __tooldeps__");
@@ -148,17 +171,18 @@ sub BuildNative() return 1; }
-sub BuildCross($) +sub BuildCross($$$) { - my ($Bits) = @_; + my ($Targets, $NoRm, $Bits) = @_;
- my $Host = ($Bits == 64 ? "x86_64-w64-mingw32" : "i686-w64-mingw32"); - mkdir "$DataDir/build-mingw$Bits" if (! -d "$DataDir/build-mingw$Bits"); + return 1 if (!$Targets->{"exe$Bits"}); + mkdir "$DataDir/build-mingw$Bits" if (!-d "$DataDir/build-mingw$Bits");
# Rebuild from scratch to make sure cruft will not accumulate InfoMsg "\nRebuilding the $Bits-bit test executables\n"; + my $Host = ($Bits == 64 ? "x86_64-w64-mingw32" : "i686-w64-mingw32"); system("cd '$DataDir/build-mingw$Bits' && set -x && ". - "rm -rf * && ". + ($NoRm ? "" : "rm -rf * && ") . "time ../wine/configure --host=$Host --with-wine-tools=../build-native --without-x --without-freetype --disable-winetest && ". "time make -j$ncpus buildtests"); if ($? != 0) @@ -170,9 +194,87 @@ sub BuildCross($) return 1; }
+ +# +# Setup and command line processing +# + $ENV{PATH} = "/usr/lib/ccache:/usr/bin:/bin"; delete $ENV{ENV};
+my %AllTargets; +map { $AllTargets{$_} = 1 } qw(update native exe32 exe64); + +my ($Usage, $TargetList, $NoRm); +while (@ARGV) +{ + my $Arg = shift @ARGV; + if ($Arg eq "--no-rm") + { + $NoRm = 1; + } + elsif ($Arg =~ /^(?:-?|-h|--help)$/) + { + $Usage = 0; + last; + } + elsif ($Arg =~ /^-/) + { + Error "unknown option '$Arg'\n"; + $Usage = 2; + last; + } + elsif (!defined $TargetList) + { + $TargetList = $Arg; + } + else + { + Error "unexpected argument '$Arg'\n"; + $Usage = 2; + last; + } +} + +# Check and untaint parameters +my $Targets; +if (!defined $Usage) +{ + $TargetList = join(",", keys %AllTargets) if (!defined $TargetList); + foreach my $Target (split /,/, $TargetList) + { + if (!$AllTargets{$Target}) + { + Error "invalid target name $Target\n"; + $Usage = 2; + last; + } + $Targets->{$Target} = 1; + } +} +if (defined $Usage) +{ + if ($Usage) + { + Error "try '$Name0 --help' for more information\n"; + exit $Usage; + } + print "Usage: $Name0 [--no-rm] [--help] [TARGETS]\n"; + print "\n"; + print "Updates Wine to the latest version and recompiles it so the host is ready to build executables for the Windows tests.\n"; + print "\n"; + print "Where:\n"; + print " TARGETS Is a comma-separated list of reconfiguration targets. By default\n"; + print " every target is run.\n"; + print " - update: Update Wine's source code.\n"; + print " - native: Rebuild the native Wine tools.\n"; + print " - exe32: Rebuild the 32 bit Windows test executables.\n"; + print " - exe64: Rebuild the 64 bit Windows test executables.\n"; + print " --no-rm Don't rebuild from scratch.\n"; + print " --help Shows this usage message.\n"; + exit 0; +} + if ($DataDir =~ /'/) { LogMsg "The install path contains invalid characters\n"; @@ -184,14 +286,19 @@ if (! -d "$DataDir/staging" and ! mkdir "$DataDir/staging") exit(1); }
+ +# +# Run the builds +# + CountCPUs();
-if (!BuildTestAgentd() || - !BuildTestLauncher() || - !GitPull() || - !BuildNative() || - !BuildCross(32) || - !BuildCross(64)) +if (!BuildTestAgentd() or + !BuildTestLauncher() or + !GitPull($Targets) or + !BuildNative($Targets, $NoRm) or + !BuildCross($Targets, $NoRm, 32) or + !BuildCross($Targets, $NoRm, 64)) { exit(1); }
-
Francois Gouget