Subject: [bug#61970] [PATCH] lint: Return exit code 1 if there are warnings.

From: Ludovic Courtès <[email-masked]>
Date: Mon, 06 Mar 2023 16:59:20 +0100

Hi,

Antero Mejr <[email-masked]> 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’.