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

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

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.

Hi,

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

> +@lisp
> +(package
> +  (name "python-scikit-learn")
> +  ;; @dots{}
> +  (properties '((lint-exclude-archival? . #t)
> +                (lint-exclude-home-page? . #t))))

To complement Maxim’s review, how about:

  (properties '((lint-excluded-checkers . (archival home-page))))

?

Apart from that, the idea sounds reasonable to me.

Thanks,
Ludo’.

On Tue, Jun 25, 2024 at 11:15 AM Ludovic Courtès <ludo@gnu.org> wrote:
>
> Hi,
>
> Simon Tournier <zimon.toutoune@gmail.com> skribis:
>
> > +@lisp
> > +(package
> > +  (name "python-scikit-learn")
> > +  ;; @dots{}
> > +  (properties '((lint-exclude-archival? . #t)
> > +                (lint-exclude-home-page? . #t))))
>
> To complement Maxim’s review, how about:
>
>   (properties '((lint-excluded-checkers . (archival home-page))))
>
> ?
>
> Apart from that, the idea sounds reasonable to me.
>
> Thanks,
> Ludo’.

Could we not instead create a GUIX_LINT_OPTIONS, similar to
GUIX_BUILD_OPTIONS? Then anyone wishing to universally exclude certain
checkers (or disable network checks) on their own system would be free
to do so.

I find the current implementation confusing since I don't believe the
project would accept a new or modified package missing the home page
or with archiving disabled. Stated another way, to which Guix packages
are we adding lint exclusions?

Greg

Greg Hogan <code@greghogan.com> writes:

> I find the current implementation confusing since I don't believe the
> project would accept a new or modified package missing the home page
> or with archiving disabled. Stated another way, to which Guix packages
> are we adding lint exclusions?

To packages in your own channel.

Hi Greg,

Greg Hogan <code@greghogan.com> writes:

> On Tue, Jun 25, 2024 at 11:15 AM Ludovic Courtès <ludo@gnu.org> wrote:
>>
>> Hi,
>>
>> Simon Tournier <zimon.toutoune@gmail.com> skribis:
>>
>> > +@lisp
>> > +(package
>> > +  (name "python-scikit-learn")
>> > +  ;; @dots{}
>> > +  (properties '((lint-exclude-archival? . #t)
>> > +                (lint-exclude-home-page? . #t))))
>>
>> To complement Maxim’s review, how about:
>>
>>   (properties '((lint-excluded-checkers . (archival home-page))))
>>
>> ?
>>
>> Apart from that, the idea sounds reasonable to me.
>>
>> Thanks,
>> Ludo’.
>
> Could we not instead create a GUIX_LINT_OPTIONS, similar to
> GUIX_BUILD_OPTIONS? Then anyone wishing to universally exclude certain
> checkers (or disable network checks) on their own system would be free
> to do so.

That would be a good option to have too, on top of the other one.

> I find the current implementation confusing since I don't believe the
> project would accept a new or modified package missing the home page
> or with archiving disabled. Stated another way, to which Guix packages
> are we adding lint exclusions?

I don't think these exclusions should be committed in general to the
repo, except when we have for example the author of some software
explicitly requesting that SWH archival be disabled for it in Guix.

It may also be useful e.g. for some project that really don't have a
home page, to avoid a spurious lint warning in this case.

On Wed, Jun 26, 2024 at 3:28 PM Maxim Cournoyer
<maxim.cournoyer@gmail.com> wrote:
>
> I don't think these exclusions should be committed in general to the
> repo, except when we have for example the author of some software
> explicitly requesting that SWH archival be disabled for it in Guix.

Author requests are as problematic to a free software distribution as
the earlier demands to modify historical data are to reproducibility.

How do we authenticate authorship? Is it a single author, all authors,
majority of authorship? How would the latter be measured and valued?
Are author requests transitive? In which direction? Do the requests
propagate to dependent packages, or must a request include author
approval from all project dependencies? How do we handle cases where
copyright has not been noted as carefully as in Guix? Must the request
be made specifically to the Guix project? How do we monitor projects
for new authors or changes to requests?

We have a system for honoring author requests that resolves every
single one of these issues: software licenses. And this is not some
new issue, developers have been debating commercial use ("Micro$oft")
of their work for decades, yet here we are writing free software and
building a free Gnu/OS.

These requests to turn free software non-free are simply the tip of
the iceberg. We have always considered the artist (author) to be
separate from the art (licensed software). Now we get (from the
initiator of these demands) that "Not every political opinion should
be respected." which is a clear contradiction of the Guix Code of
Conduct's "Being respectful of differing opinions, viewpoints, and
experiences". Which individuals or demographic subgroups will be next
claimed problematic and need to have their contributions excluded?

> It may also be useful e.g. for some project that really don't have a
> home page, to avoid a spurious lint warning in this case.

If this is the best use case for a spurious feature request then I
find this a dangerous addition to the project. Those denigrading and
demanding that Guix pressure partner projects to restrict the use of
free software are unlikely to be content adding these flags to their
private packages as may exist.