Message ID | 20230304231601.13352-1-antero@mailbox.org |
---|---|
State | New |
Headers | show |
Series | [bug#61970] lint: Return exit code 1 if there are warnings. | expand |
Hi, Antero Mejr <antero@mailbox.org> skribis: > * guix/scripts/lint.scm (guix-lint, run-checkers): Modify procedure. Please expound a little bit. :-) Could you also add a sentence under “Invoking guix lint” in the manual? > Exiting 1 makes it a lot easier to include a "guix lint" step in external > CI pipelines. Yeah, though some lint warnings are more critical than others, and often they’re just warnings, which is why ‘guix lint’ always returned zero so far. > (define* (run-checkers package checkers #:key store) > "Run the given CHECKERS on PACKAGE." > - (let ((tty? (isatty? (current-error-port)))) > - (for-each (lambda (checker) > - (when tty? > - (format (current-error-port) "checking ~a@~a [~a]...\x1b[K\r" > - (package-name package) (package-version package) > - (lint-checker-name checker)) > - (force-output (current-error-port))) > - (emit-warnings > - (if (lint-checker-requires-store? checker) > - ((lint-checker-check checker) package #:store store) > - ((lint-checker-check checker) package)))) > - checkers) > + (let* ((tty? (isatty? (current-error-port))) > + (results > + (map (lambda (checker) > + (when tty? > + (format (current-error-port) "checking ~a@~a [~a]...\x1b[K\r" > + (package-name package) (package-version package) > + (lint-checker-name checker)) > + (force-output (current-error-port))) > + (let ((results (if (lint-checker-requires-store? checker) > + ((lint-checker-check checker) package > + #:store store) > + ((lint-checker-check checker) package)))) > + (emit-warnings results) > + results)) > + checkers))) For clarity I would separate warning collection from warning printing. So: (let ((tty? …) (warnings (append-map (lambda (checker) …) checkers))) (for-each (lambda (warning) …) warnings) (null? warnings)) ;return #t when WARNINGS is empty > + (define (null?-rec lst) > + (if (list? lst) > + (not (member #f (map null?-rec lst))) > + #f)) > + > + (if (null?-rec > + (call-maybe-with-store > + (lambda (store) > + (cond > + ((null? args) > + (fold-packages (lambda (p r) > + (cons (run-checkers p checkers > + #:store store) r)) '())) > + (else > + (map (lambda (package) > + (run-checkers package checkers #:store store)) > + args)))))) > + (exit 0) > + (exit 1)))))) I’d suggest something similar here. Could you send an updated patch? Thanks, Ludo’.
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm index 9920c3ee62..a4bec357c7 100644 --- a/guix/scripts/lint.scm +++ b/guix/scripts/lint.scm @@ -61,21 +61,25 @@ (define (emit-warnings warnings) (define* (run-checkers package checkers #:key store) "Run the given CHECKERS on PACKAGE." - (let ((tty? (isatty? (current-error-port)))) - (for-each (lambda (checker) - (when tty? - (format (current-error-port) "checking ~a@~a [~a]...\x1b[K\r" - (package-name package) (package-version package) - (lint-checker-name checker)) - (force-output (current-error-port))) - (emit-warnings - (if (lint-checker-requires-store? checker) - ((lint-checker-check checker) package #:store store) - ((lint-checker-check checker) package)))) - checkers) + (let* ((tty? (isatty? (current-error-port))) + (results + (map (lambda (checker) + (when tty? + (format (current-error-port) "checking ~a@~a [~a]...\x1b[K\r" + (package-name package) (package-version package) + (lint-checker-name checker)) + (force-output (current-error-port))) + (let ((results (if (lint-checker-requires-store? checker) + ((lint-checker-check checker) package + #:store store) + ((lint-checker-check checker) package)))) + (emit-warnings results) + results)) + checkers))) (when tty? (format (current-error-port) "\x1b[K") - (force-output (current-error-port))))) + (force-output (current-error-port))) + results)) (define (list-checkers-and-exit checkers) ;; Print information about all available checkers and exit. @@ -218,14 +222,22 @@ (define (call-maybe-with-store proc) (proc store)) (proc #f))) - (call-maybe-with-store - (lambda (store) - (cond - ((null? args) - (fold-packages (lambda (p r) (run-checkers p checkers - #:store store)) '())) - (else - (for-each (lambda (package) - (run-checkers package checkers - #:store store)) - args))))))))) + (define (null?-rec lst) + (if (list? lst) + (not (member #f (map null?-rec lst))) + #f)) + + (if (null?-rec + (call-maybe-with-store + (lambda (store) + (cond + ((null? args) + (fold-packages (lambda (p r) + (cons (run-checkers p checkers + #:store store) r)) '())) + (else + (map (lambda (package) + (run-checkers package checkers #:store store)) + args)))))) + (exit 0) + (exit 1))))))