bug#27284: [PATCH 1/8] build: Factorize module compilation in (guix build compile).

[ludo@gnu.org (Ludovic Courtès)]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

Eric Bavier <ericbavier@centurylink.net> skribis:

> On Sun, 22 Oct 2017 19:51:14 -0700
> ludo@gnu.org (Ludovic Courtès) wrote:

[...]

>> > It Would Be Nice if the functionality worked for older versions of make
>> > that people might have on their systems.  
>> 
>> With the patch I posted, Scheme compilation would always use one thread
>> per core, which is what it currently does in ‘master’.

Oops, that was not quite true: it would use one thread.

I fixed it like this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2368 bytes --]

diff --git a/build-aux/compile-all.scm b/build-aux/compile-all.scm
index 4aa4ac9b9..c7ca5a6f6 100644
--- a/build-aux/compile-all.scm
+++ b/build-aux/compile-all.scm
@@ -19,6 +19,7 @@
 
 (use-modules (ice-9 match)
              (ice-9 threads)
+             (srfi srfi-1)
              (guix build compile)
              (guix build utils))
 
@@ -50,24 +51,32 @@ to 'make'."
   (match flags
     (#f (current-processor-count))
     (flags
-     (let loop ((flags (string-tokenize flags)))
-       (match flags
-         (()
-          1)
-         (("-j" (= string->number count) _ ...)
-          (if (integer? count)
-              count
-              (current-processor-count)))
-         ((head tail ...)
-          (if (string-prefix? "-j" head)
-              (match (string-drop head 2)
-                (""
-                 (current-processor-count))
-                ((= string->number count)
-                 (if (integer? count)
-                     count
-                     (current-processor-count))))
-              (loop tail))))))))
+     (let ((initial-flags (string-tokenize flags)))
+       (let loop ((flags initial-flags))
+         (match flags
+           (()
+            ;; Note: GNU make prior to version 4.2 would hide "-j" flags from
+            ;; $MAKEFLAGS.  Thus, check for a "--jobserver" flag here and
+            ;; assume we're using all cores if specified.
+            (if (any (lambda (flag)
+                       (string-prefix? "--jobserver" flag))
+                     initial-flags)
+                (current-processor-count)         ;GNU make < 4.2
+                1))                               ;sequential make
+           (("-j" (= string->number count) _ ...)
+            (if (integer? count)
+                count
+                (current-processor-count)))
+           ((head tail ...)
+            (if (string-prefix? "-j" head)
+                (match (string-drop head 2)
+                  (""
+                   (current-processor-count))
+                  ((= string->number count)
+                   (if (integer? count)
+                       count
+                       (current-processor-count))))
+                (loop tail)))))))))
 
 ;; Install a SIGINT handler to give unwind handlers in 'compile-file' an
 ;; opportunity to run upon SIGINT and to remove temporary output files.

[-- Attachment #3: Type: text/plain, Size: 1182 bytes --]


>> > Using the jobserver directly would require quite a bit of work for the
>> > current patch set, but I wonder if there is another way to determine
>> > the -jN parameter for make<4.2 that we could use.  Maybe simply
>> > polling the jobserver fds at the start?  
>> 
>> AIUI the job server does not reveal how many jobs are allowed.  It
>> merely grants you an execution token.
>> 
>> Or did you have something else in mind?
>
> The idea (hardely tested) would be to read tokens from the input fd
> until it blocks, do the scheme compiles with however many tokens were
> read, then write them back out.  Crude, I guess, and probably error
> prone; compile-all.scm could be invoked when make has job tokens tied
> up building the daemon source...
>
> Anyhow, the current patch works well for me with a recent make.  I'm
> content leaving further improvements to a future hypothetical hacker. :)

Sounds reasonable.  Let’s make sure the hypothetical hacker has enough
on their plate.  :-)

I’ve pushed this and the easy parts of this patch series, with commit
d298c815e638581d466222f3a883b280f019b368 as the tip.

Thanks for the review!

Ludo’.