From 0c5a9f9b92af1634dc60fa21e9ac86ed50e5d595 Mon Sep 17 00:00:00 2001
From: Paul Smith <psmith@gnu.org>
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 <secs> : maximum # of seconds the script can run; else we fail.
+# Default is 4 seconds.
+# -e <word> : echo <word> to stdout
+# -f <word> : echo <word> to stdout AND create an (empty) file named <word>
+# -w <word> : wait for a file named <word> 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=<auth> $np/\n#MAKE#[1]: warning: -jN forced in submake: disabling jobserver mode.\nrecurse2: /-j3 --jobserver-auth=<auth> $np/\nall: /-j3 --jobserver-auth=<auth> $np/\n");
+ "-j2 $np", "recurse: /-j2 --jobserver-auth=<auth> $np/\n#MAKE#[1]: warning: -j3 forced in submake: resetting jobserver mode.\nrecurse2: /-j3 --jobserver-auth=<auth> $np/\nall: /-j3 --jobserver-auth=<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=<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=<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: