[PATCH] lint: Return exit code 1 if there are warnings.

  • Open
  • quality assurance status badge
Details
2 participants
  • Antero Mejr
  • Ludovic Courtès
Owner
unassigned
Submitted by
Antero Mejr
Severity
normal
A
A
Antero Mejr wrote on 5 Mar 2023 00:16
(address . guix-patches@gnu.org)(name . Antero Mejr)(address . antero@mailbox.org)
20230304231601.13352-1-antero@mailbox.org
* 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(-)

Toggle diff (79 lines)
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
L
L
Ludovic Courtès wrote on 6 Mar 2023 16:59
(name . Antero Mejr)(address . antero@mailbox.org)(address . 61970@debbugs.gnu.org)
87cz5lan1j.fsf@gnu.org
Hi,

Antero Mejr <antero@mailbox.org> skribis:

Toggle quote (2 lines)
> * 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?

Toggle quote (3 lines)
> 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.

Toggle quote (30 lines)
> (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

Toggle quote (20 lines)
> + (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’.
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 61970@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 61970
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch