--- Begin Message ---
|
Subject: |
[PATCH] scripts: substitute: Improve fetch-narinfos progress reporting. |
|
Date: |
Wed, 9 Dec 2020 18:57:59 +0000 |
At least in guix weather, these changes make the progress bar actually appear.
* guix/scripts/substitute.scm (fetch-narinfos): Use (guix progress) for
progress reporting.
---
guix/scripts/substitute.scm | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm
index 25075eedff..5128310439 100755
--- a/guix/scripts/substitute.scm
+++ b/guix/scripts/substitute.scm
@@ -614,16 +614,8 @@ print a warning and return #f."
(define (fetch-narinfos url paths)
"Retrieve all the narinfos for PATHS from the cache at URL and return them."
- (define update-progress!
- (let ((done 0)
- (total (length paths)))
- (lambda ()
- (display "\r\x1b[K" (current-error-port)) ;erase current line
- (force-output (current-error-port))
- (format (current-error-port)
- (G_ "updating substitutes from '~a'... ~5,1f%")
- url (* 100. (/ done total)))
- (set! done (+ 1 done)))))
+ (define fetch-narinfos-progress-reporter
+ (progress-reporter/bar (length paths)))
(define hash-part->path
(let ((mapping (fold (lambda (path result)
@@ -641,7 +633,7 @@ print a warning and return #f."
(len (response-content-length response))
(cache (response-cache-control response))
(ttl (and cache (assoc-ref cache 'max-age))))
- (update-progress!)
+ (progress-reporter-report! fetch-narinfos-progress-reporter)
;; Make sure to read no more than LEN bytes since subsequent bytes may
;; belong to the next response.
@@ -673,7 +665,7 @@ print a warning and return #f."
(#f
'())
(port
- (update-progress!)
+ (start-progress-reporter! fetch-narinfos-progress-reporter)
;; Note: Do not check HTTPS server certificates to avoid depending
;; on the X.509 PKI. We can do it because we authenticate
;; narinfos, which provides a much stronger guarantee.
@@ -683,7 +675,7 @@ print a warning and return #f."
#:verify-certificate? #f
#:port port)))
(close-port port)
- (newline (current-error-port))
+ (stop-progress-reporter! fetch-narinfos-progress-reporter)
result)))))
((file #f)
(let* ((base (string-append (uri-path uri) "/"))
--
2.29.2
--- End Message ---
--- Begin Message ---
|
Subject: |
Re: [bug#45146] [PATCH] scripts: substitute: Improve fetch-narinfos progress reporting. |
|
Date: |
Tue, 09 Mar 2021 20:29:40 +0000 |
|
User-agent: |
mu4e 1.4.15; emacs 27.1 |
Christopher Baines <mail@cbaines.net> writes:
> Christopher Baines <mail@cbaines.net> writes:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Christopher Baines <mail@cbaines.net> skribis:
>>>
>>>> At least in guix weather, these changes make the progress bar actually
>>>> appear.
>>>>
>>>> * guix/scripts/substitute.scm (fetch-narinfos): Use (guix progress) for
>>>> progress reporting.
>>>
>>> Cool. I noticed that something was wrong with ‘guix weather’, but I
>>> suspected it had to do with the order in which the erase-line sequence
>>> and \r are sent.
>>>
>>>> - (lambda ()
>>>> - (display "\r\x1b[K" (current-error-port)) ;erase current line
>>>> - (force-output (current-error-port))
>>>> - (format (current-error-port)
>>>> - (G_ "updating substitutes from '~a'... ~5,1f%")
>>>> - url (* 100. (/ done total)))
>>>> - (set! done (+ 1 done)))))
>>>> + (define fetch-narinfos-progress-reporter
>>>> + (progress-reporter/bar (length paths)))
>>>
>>> The problem here is that we’d see a progress bar without knowing what it
>>> represents.
>>>
>>> Besides, currently output from ‘guix substitute’ is printed as is by
>>> client commands, regardless of whether stdout is a tty. The problem
>>> already exists but it would become a bit more visible as logs get filled
>>> with progress bars.
>>
>> Maybe it's best to circle back to fixing guix weather after trying to
>> restructure some of the guix substitute code.
>>
>> I've made an initial attempt at moving things around in [1]. If the
>> underlying code can live in a module, and then the substitute, weather
>> and challenge scripts use that code with whatever UI stuff they want,
>> maybe that will allow for better addressing this weather specific issue.
>>
>> 1: https://issues.guix.info/45409
>
> I've sent a couple of updated patches for this. They're not particularly
> dependent on the above work to create the (guix substitutes) module, but
> I based the commits on that.
>
> These commits make more careful changes, the substitute script behaviour
> should remain the same.
I've gone ahead and pushed these patches now as
f5ffb3bd9cd59cfff5b4625d6d65e8d116d0a25b.
signature.asc
Description: PGP signature
--- End Message ---