Subject: bug#28756: Substitute download progress bar doesn't reach 100%

From: [email-masked] (Ludovic Courtès)
Date: Sun, 15 Oct 2017 21:33:11 +0200

Hello,

[email-masked] (宋文武) skribis:

> [email-masked] (宋文武) writes:
>
>> [email-masked] (Ludovic Courtès) writes:
>>
>>> Hello,
>>>
>>> Leo Famulari <[email-masked]> skribis:
>>>
>>>> I just upgraded to the latest Guix and, while downloading substitutes, I
>>>> noticed the progress bar seems to never reach 100%, as shown below. I'm
>>>> not sure if this started recently or not.
>>>>
>>>> Downloading https://mirror.hydra.gnu.org/guix/nar/gzip/drm4pj1k5mkb5784i0rkqb0bg2z8lmyw-libabw-0.1.1...
>>>>  libabw-0.1.1  346KiB
>>>
>>> Indeed, I’ve noticed too, and I think it relates to the new progress
>>> reporters.  宋文武, do you experience this as well?  Thoughts?
>>>
>>
>> Yes, thanks for the report, commit abaee53c8 should fix it.
>>
>
> Well, it didn't...  I have to learn it's a child process will read and
> report the process:
>
> From 93b42f62ece1ad5181ed1119fc750bcbb74c5d3c Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?=E5=AE=8B=E6=96=87=E6=AD=A6?= <[email-masked]>
> Date: Sat, 14 Oct 2017 22:45:55 +0800
> Subject: [PATCH] guix: substitute: Report the last process in the child
>  process.
>
> Fixes <https://bugs.gnu.org/28756>.
>
> * guix/utils.scm (filtered-port): Close the 'input' port in the child process.
> * guix/scripts/substitute.scm (progress-substitution): Close the 'progress'
> port before 'restore-file'.

[...]

> +      ;; A child process of 'decompressed-port' will read from this 'process'
                                                                  ^^^^^^^^^^^^^
… will read from PROGRESS

> +      ;; port and thus report the actual progress to the console.  As the
> +      ;; parent process, we should close it at the start.
> +      (close-port progress)
>        ;; Unpack the Nar at INPUT into DESTINATION.
>        (restore-file input destination)
>        (close-port input)
> -      (close-port progress)
> +      (every (compose zero? cdr waitpid) pids)

Since INPUT is a wrapper around PROGRESS, it seems weird to close
PROGRESS beforehand.

Shouldn’t ‘filtered-port’ always close INPUT in the parent process (the
second part of the patch you sent closes it in the child process before
quitting, but we should additionally close it in the parent):

diff --git a/guix/utils.scm b/guix/utils.scm
index de4aa6531..2cf9be36d 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -153,9 +153,11 @@ buffered data is lost."
                   (close-port in)
                   (dump-port input out))
                 (lambda ()
+                  (close-port input)
                   (false-if-exception (close out))
                   (primitive-_exit 0))))
              (child
+              (close-port input)
               (close-port out)
               (loop in (cons child pids)))))))))

Thanks,
Ludo’.