[bug#71697] [PATCH v3 2/2] scripts: lint: Honor package property to exclude checkers.

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

Hi Simon,

Simon Tournier <zimon.toutoune@gmail.com> writes:

> * guix/scripts/lint.scm (exclude-package-checkers): New procedure, filter the
> checker if the package is marked.
> (guix-lint)[show-package-checkers]: New procedure.
> * doc/guix.texi: Document it.
>
> Change-Id: Idf8e5c67102a1701ebd917bbc6212cfeb6ea2054
> ---
>  doc/guix.texi         | 17 ++++++++++++++++-
>  guix/scripts/lint.scm | 26 +++++++++++++++++++++++++-
>  2 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 037b1a2f24..1baf3fafe6 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -71,7 +71,7 @@
>  Copyright @copyright{} 2019 Alex Griffin@*
>  Copyright @copyright{} 2019, 2020, 2021, 2022 Guillaume Le Vaillant@*
>  Copyright @copyright{} 2020 Liliana Marie Prikler@*
> -Copyright @copyright{} 2019, 2020, 2021, 2022, 2023 Simon Tournier@*
> +Copyright @copyright{} 2019, 2020, 2021, 2022, 2023, 2024 Simon Tournier@*
>  Copyright @copyright{} 2020 Wiktor Żelazny@*
>  Copyright @copyright{} 2020 Damien Cassou@*
>  Copyright @copyright{} 2020 Jakub Kądziołka@*
> @@ -15444,6 +15444,21 @@ Invoking guix lint
>  to the new style.
>  @end table
>
> +Sometimes it is not desired to run the same checker each time
> +@command{guix lint} is invoked---e.g., because the checker takes time or
> +to avoid to send again and again the same request for archiving.

The rationale sounds odd in the context of creating Guix packages for
Guix -- I wouldn't want someone to start adding random lint exclusions
to package properties because some check "takes time".  I think it'd be
better to give as an example which problem the mechanism was created
for, which is, to opt out of the Software Heritage archival requests.

From there the text could mention that the mechanism is general can be
used to disable other lint checks as well, such as the home page check.

> +Instead of excluding the checker at the command-line via the option
> +@code{--exclude}, the package might be marked to skip the checker by
> +honoring the property in package definition, e.g.,
> +
> +@lisp
> +(package
> +  (name "python-scikit-learn")
> +  ;; @dots{}
> +  (properties '((lint-exclude-archival? . #t)
> +                (lint-exclude-home-page? . #t))))
> +@end lisp
> +
>  The general syntax is:
>
>  @example
> diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
> index b98266c831..7aed467eae 100644
> --- a/guix/scripts/lint.scm
> +++ b/guix/scripts/lint.scm
> @@ -9,7 +9,7 @@
>  ;;; Copyright © 2017 Tobias Geerinckx-Rice <me@tobias.gr>
>  ;;; Copyright © 2017, 2018 Efraim Flashner <efraim@flashner.co.il>
>  ;;; Copyright © 2018, 2019 Arun Isaac <arunisaac@systemreboot.net>
> -;;; Copyright © 2019, 2020 Simon Tournier <zimon.toutoune@gmail.com>
> +;;; Copyright © 2019, 2020, 2024 Simon Tournier <zimon.toutoune@gmail.com>
>  ;;; Copyright © 2020 Brice Waegeneire <brice@waegenei.re>
>  ;;;
>  ;;; This file is part of GNU Guix.
> @@ -39,6 +39,7 @@ (define-module (guix scripts lint)
>    #:use-module (ice-9 format)
>    #:use-module (srfi srfi-1)
>    #:use-module (srfi srfi-37)
> +  #:use-module (srfi srfi-26)
>    #:export (guix-lint
>              run-checkers))
>
> @@ -59,6 +60,18 @@ (define (emit-warnings warnings)
>                 name version message))))
>     warnings))
>
> +(define (exclude-package-checkers package checkers)
> +  "Filter the CHECKERS list using PACKAGE properties field."
> +  (let ((properties (package-properties package)))
> +    (filter (lambda (checker)
> +              (not (assq-ref properties
> +                             ((compose string->symbol
> +                                       (cut string-append "lint-exclude-" <> "?")
> +                                       symbol->string
> +                                       lint-checker-name)
> +                              checker))))
> +            checkers)))

Instead of using filter + a negated test, I'd use 'remove' (from SRFI
1).

>  (define* (run-checkers package checkers #:key store)
>    "Run the given CHECKERS on PACKAGE."
>    (let ((tty? (isatty? (current-error-port))))
> @@ -223,16 +236,27 @@ (define-command (guix-lint . args)
>                  (proc store))
>                (proc #f)))
>
> +        (define (show-package-checkers package checkers)
> +          (format (current-error-port) "~a@~a checked by~{ ~a~}.~%"
> +                  (package-name package)
> +                  (package-version package)
> +                  (sort (map (compose symbol->string lint-checker-name)
> +                             (exclude-package-checkers
> +                              package checkers))
> +                   string<?)))
> +
>          (call-maybe-with-store
>           (lambda (store)
>             (cond
>              ((null? args)
>               (fold-packages (lambda (p r)
> +                              (show-package-checkers p checkers)
>                                (when (not (assoc-ref opts 'dry-run?))
>                                  (run-checkers p checkers
>                                                #:store store))) '()))
>              (else
>               (for-each (lambda (package)
> +                         (show-package-checkers package checkers)
>                           (when (not (assoc-ref opts 'dry-run?))
>                               (run-checkers package checkers
>                                             #:store store)))

I haven't tried it, but this looks reasonable to me.

-- 
Thanks,
Maxim