Date: Wed, 2 Oct 2013 18:22:43 +0000 (UTC) From: Dag-Erling Smørgrav <des@FreeBSD.org> To: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: svn commit: r255999 - user/des/tinderbox Message-ID: <201310021822.r92IMhj6092441@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: des Date: Wed Oct 2 18:22:42 2013 New Revision: 255999 URL: http://svnweb.freebsd.org/changeset/base/255999 Log: Improve the logic used to determine the default value of the HOSTNAME configuration variable. Introduce an NCPU configuration variable and implement similar logic as for the HOSTNAME configuration variable. Change the configuration variable substitution syntax from %%FOO%% to ${FOO}. The old syntax is still supported for backward compatibility, but is not used internally and is deemphasized in the documentation. Introduce support for using environment variables in configuration files. This requires a fair amount of validation and untainting. Clean up the man page, especially the list of configuration variables, and provide more detailed explanations of some of the darker corners of the code as well as advice about tuning JOBS and NCPU. Modified: user/des/tinderbox/tbmaster.1 user/des/tinderbox/tbmaster.pl Modified: user/des/tinderbox/tbmaster.1 ============================================================================== --- user/des/tinderbox/tbmaster.1 Wed Oct 2 18:12:18 2013 (r255998) +++ user/des/tinderbox/tbmaster.1 Wed Oct 2 18:22:42 2013 (r255999) @@ -25,7 +25,7 @@ .\" .\" $FreeBSD$ .\" -.Dd September 23, 2013 +.Dd October 2, 2013 .Dt TBMASTER 1 .Os .Sh NAME @@ -66,17 +66,30 @@ Dumps the configuration and exits withou The directory where configuration files are located. The default value is .Pa $HOME/etc . -.It Fl l Ar FILE +.It Fl h Ar HOSTNAME , Fl -hostname Ns = Ns Ar HOSTNAME +The name of the host running the tinderbox, used in logs and reports. +Can be overridden by the +.Va HOSTNAME +configuration variable. +The default value is that reported by +.Xr uname 1 . +.It Fl l Ar FILE , Fl -lockfile Ns = Ns Ar FILE The name of a file to lock upon startup. If the lock is already held by another process, .Nm will terminate immediately rather than block. -.It Fl n Ar ncpu -The number of concurrent jobs to run. -The default value is the number of CPUs in the machine, or 1 if that +.It Fl n Ar NUMBER , Fl -ncpu Ns = Ns Ar NUMBER +The maximum number of concurrent builds to run. +Can be overridden by the +.Va NCPU +configuration variable. +The default value is the number of cores in the machine, or 1 if that number could not be determined. +See the +.Sx Concurrency +section for additional information. .El -.Ss Configuration +.Ss Configuration files The .Nm script uses named configurations located in individual files named for @@ -96,14 +109,15 @@ and .Pa site.rc before and after the actual configuration file, respectively; thus, they can be used to specify default values shared by multiple -configurations. +configurations and to override the values set in the individual +configuration files. .Pp -The configuration consists of a list of single- or multiple-value -variable assignments: +Each configuration file consists of a list of single- or +multiple-value variable assignments: .Bl -tag .It Va single_variable = Ar value -.It Va multi_variable = Ar value1 , Ar value2 , ... -.It Va multi_variable += Ar value3 , ... +.It Va multi_variable = Ar value1 Op , Ar value2 ... +.It Va multi_variable += Ar value3 Op , Ar value4 ... .El .Pp Whitespace around the equal sign and around the commas separating @@ -112,23 +126,40 @@ multiple values is optional. Blank lines are ignored, as is anything following a hash sign .Pq Sq # . .Pp -Additionally, the +Additionally, .Cm include -statement can be used to include one configuration in another: +statements can be used to include one configuration in another: .Bl -tag .It Cm include Ar otherconfig .El +.Ss Configuration variables +Below is a list of the configuration variables +.Nm +recognizes and their semantics. +.Pp +Note that many of these variables are passed on as command-line +arguments to +.Xr tinderbox 1 , +which may provide its own default values for variables which are left +undefined by +.Nm . .Pp -The following configuration variables are defined: +Some variables are read-only and are provided so that other variables +may include them. +For instance, a common idiom is to derive +.Va OBJDIR +from a combination of +.Va BRANCH , +.Va ARCH +and +.Va MACHINE . .Bl -tag -width 12n .It ARCH -.Pq Vt single +.Pq Vt single, read-only The architecture currently being built for. -Read-only. .It BRANCH -.Pq Vt single +.Pq Vt single, read-only The branch currently being built. -Read-only. .It BRANCHES .Pq Vt multiple A list of source branches to build. @@ -160,12 +191,6 @@ No default value. .Pq Vt single A terse comment describing the setup. No default value. -.It CVSUP -.Pq Vt single -The name of the -.Xr cvsup 1 -server to use. -No default value. .It ENV .Pq Vt multiple A list of environment variables to pass to the @@ -176,43 +201,42 @@ by an equal sign .Pq Sq = . No default value. .It HOME -.Pq Vt single +.Pq Vt single, read-only The current user's home directory, as specified by the .Ev HOME environment variable. Note that it will not be defined unless it passes some simple sanity checks. -Read-only. .It HOSTNAME .Pq Vt single The name of the host running the tinderbox. -This defaults to the name reported by the -.Fl n -option of the -.Xr uname 1 -command, and is only used for cosmetic purposes. .It JOBS The maximum number of concurrent .Xr make 1 -jobs to run. +processes to run within each build. No default value. +See the +.Sx Concurrency +section for additional information. .It LOGDIR .Pq Vt single The location of the log directory. The default value is -.Pa %%SANDBOX%%/logs . +.Pa ${SANDBOX}/logs . .It MACHINE -.Pq Vt single +.Pq Vt single, read-only The machine currently being built for. -Read-only. +.It NCPU +.Pq Vt single +The maximum number of concurrent builds to run. +No default value. +See the +.Sx Concurrency +section for additional information. .It OBJDIR .Pq Vt single The object directory. -There is no default value; see the -.Xr tinderbox 1 -script's -.Fl -objdir -option for details. +No default value. .It OPTIONS .Pq Vt multiple A list of additional options to pass to the @@ -241,19 +265,13 @@ The default value is .Pq Vt multiple The addresses to which failure reports should be mailed. The default value is -.Dq %%SENDER%% . +.Dq ${SENDER} . .Pp To avoid unintentional spamming, .Nm will strip recipients in the .Li freebsd.org domain from this list unless the correct magic sauce is used. -.It REPOSITORY -.Pq Vt single -The location of the -.Xr cvs 1 -repository. -No default value. .It SANDBOX .Pq Vt single The location of the sandbox directory. @@ -267,11 +285,7 @@ No default value. .It SRCDIR .Pq Vt single The source directory. -There is no default value; see the -.Xr tinderbox 1 -script's -.Fl -srcdir -option for details. +No default value. .Pp Normally, a separate directory within the sandbox will be used for each build. @@ -288,10 +302,11 @@ targets. .Pq Vt single The subject to use on failure reports. The default value is -.Dq Tinderbox failure on %%arch%%/%%machine%% . +.Dq Tinderbox failure on ${arch}/${machine} . .It SVNBASE .Pq Vt single The URL to the base of the Subversion repository. +No default value. .It TARGETS .Pq Vt multiple A list of targets (commands) to pass to the @@ -310,7 +325,7 @@ The location of the .Xr tinderbox 1 script. The default value is -.Dq %%HOME%%/bin/tinderbox . +.Dq ${HOME}/bin/tinderbox . .It URLBASE .Pq Vt single If defined, a URL constructed by appending the file name of the full @@ -324,26 +339,32 @@ immediately before use: .Bl -bullet .It If a single-value variable contains substrings of the form -.Va %%VAR%% +.Va ${VAR} or -.Va %%var%% , +.Va ${var} , those substrings are replaced with the values of the corresponding variables, after recursive substitution. The difference between the first and the second form is that the latter is converted to lower-case before use. For instance, -.Dq %%BRANCH%% +.Dq ${BRANCH} might expand to .Dq RELENG_4 while -.Dq %%branch%% +.Dq ${branch} would expand to .Dq releng_4 . .It +If a single-value varaible contains substrings of the form +.Va $ENV{VAR} , +those substrings are replaced with the values of the corresponding +environment variables. +Use this with care. +.It If an element of a multiple-value variable is of the form -.Va %%VAR%% +.Va ${VAR} or -.Va %%var%% +.Va ${var} and the corresponding variable is a multiple-value variable, recursive substitution is first performed on that variable, and the resulting values are included individually in the result. @@ -351,8 +372,53 @@ values are included individually in the Otherwise, elements of multiple-value variables are expanded individually according to the same rules as single-value variables. .El +.Pp +For backward compatibility with earlier versions, the forms +.Va %%VAR%% +and +.Va %%var%% +may be used instead of +.Va ${VAR} +and +.Va ${var} . +.Ss Concurrency +On multiprocessor machines, performance can generally be improved by +running multiple builds in parallel, up to a certain limit. +By default, +.Nm +will run one build for each processor core in the system. +This can be overridden with the +.Fl -ncpu +command-line option and the +.Va NCPU +configuration variable, the latter taking precedence. +.Pp +In addition, each build may run multiple +.Xr make 1 +processes in parallel, up to the number specified by the +.Va JOBS +configuration variable. +.Pp +The total number of parallel +.Xr make 1 +processes will vary, but can be as high as the product of of +.Va NCPU +and +.Va JOBS. +As a result of processor, memory and filesystem contention, an +excessively large value can have a significant negative impact on +performance. +.Pp +As a rule of thumb, +.Va NCPU +should not exceed one build per gigabyte of physical memory in the +system, and the +.Va NCPU +x +.Va JOBS +product should not exceed the number of processor cores in the system +by a large amount. .Sh SEE ALSO -.Xr perl 1 , .Xr tinderbox 1 .Sh AUTHORS .Nm Modified: user/des/tinderbox/tbmaster.pl ============================================================================== --- user/des/tinderbox/tbmaster.pl Wed Oct 2 18:12:18 2013 (r255998) +++ user/des/tinderbox/tbmaster.pl Wed Oct 2 18:22:42 2013 (r255999) @@ -34,7 +34,7 @@ use POSIX; use Getopt::Long; use Storable qw(dclone); -my $VERSION = "2.11"; +my $VERSION = "2.20"; my $COPYRIGHT = "Copyright (c) 2003-2013 Dag-Erling Smørgrav. " . "All rights reserved."; @@ -46,6 +46,7 @@ my $dump; # Dump configuration and exi my $etcdir; # Configuration directory my $lockfile; # Lock file name my $lock; # Lock file descriptor +my $hostname; # Hostname my $ncpu; # Number of CPUs my %platforms; # Specific platforms to build @@ -54,27 +55,30 @@ my %INITIAL_CONFIG = ( 'CFLAGS' => '', 'COPTFLAGS' => '', 'COMMENT' => '', - 'CVSUP' => '', 'ENV' => [ ], 'HOSTNAME' => '', 'JOBS' => '', - 'LOGDIR' => '%%SANDBOX%%/logs', + 'LOGDIR' => '${SANDBOX}/logs', + 'NCPU' => '', 'OBJDIR' => '', 'OPTIONS' => [ ], 'PATCH' => '', 'PLATFORMS' => [ 'i386' ], - 'RECIPIENT' => [ '%%SENDER%%' ], - 'REPOSITORY'=> '', + 'RECIPIENT' => [ '${SENDER}' ], 'SANDBOX' => '/tmp/tinderbox', 'SENDER' => '', 'SRCDIR' => '', - 'SUBJECT' => 'Tinderbox failure on %%arch%%/%%machine%%', + 'SUBJECT' => 'Tinderbox failure on ${arch}/${machine}', 'SVNBASE' => '', 'TARGETS' => [ 'update', 'world' ], 'TIMEOUT' => '', - 'TINDERBOX' => '%%HOME%%/bin/tinderbox', + 'TINDERBOX' => '${HOME}/bin/tinderbox', 'URLBASE' => '', ); +my %NUMERIC_OPTIONS = map { $_ => 1 } qw(JOBS NCPU TIMEOUT); +my %PATHNAME_OPTIONS = + map { $_ => 1 } qw(LOGDIR OBJDIR PATCH SANDBOX SRCDIR TINDERBOX); +my %WORD_OPTIONS = map { $_ => 1 } qw(PLATFORMS TARGETS); my %CONFIG; # @@ -122,18 +126,45 @@ sub expand($) { if (ref($elem)) { # prepend to queue for further processing unshift(@elements, @{$elem}); - } elsif ($elem =~ m/^\%\%(\w+)\%\%$/) { + } elsif ($elem =~ m/^\%\%(\w+)\%\%$/ || $elem =~ m/^\%\{(\w+)\}$/) { # prepend to queue for further processing # note - can expand to a list unshift(@elements, expand($1)); } else { + $elem =~ s/\$ENV\{(\w+)\}/$ENV{$1}/g; $elem =~ s/\%\%(\w+)\%\%/expand($1)/eg; + $elem =~ s/\$\{(\w+)\}/expand($1)/eg; push(@expanded, $elem); } } + + # Upper / lower case if ($key !~ m/[A-Z]/) { @expanded = map { lc($_) } @expanded; } + + # Validate and untaint expanded value(s) + if ($NUMERIC_OPTIONS{uc($key)}) { + @expanded = map { + m/^(\d+|)$/ + or die("invalid value for numeric variable $key: $_\n"); + $1 + } @expanded; + } elsif ($PATHNAME_OPTIONS{uc($key)}) { + @expanded = map { + m@^((?:/+[\w.-]+)+/*|)$@ + or die("invalid value for pathname variable $key: $_\n"); + $1 + } @expanded; + } elsif ($WORD_OPTIONS{uc($key)}) { + @expanded = map { + m/^([\w.-]+|)$/ + or die("invalid value for word variable $key: $_\n"); + $1 + } @expanded; + } + + # Verify single / multiple and return result if (ref($value)) { return @expanded; } elsif (@expanded != 1) { @@ -158,7 +189,7 @@ sub readconf($); sub readconf($) { my $fn = shift; - open(my $fh, "<", $fn) + open(my $fh, '<', $fn) or return undef; my $line = ""; my $n = 0; @@ -227,7 +258,7 @@ sub history($$$) { $history .= $success ? "OK\n" : "FAIL\n"; my $fn = expand('LOGDIR') . "/history"; - if (open(my $fh, ">>", $fn)) { + if (open(my $fh, '>>', $1)) { print($fh $history); close($fh); } else { @@ -254,7 +285,7 @@ sub report($$$$) { return; } - if (open(my $pipe, "|-", "/usr/sbin/sendmail", "-i", "-t", "-f$sender")) { + if (open(my $pipe, '|-', qw(/usr/sbin/sendmail -i -t -f), $sender)) { print($pipe "Sender: $sender\n"); print($pipe "From: $sender\n"); print($pipe "To: $recipient\n"); @@ -288,18 +319,22 @@ sub tinderbox($$$) { # Open log files: one for the full log and one for the summary my $logdir = expand('LOGDIR'); - my $logfile = "tinderbox-$config-$branch-$arch-$machine"; + if (!-d $logdir) { + die("nonexistent log directory: $logdir\n"); + } + my $logname = "tinderbox-$config-$branch-$arch-$machine"; + my $logbase = "$logdir/$logname"; my $full; - if (!open($full, ">", "$logdir/$logfile.full.$$")) { - warn("$logdir/$logfile.full.$$: $!\n"); + if (!open($full, '>', "$logbase.full.$$")) { + warn("$logbase.full.$$: $!\n"); return undef; } select($full); $| = 1; select(STDOUT); my $brief; - if (!open($brief, ">", "$logdir/$logfile.brief.$$")) { - warn("$logdir/$logfile.brief.$$: $!\n"); + if (!open($brief, '>', "$logbase.brief.$$")) { + warn("$logbase.brief.$$: $!\n"); return undef; } select($brief); @@ -310,9 +345,9 @@ sub tinderbox($$$) { my ($rpipe, $wpipe); if (!pipe($rpipe, $wpipe)) { warn("pipe(): $!\n"); - unlink("$logdir/$logfile.brief.$$"); + unlink("$logbase.brief.$$"); close($brief); - unlink("$logdir/$logfile.full.$$"); + unlink("$logbase.full.$$"); close($full); return undef; } @@ -327,8 +362,6 @@ sub tinderbox($$$) { if ($CONFIG{'OBJDIR'}); push(@args, "--arch=$arch"); push(@args, "--machine=$machine"); - push(@args, "--cvsup=" . expand('CVSUP')) - if ($CONFIG{'CVSUP'}); push(@args, "--repository=" . expand('REPOSITORY')) if ($CONFIG{'REPOSITORY'}); push(@args, "--branch=$branch"); @@ -349,15 +382,15 @@ sub tinderbox($$$) { my $pid = fork(); if (!defined($pid)) { warn("fork(): $!\n"); - unlink("$logdir/$logfile.brief.$$"); + unlink("$logbase.brief.$$"); close($brief); - unlink("$logdir/$logfile.full.$$"); + unlink("$logbase.full.$$"); close($full); return undef; } elsif ($pid == 0) { close($rpipe); - open(STDOUT, ">&", $wpipe); - open(STDERR, ">&", $wpipe); + open(STDOUT, '>&', $wpipe); + open(STDERR, '>&', $wpipe); $| = 1; exec(expand('TINDERBOX'), @args); die("exec(): $!\n"); @@ -440,13 +473,13 @@ sub tinderbox($$$) { my $recipient = join(', ', @recipients); my $subject = expand('SUBJECT'); if ($CONFIG{'URLBASE'}) { - $summary .= "\n\n" . expand('URLBASE') . "$logfile.full"; + $summary .= "\n\n" . expand('URLBASE') . "$logname.full"; } report($sender, $recipient, $subject, $summary); } - rename("$logdir/$logfile.full.$$", "$logdir/$logfile.full"); - rename("$logdir/$logfile.brief.$$", "$logdir/$logfile.brief"); + rename("$logbase.full.$$", "$logbase.full"); + rename("$logbase.brief.$$", "$logbase.brief"); } # @@ -547,6 +580,7 @@ sub tbmaster($) { die("Where is the tinderbox script?\n"); } + # Check stop file my $stopfile = expand('SANDBOX') . "/stop"; my @jobs; foreach my $branch (expand('BRANCHES')) { @@ -559,12 +593,15 @@ sub tbmaster($) { } } + # Main loop: start as many concurrent jobs as permitted, then keep + # starting new jobs as soon as existing jobs terminate, until all + # jobs have terminated and there are none left in the queue. $0 = "tbmaster [$config]: supervisor"; my %children; my $done = 0; while (@jobs || keys(%children)) { # start more children if we can - while (@jobs && keys(%children) < $ncpu) { + while (@jobs && keys(%children) < expand('NCPU')) { my ($branch, $arch, $machine) = @{shift(@jobs)}; if (-e $stopfile || -e "$stopfile.$branch" || -e "$stopfile.$arch" || -e "$stopfile.$arch.$machine") { @@ -600,30 +637,33 @@ sub tbmaster($) { } # +# Read the input from a command +# +sub slurp(@) { + my @cmdline = @_; + + if (open(my $pipe, '-|', @cmdline)) { + local $/; + my $input = <$pipe>; + close($pipe); + return $input; + } + return undef; +} + +# # Main # MAIN:{ # Set defaults $ENV{'TZ'} = "UTC"; $ENV{'PATH'} = "/usr/bin:/usr/sbin:/bin:/sbin"; - $INITIAL_CONFIG{'HOSTNAME'} = `/usr/bin/uname -n`; - if ($INITIAL_CONFIG{'HOSTNAME'} =~ m/^([0-9a-z-]+(?:\.[0-9a-z-]+)*)$/) { - $INITIAL_CONFIG{'HOSTNAME'} = $1; - } else { - $INITIAL_CONFIG{'HOSTNAME'} = 'unknown'; - } if ($ENV{'HOME'} =~ m/^((?:\/[\w\.-]+)+)\/*$/) { $INITIAL_CONFIG{'HOME'} = realpath($1); $etcdir = "$1/etc"; $ENV{'PATH'} = "$1/bin:$ENV{'PATH'}" if (-d "$1/bin"); } - $ncpu = `/sbin/sysctl -n hw.ncpu`; - if ($ncpu =~ m/^\s*(\d+)\s*$/) { - $ncpu = int($1); - } else { - $ncpu = 1; - } # Get options {Getopt::Long::Configure("auto_abbrev", "bundling");} @@ -632,10 +672,12 @@ MAIN:{ "c|config=s" => \@configs, "d|dump" => \$dump, "e|etcdir=s" => \$etcdir, + "h|hostname=s" => \$hostname, "l|lockfile=s" => \$lockfile, "n|ncpu=i" => \$ncpu, ) or usage(); + # Subsequent arguments are platforms to build foreach (@ARGV) { if (m/^(\w+(?:\/\w+)?)$/) { $platforms{$1} = 1; @@ -644,11 +686,34 @@ MAIN:{ } } + # Get / check hostname + if (!$hostname) { + $hostname = slurp(qw(/usr/bin/uname -n)); + } + if ($hostname && + $hostname =~ m/^\s*([a-z][0-9a-z-]+(?:\.[a-z][0-9a-z-]+)*)\s*$/s) { + $hostname = $1; + } else { + $hostname = 'unknown'; + } + $INITIAL_CONFIG{'HOSTNAME'} = $hostname; + + # Get / check number of CPUs + if (!$ncpu) { + $ncpu = slurp(qw(/sbin/sysctl -n hw.ncpu)); + } + if ($ncpu && $ncpu =~ m/^\s*(\d+)\s*$/s) { + $ncpu = int($1); + } else { + $ncpu = 1; + } + $INITIAL_CONFIG{'NCPU'} = $ncpu; + # Check options if (@configs) { @configs = split(/,/, join(',', @configs)); } else { - $configs[0] = `/usr/bin/uname -n`; + $configs[0] = $hostname; chomp($configs[0]); $configs[0] =~ s/^(\w+)(\..*)?/$1/; } @@ -672,7 +737,7 @@ MAIN:{ die("invalid lockfile\n"); } $lockfile = $1; - $lock = open_locked($lockfile, ">", 0600) + $lock = open_locked($lockfile, '>', 0600) or die("unable to acquire lock on $lockfile\n"); # Lock will be released upon termination. }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201310021822.r92IMhj6092441>