GNU bug report logs - #61970
[PATCH] lint: Return exit code 1 if there are warnings.

Previous Next

To reply to this bug, email your comments to 61970 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Antero Mejr <antero <at> mailbox.org>
To: guix-patches <at> gnu.org
Cc: Antero Mejr <antero <at> mailbox.org>
Subject: [PATCH] lint: Return exit code 1 if there are warnings.
Date: Sat,  4 Mar 2023 23:16:01 +0000

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

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

Message #8 received at 61970 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Antero Mejr <antero <at> mailbox.org>
Cc: 61970 <at> debbugs.gnu.org
Subject: Re: [bug#61970] [PATCH] lint: Return exit code 1 if there are
 warnings.
Date: Mon, 06 Mar 2023 16:59:20 +0100

Hi,

Antero Mejr <antero <at> 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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.