diff mbox series

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

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

Commit Message

Antero Mejr March 4, 2023, 11:16 p.m. UTC
* guix/scripts/lint.scm (guix-lint, run-checkers): Modify procedure.
---
Exiting 1 makes it a lot easier to include a "guix lint" step in external
CI pipelines.

 guix/scripts/lint.scm | 60 ++++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 24 deletions(-)

Comments

Ludovic Courtès March 6, 2023, 3:59 p.m. UTC | #1
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 mbox series

Patch

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))))))