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: