From 0c5a9f9b92af1634dc60fa21e9ac86ed50e5d595 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Mon, 30 Oct 2017 12:53:49 -0400 Subject: * main.c (main): [SV 48274] Allow -j in makefile MAKEFLAGS variable. * tests/jhelp.pl: New file to allow testing parallelism without sleep. * tests/scripts/features/parallelism: Test this. * tests/scripts/features/jobserver: Update tests. * tests/scripts/features/output-sync: Remove useless rm command. --- main.c | 80 ++++++++++++++++++++++++++-------- tests/jhelp.pl | 63 +++++++++++++++++++++++++++ tests/scripts/features/jobserver | 6 +-- tests/scripts/features/output-sync | 2 +- tests/scripts/features/parallelism | 89 +++++++++++++++++++++----------------- 5 files changed, 177 insertions(+), 63 deletions(-) create mode 100755 tests/jhelp.pl diff -rupN a/main.c b/main.c --- a/main.c 2021-10-18 15:23:21.769274000 -0400 +++ b/main.c 2021-10-18 15:30:43.579608645 -0400 @@ -1482,7 +1482,7 @@ main (int argc, char **argv, char **envp || output_sync == OUTPUT_SYNC_TARGET); OUTPUT_SET (&make_sync); - /* Remember the job slots set through the environment vs. command line. */ + /* Parse the command line options. Remember the job slots set this way. */ { int env_slots = arg_job_slots; arg_job_slots = INVALID_JOB_SLOTS; @@ -1609,41 +1609,38 @@ main (int argc, char **argv, char **envp /* We may move, but until we do, here we are. */ starting_directory = current_directory; - /* Set up the job_slots value and the jobserver. This can't be usefully set - in the makefile, and we want to verify the authorization is valid before - make has a chance to start using it for something else. */ + /* Validate the arg_job_slots configuration before we define MAKEFLAGS so + users get an accurate value in their makefiles. + At this point arg_job_slots is the argv setting, if there is one, else + the MAKEFLAGS env setting, if there is one. */ if (jobserver_auth) { + /* We're a child in an existing jobserver group. */ if (argv_slots == INVALID_JOB_SLOTS) { + /* There's no -j option on the command line: check authorization. */ if (jobserver_parse_auth (jobserver_auth)) { /* Success! Use the jobserver. */ - job_slots = 0; goto job_setup_complete; } + /* Oops: we have jobserver-auth but it's invalid :(. */ O (error, NILF, _("warning: jobserver unavailable: using -j1. Add '+' to parent make rule.")); arg_job_slots = 1; } - /* The user provided a -j setting on the command line: use it. */ + /* The user provided a -j setting on the command line so use it: we're + the master make of a new jobserver group. */ else if (!restarts) - /* If restarts is >0 we already printed this message. */ - O (error, NILF, - _("warning: -jN forced in submake: disabling jobserver mode.")); + ON (error, NILF, + _("warning: -j%d forced in submake: resetting jobserver mode."), + argv_slots); - /* We failed to use our parent's jobserver. */ + /* We can't use our parent's jobserver, so reset. */ reset_jobserver (); - job_slots = (unsigned int)arg_job_slots; } - else if (arg_job_slots == INVALID_JOB_SLOTS) - /* The default is one job at a time. */ - job_slots = 1; - else - /* Use whatever was provided. */ - job_slots = (unsigned int)arg_job_slots; job_setup_complete: @@ -1999,6 +1996,9 @@ main (int argc, char **argv, char **envp { int old_builtin_rules_flag = no_builtin_rules_flag; int old_builtin_variables_flag = no_builtin_variables_flag; + int old_arg_job_slots = arg_job_slots; + + arg_job_slots = INVALID_JOB_SLOTS; /* Decode switches again, for variables set by the makefile. */ decode_env_switches (STRING_SIZE_TUPLE ("GNUMAKEFLAGS")); @@ -2011,6 +2011,24 @@ main (int argc, char **argv, char **envp decode_env_switches (STRING_SIZE_TUPLE ("MFLAGS")); #endif + /* If -j is not set in the makefile, or it was set on the command line, + reset to use the previous value. */ + if (arg_job_slots == INVALID_JOB_SLOTS || argv_slots != INVALID_JOB_SLOTS) + arg_job_slots = old_arg_job_slots; + + else if (jobserver_auth) + { + /* Makefile MAKEFLAGS set -j, but we already have a jobserver. + Make us the master of a new jobserver group. */ + if (!restarts) + ON (error, NILF, + _("warning: -j%d forced in makefile: resetting jobserver mode."), + arg_job_slots); + + /* We can't use our parent's jobserver, so reset. */ + reset_jobserver (); + } + /* Reset in case the switches changed our mind. */ syncing = (output_sync == OUTPUT_SYNC_LINE || output_sync == OUTPUT_SYNC_TARGET); @@ -2037,8 +2055,31 @@ main (int argc, char **argv, char **envp undefine_default_variables (); } + /* Final jobserver configuration. + + If we have jobserver_auth then we are a client in an existing jobserver + group, that's already been verified OK above. If we don't have + jobserver_auth and jobserver is enabled, then start a new jobserver. + + arg_job_slots = INVALID_JOB_SLOTS if we don't want -j in MAKEFLAGS + + arg_job_slots = # of jobs of parallelism + + job_slots = 0 for no limits on jobs, or when limiting via jobserver. + + job_slots = 1 for standard non-parallel mode. + + job_slots >1 for old-style parallelism without jobservers. */ + + if (jobserver_auth) + job_slots = 0; + else if (arg_job_slots == INVALID_JOB_SLOTS) + job_slots = 1; + else + job_slots = arg_job_slots; + #if defined (__MSDOS__) || defined (__EMX__) || defined (VMS) - if (arg_job_slots != 1 + if (job_slots != 1 # ifdef __EMX__ && _osmode != OS2_MODE /* turn off -j if we are in DOS mode */ # endif @@ -2047,7 +2088,8 @@ main (int argc, char **argv, char **envp O (error, NILF, _("Parallel jobs (-j) are not supported on this platform.")); O (error, NILF, _("Resetting to single job (-j1) mode.")); - arg_job_slots = job_slots = 1; + arg_job_slots = INVALID_JOB_SLOTS; + job_slots = 1; } #endif diff -rupN a/tests/jhelp.pl b/tests/jhelp.pl --- a/tests/jhelp.pl 1969-12-31 19:00:00.000000000 -0500 +++ b/tests/jhelp.pl 2021-10-18 15:30:43.582608763 -0400 @@ -0,0 +1,63 @@ +#!/usr/bin/env perl +# -*-perl-*- +# +# This script helps us test jobserver/parallelism without a lot of unreliable +# (and slow) sleep calls. Written in Perl to get portable sub-second sleep. +# +# It can run the following steps based on arguments: +# -t : maximum # of seconds the script can run; else we fail. +# Default is 4 seconds. +# -e : echo to stdout +# -f : echo to stdout AND create an (empty) file named +# -w : wait for a file named to exist + +# Force flush +$| = 1; + +my $timeout = 4; + +sub op { + my ($op, $nm) = @_; + + defined $nm or die "Missing value for $op\n"; + + if ($op eq '-e') { + print "$nm\n"; + return 1; + } + + if ($op eq '-f') { + print "$nm\n"; + open(my $fh, '>', $nm) or die "$nm: open: $!\n"; + close(my $fh); + return 1; + } + + if ($op eq '-w') { + if (-f $nm) { + return 1; + } + select(undef, undef, undef, 0.1); + return 0; + } + + if ($op eq '-t') { + $timeout = $nm; + return 1; + } + + die("Invalid command: $op $nm\n"); +} + +my $start = time(); +while (@ARGV) { + if (op($ARGV[0], $ARGV[1])) { + shift; + shift; + } + if ($start + $timeout < time()) { + die("Timeout after ".(time()-$start-1)." seconds\n"); + } +} + +exit(0); diff -rupN a/tests/scripts/features/jobserver b/tests/scripts/features/jobserver --- a/tests/scripts/features/jobserver 2016-04-09 19:07:29.000000000 -0400 +++ b/tests/scripts/features/jobserver 2021-10-18 15:30:43.585608880 -0400 @@ -42,7 +42,7 @@ recurse: ; @echo $@: "/$(SHOW)/"; $(MAKE recurse2: ; @echo $@: "/$(SHOW)/"; $(MAKE) -f #MAKEFILE# all all:;@echo $@: "/$(SHOW)/" !, - "-j2 $np", "recurse: /-j2 --jobserver-auth= $np/\n#MAKE#[1]: warning: -jN forced in submake: disabling jobserver mode.\nrecurse2: /-j3 --jobserver-auth= $np/\nall: /-j3 --jobserver-auth= $np/\n"); + "-j2 $np", "recurse: /-j2 --jobserver-auth= $np/\n#MAKE#[1]: warning: -j3 forced in submake: resetting jobserver mode.\nrecurse2: /-j3 --jobserver-auth= $np/\nall: /-j3 --jobserver-auth= $np/\n"); delete $extraENV{MAKEFLAGS}; # Test override of -jN with -j @@ -52,7 +52,7 @@ recurse: ; @echo $@: "/$(SHOW)/"; $(MAKE recurse2: ; @echo $@: "/$(SHOW)/"; $(MAKE) -f #MAKEFILE# all all:;@echo $@: "/$(SHOW)/" !, - "-j2 $np", "recurse: /-j2 --jobserver-auth= $np/\n#MAKE#[1]: warning: -jN forced in submake: disabling jobserver mode.\nrecurse2: /-j $np/\nall: /-j $np/\n"); + "-j2 $np", "recurse: /-j2 --jobserver-auth= $np/\n#MAKE#[1]: warning: -j0 forced in submake: resetting jobserver mode.\nrecurse2: /-j $np/\nall: /-j $np/\n"); # Don't put --jobserver-auth into a re-exec'd MAKEFLAGS. # We can't test this directly because there's no way a makefile can @@ -76,7 +76,7 @@ inc.mk: # @echo 'MAKEFLAGS = $(MAKEFLAGS)' @echo 'FOO = bar' > $@ !, - "$np -j2", "#MAKE#[1]: warning: -jN forced in submake: disabling jobserver mode.\nall\n"); + "$np -j2", "#MAKE#[1]: warning: -j2 forced in submake: resetting jobserver mode.\nall\n"); unlink('inc.mk'); diff -rupN a/tests/scripts/features/output-sync b/tests/scripts/features/output-sync --- a/tests/scripts/features/output-sync 2016-04-23 10:09:35.000000000 -0400 +++ b/tests/scripts/features/output-sync 2021-10-18 15:31:40.903857757 -0400 @@ -45,7 +45,7 @@ sub output_sync_clean { # reliable. If things are too fast, then sometimes a different job will steal # the output sync lock and the output is mis-ordered from what we expect. sub output_sync_wait { - return "while [ ! -f ../mksync.$_[0] ]; do :; done; rm -f ../mksync.$_[0].wait; $sleep_command 1"; + return "while [ ! -f ../mksync.$_[0] ]; do :; done; $sleep_command 1"; } sub output_sync_set { return "date > ../mksync.$_[0]"; diff -rupN a/tests/scripts/features/parallelism b/tests/scripts/features/parallelism --- a/tests/scripts/features/parallelism 2016-04-11 07:50:44.000000000 -0400 +++ b/tests/scripts/features/parallelism 2021-10-18 17:12:39.005009030 -0400 @@ -1,17 +1,7 @@ # -*-perl-*- $description = "Test parallelism (-j) option."; - - -$details = "This test creates a makefile with two double-colon default -rules. The first rule has a series of sleep and echo commands -intended to run in series. The second and third have just an -echo statement. When make is called in this test, it is given -the -j option with a value of 4. This tells make that it may -start up to four jobs simultaneously. In this case, since the -first command is a sleep command, the output of the second -and third commands will appear before the first if indeed -make is running all of these commands in parallel."; +$details = ""; if (!$parallel_jobs) { return -1; @@ -24,13 +14,36 @@ else { $sleep_command = "sleep"; } +rmfiles(qw(ONE TWO THREE FOUR)); run_make_test(" all : def_1 def_2 def_3 -def_1 : ; \@echo ONE; $sleep_command 3 ; echo TWO -def_2 : ; \@$sleep_command 2 ; echo THREE -def_3 : ; \@$sleep_command 1 ; echo FOUR", +def_1 : ; \@#PERL# jhelp.pl -f ONE -w THREE -e TWO +def_2 : ; \@#PERL# jhelp.pl -w FOUR -f THREE +def_3 : ; \@#PERL# jhelp.pl -w ONE -f FOUR", '-j4', "ONE\nFOUR\nTHREE\nTWO"); +rmfiles(qw(ONE TWO THREE FOUR)); + +# Verify -j added to MAKEFLAGS in the makefile +run_make_test(" +MAKEFLAGS += -j4 +all : def_1 def_2 def_3 +def_1 : ; \@#PERL# jhelp.pl -f ONE -w THREE -e TWO +def_2 : ; \@#PERL# jhelp.pl -w FOUR -f THREE +def_3 : ; \@#PERL# jhelp.pl -w ONE -f FOUR", + '', "ONE\nFOUR\nTHREE\nTWO"); +rmfiles(qw(ONE TWO THREE FOUR)); + +# Command line should take precedence +rmfiles(qw(ONE TWO THREE FOUR)); +run_make_test(" +MAKEFLAGS += -j2 +all : def_1 def_2 def_3 +def_1 : ; \@#PERL# jhelp.pl -f ONE -w THREE -e TWO +def_2 : ; \@#PERL# jhelp.pl -w FOUR -f THREE +def_3 : ; \@#PERL# jhelp.pl -w ONE -f FOUR", + '-j4', "ONE\nFOUR\nTHREE\nTWO"); +rmfiles(qw(ONE TWO THREE FOUR)); # Test parallelism with included files. Here we sleep/echo while # building the included files, to test that they are being built in @@ -38,12 +51,12 @@ def_3 : ; \@$sleep_command 1 ; echo FOUR run_make_test(" all: 1 2; \@echo success -include 1.inc 2.inc -1.inc: ; \@echo ONE.inc; $sleep_command 2; echo TWO.inc; echo '1: ; \@echo ONE; $sleep_command 2; echo TWO' > \$\@ -2.inc: ; \@$sleep_command 1; echo THREE.inc; echo '2: ; \@$sleep_command 1; echo THREE' > \$\@", +1.inc: ; \@#PERL# jhelp.pl -f ONE.inc -w THREE.inc -f TWO.inc; echo '1: ; \@#PERL# jhelp.pl -f ONE -w THREE -f TWO' > \$\@ +2.inc: ; \@#PERL# jhelp.pl -w ONE.inc -f THREE.inc; echo '2: ; \@#PERL# jhelp.pl -w ONE -f THREE' > \$\@", "-j4", "ONE.inc\nTHREE.inc\nTWO.inc\nONE\nTHREE\nTWO\nsuccess\n", 0, 7); -rmfiles(qw(1.inc 2.inc)); +rmfiles(qw(ONE.inc TWO.inc THREE.inc ONE TWO THREE 1.inc 2.inc)); # Test parallelism with included files--this time recurse first and make @@ -57,12 +70,12 @@ ifeq (\$(INC),yes) -include 1.inc 2.inc endif -1.inc: ; \@echo ONE.inc; $sleep_command 2; echo TWO.inc; echo '1: ; \@echo ONE; $sleep_command 2; echo TWO' > \$\@ -2.inc: ; \@$sleep_command 1; echo THREE.inc; echo '2: ; \@$sleep_command 1; echo THREE' > \$\@", +1.inc: ; \@#PERL# jhelp.pl -f ONE.inc -w THREE.inc -f TWO.inc; echo '1: ; \@#PERL# jhelp.pl -f ONE -w THREE -f TWO' > \$\@ +2.inc: ; \@#PERL# jhelp.pl -w ONE.inc -f THREE.inc; echo '2: ; \@#PERL# jhelp.pl -w ONE -f THREE' > \$\@", "-j4", "ONE.inc\nTHREE.inc\nTWO.inc\nONE\nTHREE\nTWO\nsuccess\n", 0, 7); -rmfiles(qw(1.inc 2.inc)); +rmfiles(qw(ONE.inc TWO.inc THREE.inc ONE TWO THREE 1.inc 2.inc)); # Grant Taylor reports a problem where tokens can be lost (not written back # to the pipe when they should be): this happened when there is a $(shell ...) @@ -90,21 +103,23 @@ run_make_test(" .PHONY: all fail.1 fail.2 fail.3 ok all: fail.1 ok fail.2 fail.3 +.RECIPEPREFIX := > + fail.1 fail.2 fail.3: - \@$sleep_command \$(patsubst fail.%,%,\$\@) - \@echo Fail - \@exit 1 +> \@$sleep_command \$(patsubst fail.%,%,\$\@) +> \@echo Fail +> \@exit 1 ok: - \@$sleep_command 4 - \@echo Ok done", +> \@$sleep_command 4 +> \@echo Ok done", '-rR -j5', "Fail -#MAKE#: *** [#MAKEFILE#:8: fail.1] Error 1 +#MAKE#: *** [#MAKEFILE#:10: fail.1] Error 1 #MAKE#: *** Waiting for unfinished jobs.... Fail -#MAKE#: *** [#MAKEFILE#:8: fail.2] Error 1 +#MAKE#: *** [#MAKEFILE#:10: fail.2] Error 1 Fail -#MAKE#: *** [#MAKEFILE#:8: fail.3] Error 1 +#MAKE#: *** [#MAKEFILE#:10: fail.3] Error 1 Ok done", 512); @@ -117,13 +132,11 @@ all:; @: -include foo.d -foo.d: comp - @echo building $@ +foo.d: comp ; @echo building $@ comp: mod_a.o mod_b.o; @: -mod_a.o mod_b.o: - @exit 1 +mod_a.o mod_b.o: ; @exit 1 ', '-j2', ''); @@ -148,15 +161,15 @@ $extraENV{MAKEFLAGS} = '-j4'; run_make_test(q! things = thing1 thing2 all: $(things) -thing1:; @sleep 1; echo '$@ start'; sleep 2; echo '$@ end' -thing2:; @echo '$@ start'; sleep 2; echo '$@ end' +thing1:; @#PERL# jhelp.pl -w thing2start -f $@start -w thing2end -e $@end +thing2:; @#PERL# jhelp.pl -f $@start -w thing1start -f $@end -include inc.mk inc.mk: ; @touch $@ !, - '', "thing2 start\nthing1 start\nthing2 end\nthing1 end\n"); + '', "thing2start\nthing1start\nthing2end\nthing1end\n"); delete $extraENV{MAKEFLAGS}; -rmfiles('inc.mk'); +rmfiles(qw(inc.mk thing1start thing1end thing2start thing2end)); # Ensure intermediate/secondary files are not pruned incorrectly. # See Savannah bug #30653 @@ -211,7 +224,3 @@ rmfiles('file1', 'file2', 'file3', 'file # rmfiles(qw(dependfile output)); 1; - -### Local Variables: -### eval: (setq whitespace-action (delq 'auto-cleanup whitespace-action)) -### End: