[bug#37510] [PATCH 1/1] compile: Fix race condition on completion progress.

[Eric Bavier <ericbavier@centurylink.net>]

----- On Sep 26, 2019, at 4:28 AM, Ludovic Courtès ludo@gnu.org wrote:

> Hello,
> 
> ericbavier@centurylink.net skribis:
> 
>> From: Eric Bavier <bavier@member.fsf.org>
>>
>> This prevent a race condition where multiple compilation threads could report
>> the same completion.
>>
>> * guix/build/compile.scm (compile-files)<completed>: Increment in same mutex
>> region as the compilation is reported.
>>
>>
>> Further reading:
>>
>> When compiling many scheme files, or with '-j1', this is not usually a
>> problem, but with multiple build jobs and a handful of scheme files to update,
>> you may encounter unexpected output.  E.g. I recently saw this from `make -j2`:
>>
>> ```
>> Compiling Scheme modules...
>> [ 25%] LOAD     gnu/packages/haskell.scm
>> ;;; note: source file ./gnu/packages/haskell.scm
>> ;;;       newer than compiled /home/bavier/projects/guix/gnu/packages/haskell.go
>> ;;; note: source file ./gnu/packages/haskell.scm
>> ;;;       newer than compiled /home/bavier/projects/guix/gnu/packages/haskell.go
>> [ 50%] LOAD     gnu/packages/idris.scm
>> ;;; note: source file ./gnu/packages/idris.scm
>> ;;;       newer than compiled /home/bavier/projects/guix/gnu/packages/idris.go
>> ;;; note: source file ./gnu/packages/idris.scm
>> ;;;       newer than compiled /home/bavier/projects/guix/gnu/packages/idris.go
>> [ 75%] GUILEC   gnu/packages/haskell.go
>> [ 75%] GUILEC   gnu/packages/idris.go
> 
> I think it’s expected: it shows completion at the time we started to
> build these files.  Compilation of haskell.scm and idris.scm started at
> the same time, and at that point we had built 75% of the files.  I agree
> it’s confusing though.  :-)

Right.  If we did indeed expect to see completion at the time we started, then I would expect to see "0%" displayed with the first "LOAD".

> 
>> +++ b/guix/build/compile.scm
>> @@ -173,7 +173,8 @@ files are for HOST, a GNU triplet such as
>> \"x86_64-linux-gnu\"."
>>  
>>    (define (build file)
>>      (with-mutex progress-lock
>> -      (report-compilation file total completed))
>> +      (report-compilation file total completed)
>> +      (set! completed (+ 1 completed)))
> 
> Here ‘completed’ is incremented before the thing is even started.

Maybe a more generic name like "progress" would be appropriate?

> 
> Anyway, LGTM!  :-)

Thanks for reviewing.

-- 
`~Eric