[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#32634] [PATCH 2/2] ui: Add soft port for styling and filtering buil
[bug#32634] [PATCH 2/2] ui: Add soft port for styling and filtering build output.
Mon, 10 Sep 2018 15:17:22 +0200
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
Ricardo Wurmus <address@hidden> skribis:
> * guix/ui.scm (build-output-port): New procedure.
> * guix/scripts/package.scm (%default-options): Print build trace.
> (guix-package): Use build-output-port.
> * guix/scripts/build.scm (guix-build): Use build-output-port.
> Co-authored-by: Sahithi Yarlagadda <address@hidden>
Really nice to see this happen!
I’m late to the party but here are some comments:
> +(define* (build-output-port #:key
> + (colorize? #t)
> + verbose?
> + (port (current-error-port)))
> + "Return a soft port that processes build output. By default it colorizes
> +phase announcements and replaces any other output with a spinner."
> + (define spun? #f)
> + (define spin!
> + (let ((steps (circular-list "\\" "|" "/" "-")))
> + (lambda ()
> + (match steps
> + ((first . rest)
> + (set! steps rest)
> + (set! spun? #t) ; remember to erase spinner
> + first)))))
> + (define use-color?
> + (and colorize?
> + (not (or (getenv "NO_COLOR")
> + (getenv "INSIDE_EMACS")
> + (not (isatty? port))))))
I would rather do:
(define* (build-output-port #:key
(colorize? (not (or …))))
so that callers can still choose to turn it on and off.
Also, perhaps we can optimize things:
(if (and verbose? (not colorize?))
> + (define handle-string
> + (let ((rules `(("^(@ build-started) (.*) (.*)"
> + #:transform
> + ,(lambda (m)
> + (string-append
> + (colorize-string "Building " 'BLUE 'BOLD)
> + (match:substring m 2) "\n")))
It think we should precompile all the regexps with ‘make-regexp’ instead
of calling ‘string-match’ (which in turn calls ‘make-regexp’) for every
> + (make-soft-port
> + (vector
> + ;; procedure accepting one character for output
> + (cut write <> port)
> + ;; procedure accepting a string for output
> + handle-string
> + ;; thunk for flushing output
> + (lambda () (force-output port))
> + ;; thunk for getting one character
> + (const #t)
I think we should probably user ‘make-custom-binary-output-port’ instead
of ‘make-soft-port’ because it’s a cleaner and more faithful API
(‘make-soft-port’ no longer matches the operations implemented by port
The brings the issue of how to properly decode build output. Commit
ce72c780746776a86f59747f5eff8731cb4ff39b fixes issues in this area. I
wrote the tests below to see how ‘build-output-port’ behaves when passed
valid UTF-8 and garbage, and it appears to behave correctly, though the
question mark that we get in return when throwing garbage at it suggests
that decoding substitution is not happening where it should.
If we use a custom-binary-output-port, we’ll have to do something
similar to what ‘read-maybe-utf8-string’, and that way we’ll be fully in
> + ;; thunk for closing port (not by garbage collection)
> + (lambda () (close port)))
At call sites, we should probably do:
(build-output-port #:port (duplicate-port (current-error-port)))
instead of just:
so that stderr doesn’t get closed.
[bug#32634] RFC: Process build output, Nils Gillmann, 2018/09/08
[bug#32634] RFC: Process build output, Danny Milosavljevic, 2018/09/09
[bug#32634] RFC: Process build output, Ludovic Courtès, 2018/09/10