[bug#71697] [PATCH] guix: lint: Honor 'no-archival?' package property.

[bug#71697] [PATCH] guix: lint: Honor 'no-archival?' package property.

[Simon Tournier]

* guix/lint.scm (check-archival): Skip the checker if the package is marked.
* doc/guix.texi: Document it.

Change-Id: I2e21b60ee4f02255f298740a2e9ebb1717e490ff
---
 doc/guix.texi |  15 ++++-
 guix/lint.scm | 154 ++++++++++++++++++++++++++------------------------
 2 files changed, 93 insertions(+), 76 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 769ca1399f..5c1cb89686 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@*
@@ -15380,6 +15380,19 @@ Invoking guix lint
 prints a message and the @code{archival} checker stops doing anything until
 that limit has been reset.
 
+Sometimes it is not desired to send a request for archiving each time
+@command{guix lint} is run.  The package might be marked to skip the
+@code{archival} checker by honoring the @code{no-archival?} property in
+package definition:
+
+@lisp
+(define-public python-scikit-learn
+  (package
+    (name "python-scikit-learn")
+    ;; @dots{}
+    (properties '((no-archival? . #t)))))
+@end lisp
+
 @item cve
 @cindex security vulnerabilities
 @cindex CVE, Common Vulnerabilities and Exposures
diff --git a/guix/lint.scm b/guix/lint.scm
index 68d532968d..4c33ec6598 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -1717,84 +1717,88 @@ (define (check-archival package)
     (lookup-directory-by-nar-hash (content-hash-value hash)
                                   (content-hash-algorithm hash)))
 
-  (parameterize ((%allow-request? skip-when-limit-reached))
-    (catch #t
-      (lambda ()
-        (match (package-source package)
-          (#f                                     ;no source
-           '())
-          ((and (? origin? origin)
-                (= origin-uri (? git-reference? reference)))
-           (define url
-             (git-reference-url reference))
-           (define commit
-             (git-reference-commit reference))
-           (define hash
-             (origin-hash origin))
-
-           (match (or (lookup-by-nar-hash hash)
-                      (if (commit-id? commit)
-                          (or (lookup-revision commit)
-                              (lookup-origin-revision url commit))
-                          (lookup-origin-revision url commit)))
-             ((or (? string?) (? revision?))
-              '())
-             (#f
-              ;; Revision is missing from the archive, attempt to save it.
-              (save-package-source package))))
-          ((? origin? origin)
-           (if (and=> (origin-hash origin)          ;XXX: for ungoogled-chromium
-                      content-hash-value)           ;& icecat
-               (let ((hash (origin-hash origin)))
-                 (match (or (lookup-by-nar-hash hash)
-                            (lookup-content (content-hash-value hash)
-                                            (symbol->string
-                                             (content-hash-algorithm hash))))
-                   (#f
-                    ;; If ORIGIN is a version-control checkout, save it now.
-                    ;; If not, check whether HASH is in the Disarchive
-                    ;; database ("Save Code Now" does not accept tarballs).
-                    (if (vcs-origin origin)
-                        (save-package-source package)
-                        (match (lookup-disarchive-spec hash)
-                          (#f
-                           (list (make-warning package
-                                               (G_ "source not archived on Software \
+  (if (not (assq 'no-archival? (package-properties package)))
+    (parameterize ((%allow-request? skip-when-limit-reached))
+      (catch #t
+        (lambda ()
+          (match (package-source package)
+            (#f                                     ;no source
+             '())
+            ((and (? origin? origin)
+                  (= origin-uri (? git-reference? reference)))
+             (define url
+               (git-reference-url reference))
+             (define commit
+               (git-reference-commit reference))
+             (define hash
+               (origin-hash origin))
+
+             (match (or (lookup-by-nar-hash hash)
+                        (if (commit-id? commit)
+                            (or (lookup-revision commit)
+                                (lookup-origin-revision url commit))
+                            (lookup-origin-revision url commit)))
+               ((or (? string?) (? revision?))
+                '())
+               (#f
+                ;; Revision is missing from the archive, attempt to save it.
+                (save-package-source package))))
+            ((? origin? origin)
+             (if (and=> (origin-hash origin)          ;XXX: for ungoogled-chromium
+                        content-hash-value)           ;& icecat
+                 (let ((hash (origin-hash origin)))
+                   (match (or (lookup-by-nar-hash hash)
+                              (lookup-content (content-hash-value hash)
+                                              (symbol->string
+                                               (content-hash-algorithm hash))))
+                     (#f
+                      ;; If ORIGIN is a version-control checkout, save it now.
+                      ;; If not, check whether HASH is in the Disarchive
+                      ;; database ("Save Code Now" does not accept tarballs).
+                      (if (vcs-origin origin)
+                          (save-package-source package)
+                          (match (lookup-disarchive-spec hash)
+                            (#f
+                             (list (make-warning package
+                                                 (G_ "source not archived on Software \
 Heritage and missing from the Disarchive database")
-                                               #:field 'source)))
-                          (directory-ids
-                           (match (find (lambda (id)
-                                          (not (lookup-directory id)))
-                                        directory-ids)
-                             (#f '())
-                             (id
-                              (list (make-warning package
-                                                  (G_ "\
+                                                 #:field 'source)))
+                            (directory-ids
+                             (match (find (lambda (id)
+                                            (not (lookup-directory id)))
+                                          directory-ids)
+                               (#f '())
+                               (id
+                                (list (make-warning package
+                                                    (G_ "\
 Disarchive entry refers to non-existent SWH directory '~a'")
-                                                  (list id)
-                                                  #:field 'source))))))))
-                   ((? content?)
-                    '())
-                   ((? string? swhid)
-                    '())))
-               '()))
-          ((? local-file?)
-           '())
-          (_
-           (list (make-warning package
-                               (G_ "\
+                                                    (list id)
+                                                    #:field 'source))))))))
+                     ((? content?)
+                      '())
+                     ((? string? swhid)
+                      '())))
+                 '()))
+            ((? local-file?)
+             '())
+            (_
+             (list (make-warning package
+                                 (G_ "\
 source is not an origin, it cannot be archived")
-                               #:field 'source)))))
-      (match-lambda*
-        (('swh-error url method response)
-         (swh-response->warning package url method response))
-        ((key . args)
-         (if (eq? key skip-key)
-             '()
-             (with-networking-fail-safe
-              (G_ "while connecting to Software Heritage")
-              '()
-              (apply throw key args))))))))
+                                 #:field 'source)))))
+        (match-lambda*
+          (('swh-error url method response)
+           (swh-response->warning package url method response))
+          ((key . args)
+           (if (eq? key skip-key)
+               '()
+               (with-networking-fail-safe
+                (G_ "while connecting to Software Heritage")
+                '()
+                (apply throw key args)))))))
+    (list
+     (make-warning package
+                   (G_ "skip archiving as marked by package")))))
 
 (define (check-haskell-stackage package)
   "Check whether PACKAGE is a Haskell package ahead of the current

base-commit: bc8a41f4a8d9f1f0525d7bc97c67ed3c8aea3111
-- 
2.41.0

[bug#71697] [PATCH v2] guix: scripts: lint: Honor package property to exclude chercker.

[Simon Tournier]

* guix/scripts/lint.scm (run-checkers): Skip the checker if the package is
marked.
* doc/guix.texi: Document it.

Change-Id: Idf8e5c67102a1701ebd917bbc6212cfeb6ea2054
---
 doc/guix.texi         | 17 ++++++++++++++++-
 guix/scripts/lint.scm | 17 +++++++++++++++--
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 769ca1399f..46a4079c4b 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.
+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 '((no-archival . #t)
+                (no-name . #t))))
+@end lisp
+
 The general syntax is:
 
 @example
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index ee3de51fb1..d8bac277a0 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))
 
@@ -61,6 +62,18 @@ (define (emit-warnings warnings)
 
 (define* (run-checkers package checkers #:key store)
   "Run the given CHECKERS on PACKAGE."
+  (define (checkers* checkers)
+    (let ((properties (package-properties package)))
+      (filter (lambda (checker)
+                (any  (lambda (p)
+                        (eq? p ((compose string->symbol
+                                         (cut string-append "no-" <>)
+                                         symbol->string
+                                         lint-checker-name)
+                                checker)))
+                      properties))
+              checkers)))
+
   (let ((tty? (isatty? (current-error-port))))
     (for-each (lambda (checker)
                 (when tty?
@@ -72,7 +85,7 @@ (define* (run-checkers package checkers #:key store)
                  (if (lint-checker-requires-store? checker)
                      ((lint-checker-check checker) package #:store store)
                      ((lint-checker-check checker) package))))
-              checkers)
+              (checkers* checkers))
     (when tty?
       (format (current-error-port) "\x1b[K")
       (force-output (current-error-port)))))

base-commit: bc8a41f4a8d9f1f0525d7bc97c67ed3c8aea3111
-- 
2.41.0

[bug#71697] [PATCH v2] guix: scripts: lint: Honor package property to exclude chercker.

[Liliana Marie Prikler]

Am Freitag, dem 21.06.2024 um 20:33 +0200 schrieb Simon Tournier:
> +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.
> +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 '((no-archival . #t)
> +                (no-name . #t))))
> +@end lisp
> +
Maybe we should future-proof this by calling them "lint-exclude-CHECK".
While a generic "no-CHECK" sounds great, at least no-name might confuse
first readers :)

Cheers

[bug#71697] [PATCH v2] guix: scripts: lint: Honor package property to exclude chercker.

[MSavoritias]

On Fri, 21 Jun 2024 20:33:28 +0200
Simon Tournier <zimon.toutoune@gmail.com> wrote:

> * guix/scripts/lint.scm (run-checkers): Skip the checker if the package is
> marked.
> * doc/guix.texi: Document it.
> 
> Change-Id: Idf8e5c67102a1701ebd917bbc6212cfeb6ea2054
> ---
>  doc/guix.texi         | 17 ++++++++++++++++-
>  guix/scripts/lint.scm | 17 +++++++++++++++--
>  2 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 769ca1399f..46a4079c4b 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.
> +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 '((no-archival . #t)
> +                (no-name . #t))))
> +@end lisp
> +
>  The general syntax is:
>  
>  @example
> diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
> index ee3de51fb1..d8bac277a0 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))
>  
> @@ -61,6 +62,18 @@ (define (emit-warnings warnings)
>  
>  (define* (run-checkers package checkers #:key store)
>    "Run the given CHECKERS on PACKAGE."
> +  (define (checkers* checkers)
> +    (let ((properties (package-properties package)))
> +      (filter (lambda (checker)
> +                (any  (lambda (p)
> +                        (eq? p ((compose string->symbol
> +                                         (cut string-append "no-" <>)
> +                                         symbol->string
> +                                         lint-checker-name)
> +                                checker)))
> +                      properties))
> +              checkers)))
> +
>    (let ((tty? (isatty? (current-error-port))))
>      (for-each (lambda (checker)
>                  (when tty?
> @@ -72,7 +85,7 @@ (define* (run-checkers package checkers #:key store)
>                   (if (lint-checker-requires-store? checker)
>                       ((lint-checker-check checker) package #:store store)
>                       ((lint-checker-check checker) package))))
> -              checkers)
> +              (checkers* checkers))
>      (when tty?
>        (format (current-error-port) "\x1b[K")
>        (force-output (current-error-port)))))
> 
> base-commit: bc8a41f4a8d9f1f0525d7bc97c67ed3c8aea3111

Why not make this opt-in instead and have it `enable-archiving`?

Because as it currently stands this patch doesn't account for:
- people who dont have channels but run guix lint
- people who may not read the manual and have their code sent to SWH even tho they didnt intent it

Guix should strive to do what it is explicitly asked. nothing more. which is lint in this case, not archiving.

MSavoritias

[bug#71697] [PATCH v3 1/2] scripts: lint: Add 'dry-run' option.

[Simon Tournier]

* guix/scripts/lint.scm (show-help, %options): Add 'dry-run' option.
(guix-lint): Use it.
* doc/guix.texi: Document it.

Change-Id: I8c96e376d52c0961ccf2ab39f1fc856c762b089d
---
 doc/guix.texi         |  3 +++
 guix/scripts/lint.scm | 16 ++++++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 769ca1399f..037b1a2f24 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -15459,6 +15459,9 @@ Invoking guix lint
 List and describe all the available checkers that will be run on packages
 and exit.
 
+@item --dry-run
+Do not run the checkers.
+
 @item --checkers
 @itemx -c
 Only enable the checkers specified in a comma-separated list using the
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index ee3de51fb1..b98266c831 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -100,6 +100,8 @@ (define (show-help)
   (display (G_ "Usage: guix lint [OPTION]... [PACKAGE]...
 Run a set of checkers on the specified package; if none is specified,
 run the checkers on all packages.\n"))
+  (display (G_ "
+      --dry-run          do not run checkers "))
   (display (G_ "
   -c, --checkers=CHECKER1,CHECKER2...
                          only run the specified checkers"))
@@ -154,6 +156,9 @@ (define %options
         (option '(#\n "no-network") #f #f
                 (lambda (opt name arg result)
                   (alist-cons 'no-network? #t result)))
+        (option '("dry-run") #f #f
+                (lambda (opt name arg result)
+                  (alist-cons 'dry-run? #t result)))
         (find (lambda (option)
                 (member "load-path" (option-names option)))
               %standard-build-options)
@@ -222,10 +227,13 @@ (define-command (guix-lint . args)
          (lambda (store)
            (cond
             ((null? args)
-             (fold-packages (lambda (p r) (run-checkers p checkers
-                                                        #:store store)) '()))
+             (fold-packages (lambda (p r)
+                              (when (not (assoc-ref opts 'dry-run?))
+                                (run-checkers p checkers
+                                              #:store store))) '()))
             (else
              (for-each (lambda (package)
-                         (run-checkers package checkers
-                                       #:store store))
+                         (when (not (assoc-ref opts 'dry-run?))
+                             (run-checkers package checkers
+                                           #:store store)))
                        args)))))))))

base-commit: bc8a41f4a8d9f1f0525d7bc97c67ed3c8aea3111
-- 
2.41.0

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

[Simon Tournier]

* 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.
+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)))
+
 (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)))
-- 
2.41.0

[bug#71697] [PATCH v2] guix: scripts: lint: Honor package property to exclude chercker.

[Simon Tournier]

Hi,

On Sat, 22 Jun 2024 at 17:29, MSavoritias <email@msavoritias.me> wrote:

> Because as it currently stands this patch doesn't account for:
> - people who dont have channels but run guix lint

You misread: there is no channel involved.

Considering this patch, if an user does not want to run *any* checker
for whatever reason, then this user has at hand two means:

 + guix lint --exclude
 + rely on the ’properties’ field directly in package definition


> - people who may not read the manual and have their code sent to SWH even tho they didnt intent it

Considering this patch, all the checkers for each package are clearly
displayed.

And this patch adds also the option ’--dry-run’.  Therefore, if one does
not want to read the manual, that’s a good mitigation.

Cheers,
simon

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

[Maxim Cournoyer]

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

[bug#71697] [PATCH v3 1/2] scripts: lint: Add 'dry-run' option.

[Maxim Cournoyer]

Hi,

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

> * guix/scripts/lint.scm (show-help, %options): Add 'dry-run' option.
> (guix-lint): Use it.
> * doc/guix.texi: Document it.
>
> Change-Id: I8c96e376d52c0961ccf2ab39f1fc856c762b089d
> ---
>  doc/guix.texi         |  3 +++
>  guix/scripts/lint.scm | 16 ++++++++++++----
>  2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 769ca1399f..037b1a2f24 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -15459,6 +15459,9 @@ Invoking guix lint
>  List and describe all the available checkers that will be run on packages
>  and exit.
>  
> +@item --dry-run
> +Do not run the checkers.

Does it print which checkers would run?  Otherwise I don't see the
usefulness.

-- 
Thanks,
Maxim

[bug#71697] [PATCH v2] guix: scripts: lint: Honor package property to exclude chercker.

[MSavoritias]

On Sat, 22 Jun 2024 17:40:40 +0200
Simon Tournier <zimon.toutoune@gmail.com> wrote:

> Hi,
> 
> On Sat, 22 Jun 2024 at 17:29, MSavoritias <email@msavoritias.me> wrote:
> 
> > Because as it currently stands this patch doesn't account for:
> > - people who dont have channels but run guix lint  
> 
> You misread: there is no channel involved.
> 
> Considering this patch, if an user does not want to run *any* checker
> for whatever reason, then this user has at hand two means:
> 
>  + guix lint --exclude
>  + rely on the ’properties’ field directly in package definition
> 
> 
> > - people who may not read the manual and have their code sent to SWH even tho they didnt intent it  
> 
> Considering this patch, all the checkers for each package are clearly
> displayed.
> 
> And this patch adds also the option ’--dry-run’.  Therefore, if one does
> not want to read the manual, that’s a good mitigation.
> 
> Cheers,
> simon


Ah okay. That sounds like a good first step then.

MSavoritias

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

[Ludovic Courtès]

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’.

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

[Greg Hogan via Guix-patches]

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

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

[Ricardo Wurmus]

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.

-- 
Ricardo

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

[Maxim Cournoyer]

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.

-- 
Thanks,
Maxim

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

[Greg Hogan]

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.

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

[Maxim Cournoyer]

Hi Greg,

Greg Hogan <code@greghogan.com> writes:

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

You raise good questions, for which I do not have immediate answers.

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

While I dislike the attitude/approach used, I think the essence of the
complaint was that Guix, via SHW, was somehow facilitating the
scavenging of free software sources to train large language models
(LLM), with the opinion that these models do not respect the licenses of
the sources ingested for their produced output (the work is considered
new work, not a derived work, or perhaps it's still legally a gray area,
I don't know).  In this perspective, the original poster was seeking to
have the free software more protected against what they see as a loop
hole in the LLM business, as explained above.

That's an interesting legal and moral challenge/problem, but I don't
think GNU Guix is the right venue to debate it; especially not in the
way it's been attempted here.

-- 
Thanks,
Maxim

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

[Dale Mellor]

On Fri, 2024-06-28 at 23:12 -0400, Maxim Cournoyer wrote:
> 
> While I dislike the attitude/approach used, I think the essence of the
> complaint was that Guix, via SHW, was somehow facilitating the
> scavenging of free software sources to train large language models
> (LLM), with the opinion that these models do not respect the licenses of
> the sources ingested for their produced output (the work is considered
> new work, not a derived work, or perhaps it's still legally a gray area,
> I don't know).  In this perspective, the original poster was seeking to
> have the free software more protected against what they see as a loop
> hole in the LLM business, as explained above.

  Original, original poster here (I'm feeling pretty awkward right now TBH, like
a bad shit-stirrer).  The point is that I use GUIX to support my own, private
projects.  It is nothing to do with licensing, I'm the only one who has ever
seen the code.  In this context it is unacceptable that GUIX should give it away
to anyone.

Dale

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

[Maxim Cournoyer]

Hi Dale,

Dale Mellor <guix-devel-0brg6a@rdmp.org> writes:

> On Fri, 2024-06-28 at 23:12 -0400, Maxim Cournoyer wrote:
>> 
>> While I dislike the attitude/approach used, I think the essence of the
>> complaint was that Guix, via SHW, was somehow facilitating the
>> scavenging of free software sources to train large language models
>> (LLM), with the opinion that these models do not respect the licenses of
>> the sources ingested for their produced output (the work is considered
>> new work, not a derived work, or perhaps it's still legally a gray area,
>> I don't know).  In this perspective, the original poster was seeking to
>> have the free software more protected against what they see as a loop
>> hole in the LLM business, as explained above.
>
>   Original, original poster here (I'm feeling pretty awkward right now TBH, like
> a bad shit-stirrer).  The point is that I use GUIX to support my own, private
> projects.  It is nothing to do with licensing, I'm the only one who has ever
> seen the code.  In this context it is unacceptable that GUIX should give it away
> to anyone.

OK. From my understanding, the code is not transferred; only an archival
request to the project URL is submitted to SHW, and its up to SHW to
attempt to retrieve it (which would fail if the URL is private/protected
by some means).

Perhaps we could have a dummy 'private' or 'non-free' license added to
(guix licenses), that the 'check-archival' procedure would check to skip
the archival request?  This would need to be documented (mentioning it's
not for use for packages carried in the Guix collection, but for end
users working on a software not meant to be distributed).

-- 
Thanks,
Maxim

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

[Maxim Cournoyer]

Hi Dale,

Dale Mellor <guix-devel-0brg6a@rdmp.org> writes:

> On Mon, 2024-07-01 at 16:44 -0400, Maxim Cournoyer wrote:
>> 
>> OK. From my understanding, the code is not transferred; only an archival
>> request to the project URL is submitted to SHW, and its up to SHW to
>> attempt to retrieve it (which would fail if the URL is private/protected
>> by some means).
>> 
>> Perhaps we could have a dummy 'private' or 'non-free' license added to
>> (guix licenses), that the 'check-archival' procedure would check to skip
>> the archival request?  This would need to be documented (mentioning it's
>> not for use for packages carried in the Guix collection, but for end
>> users working on a software not meant to be distributed).
>> 
>
>   You have now brought the discussion full circle; you can pick it up at
> https://lists.gnu.org/archive/html/guix-devel/2024-06/msg00225.html.

Thanks, sorry for missing it.  I like the replies from Ricardo there.

-- 
Cheers,
Maxim

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

[Ludovic Courtès]

Greg Hogan <code@greghogan.com> skribis:

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

+1

I think this would be a slippery slope.  Free software is meant to be
freely redistributable, unconditionally.

Ludo’.

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

[Simon Tournier]

Hi Dale,

On Sun, 30 Jun 2024 at 15:48, Dale Mellor <guix-devel-0brg6a@rdmp.org> wrote:

>   Original, original poster here (I'm feeling pretty awkward right now TBH, like
> a bad shit-stirrer).  The point is that I use GUIX to support my own, private
> projects.  It is nothing to do with licensing, I'm the only one who has ever
> seen the code.  In this context it is unacceptable that GUIX should give it away
> to anyone.

Well, from my understanding, the general case for packages included in
Guix proper is to run all checkers.

Then, my willing with this patch submission is to address side projects,
as you are pointing.  The first answer is: it’s not a problem because
“guix lint” offers the option “--exclude”.  However, as you mentioned,
it’s easy to forget and… bang.  Hence, the patch set as mitigation for
public side projects using Guix:

 + Have a --dry-run option
 + Exclude lint checkers at the package definition level
 + Still have the option --exclude option

I think, having these 3 features addresses the case of public side
projects.

Cheers,
simon

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

[Simon Tournier]

Hi Greg,

On Thu, 27 Jun 2024 at 12:38, Greg Hogan <code@greghogan.com> wrote:

> If this is the best use case for a spurious feature request then I
> find this a dangerous addition to the project.

Sorry, I do not see the danger.  What I see is the same policy for the
project – nothing is changed – and the patch set provides an helper for
third-party channels outside the project.

When developing or maintaining a third-party channel outside the
project, one might systematically run:

    guix lint -L . -x refresh,github-urls foobar

because of some reasons of ’foobar’.  I do not see where it is dangerous
to also have the alternative to configure this exclusion at the package
level definition.

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

About pressure, I will not rehash here what had been said at length
elsewhere. :-)  Let me clarify about “restrict”.

For sure, I agree that by definition of free software, one cannot
restrict its usage.  The key point here seems between a right and an
obligation.  One has the right to modify and/or share but no obligation;
it’s still free software. :-)

Therefore, if one uses Guix to develop packages, it’s up to them to
decide how they want to share their developments on free software.
However, we have the right to use these developments how we want –
limited by what the license allows.

All in all, I do not see the danger. :-)

Cheers,
simon

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

[Simon Tournier]

Hi,

On Wed, 26 Jun 2024 at 15:28, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

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

Well, I am not convinced it would be helpful.  Because if you have:

    GUIX_LINT_EXCLUDE=archival,home-page guix lint -L . foobar

is as complicated as:

    guix lint -L . -x archival,home-page foobar

And if ones does:

    export GUIX_LINT_EXCLUDE=archival

then the checker ’archival’ would be always excluded, i.e., for the
third-party custom packages – that’s what the aim :-) – but also when
the person would contribute to Guix proper – and that’s against our
quality assurance, IMHO.

Somehow, from my point of view, the idea of the patch set is only to
provide a complementary mechanism of “--exclude”.  Consider that I am
packaging something for Guix proper and I am bored by some checker
because it takes time, or because the warning annoys me or because it
sends again and again the exact same request to SWH or because whatever
other reasons, then I can just temporarily turn off that checker when
developing and looping over “guix lint”, either via --exclude or either
via the package property.  Once the package is ready, I submit it with
all checkers turned on.

Obviously, a third-party channel could use this mechanism to turn off
the checkers.  It’s up to them. :-)

Well, maybe the wording of the manual could be tweaked if it does not
capture this idea.

Cheers,
simon

[bug#71697] [PATCH v4 1/2] scripts: lint: Add 'dry-run' option.

[Simon Tournier]

* guix/scripts/lint.scm (guix-lint)[show-package-checkers]: New procedure.
(show-help, %options): Add 'dry-run' option.
* doc/guix.texi: Document it.

Change-Id: I8c96e376d52c0961ccf2ab39f1fc856c762b089d
---
 doc/guix.texi         |  3 +++
 guix/scripts/lint.scm | 26 ++++++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 5b77c84b4a..6043962038 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -15463,6 +15463,9 @@ Invoking guix lint
 List and describe all the available checkers that will be run on packages
 and exit.
 
+@item --dry-run
+Do not run the checkers.
+
 @item --checkers
 @itemx -c
 Only enable the checkers specified in a comma-separated list using the
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index ee3de51fb1..1b13d6e17f 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -100,6 +100,8 @@ (define (show-help)
   (display (G_ "Usage: guix lint [OPTION]... [PACKAGE]...
 Run a set of checkers on the specified package; if none is specified,
 run the checkers on all packages.\n"))
+  (display (G_ "
+      --dry-run          do not run checkers "))
   (display (G_ "
   -c, --checkers=CHECKER1,CHECKER2...
                          only run the specified checkers"))
@@ -154,6 +156,9 @@ (define %options
         (option '(#\n "no-network") #f #f
                 (lambda (opt name arg result)
                   (alist-cons 'no-network? #t result)))
+        (option '("dry-run") #f #f
+                (lambda (opt name arg result)
+                  (alist-cons 'dry-run? #t result)))
         (find (lambda (option)
                 (member "load-path" (option-names option)))
               %standard-build-options)
@@ -218,14 +223,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)
+                             checkers)
+                   string<?)))
+
         (call-maybe-with-store
          (lambda (store)
            (cond
             ((null? args)
-             (fold-packages (lambda (p r) (run-checkers p checkers
-                                                        #:store store)) '()))
+             (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)
-                         (run-checkers package checkers
-                                       #:store store))
+                         (show-package-checkers package checkers)
+                         (when (not (assoc-ref opts 'dry-run?))
+                             (run-checkers package checkers
+                                           #:store store)))
                        args)))))))))

base-commit: 2d6a3799fcda5c017f653c6e96b91964b07a7ee0
-- 
2.41.0

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

[Simon Tournier]

* guix/scripts/lint.scm (exclude-package-checkers): New procedure, remove the
checker if the package is marked.
(run-checkers, guix-lint): Use it.
* doc/guix.texi: Document 'lint-excluded-checkers' package property.

Change-Id: Idf8e5c67102a1701ebd917bbc6212cfeb6ea2054
---
 doc/guix.texi         | 16 +++++++++++++++-
 guix/scripts/lint.scm | 17 ++++++++++++++---
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 6043962038..0558532077 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@*
@@ -15448,6 +15448,20 @@ Invoking guix lint
 to the new style.
 @end table
 
+When developing a package, 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.  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-excluded-checkers . (archival home-page)))))
+@end lisp
+
 The general syntax is:
 
 @example
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 1b13d6e17f..ca1864b459 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.
@@ -59,6 +59,15 @@ (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))
+         (excluded-checkers (or (assq-ref properties 'lint-excluded-checkers)
+                                '())))
+    (remove (lambda (checker)
+              (member (lint-checker-name checker) excluded-checkers))
+            checkers)))
+
 (define* (run-checkers package checkers #:key store)
   "Run the given CHECKERS on PACKAGE."
   (let ((tty? (isatty? (current-error-port))))
@@ -72,7 +81,8 @@ (define* (run-checkers package checkers #:key store)
                  (if (lint-checker-requires-store? checker)
                      ((lint-checker-check checker) package #:store store)
                      ((lint-checker-check checker) package))))
-              checkers)
+              (exclude-package-checkers
+               package checkers))
     (when tty?
       (format (current-error-port) "\x1b[K")
       (force-output (current-error-port)))))
@@ -228,7 +238,8 @@ (define-command (guix-lint . args)
                   (package-name package)
                   (package-version package)
                   (sort (map (compose symbol->string lint-checker-name)
-                             checkers)
+                             (exclude-package-checkers
+                              package checkers))
                    string<?)))
 
         (call-maybe-with-store
-- 
2.41.0

[bug#71697] [PATCH v4 1/2] scripts: lint: Add 'dry-run' option.

[Ludovic Courtès]

Hi,

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

> +             (fold-packages (lambda (p r)
> +                              (show-package-checkers p checkers)
> +                              (when (not (assoc-ref opts 'dry-run?))
> +                                (run-checkers p checkers
> +                                              #:store store))) '()))

I’d call ‘show-package-checkers’ only for dry runs (like it would print
“the following checkers would run: …”).  That said, I find ‘--dry-run’
in this case not so useful because it would just print the same thing on
and on for each and every package.

Also, s/when not/unless/.

Ludo’.

[bug#71697] [PATCH v4 1/2] scripts: lint: Add 'dry-run' option.

[Simon Tournier]

Hi,

On Thu, 18 Jul 2024 at 11:19, Ludovic Courtès <ludo@gnu.org> wrote:

>                                          That said, I find ‘--dry-run’
> in this case not so useful because it would just print the same thing on
> and on for each and every package.

No, it’s different; or can be.  For instance, ’archival’ is listed for
one but not for the other.

--8<---------------cut here---------------start------------->8---
$ git diff
diff --git a/gnu/packages/machine-learning.scm b/gnu/packages/machine-learning.scm
index 3680cab044..5a22f7a865 100644
--- a/gnu/packages/machine-learning.scm
+++ b/gnu/packages/machine-learning.scm
@@ -1775,6 +1775,7 @@ (define-public python-scikit-learn
            python-pytest-xdist))
     (propagated-inputs
      (list python-numpy python-threadpoolctl python-scipy python-joblib))
+    (properties '((lint-excluded-checkers . (archival name synopsis))))
     (home-page "https://scikit-learn.org/")
     (synopsis "Machine Learning in Python")
     (description

$ ./pre-inst-env guix lint hello python-scikit-learn --dry-run
hello@2.12.1 checked by archival compiler-for-target cve derivation description formatting github-url gnu-description haskell-stackage home-page input-labels inputs-should-be-native inputs-should-not-be-input license mirror-url name optional-tests patch-file-names patch-headers profile-collisions refresh source source-file-name source-unstable-tarball synopsis tests-true wrapper-inputs.
python-scikit-learn@1.4.2 checked by compiler-for-target cve derivation description formatting github-url gnu-description haskell-stackage home-page input-labels inputs-should-be-native inputs-should-not-be-input license mirror-url optional-tests patch-file-names patch-headers profile-collisions refresh source source-file-name source-unstable-tarball tests-true wrapper-inputs.
--8<---------------cut here---------------end--------------->8---

Well, the aim is to know which checker applies to which package just
before they are indeed applied.

Initially, I thought something like:

--8<---------------cut here---------------start------------->8---
modified   guix/scripts/lint.scm
@@ -223,6 +223,11 @@ (define-command (guix-lint . args)
     (when (assoc-ref opts 'list?)
       (list-checkers-and-exit checkers))
 
+    (when (assoc-ref opts 'dry-run?)
+      (if (null? args)
+          (show-checkers-and-exit)
+          (show-packages-checkers-and-exit args checkers)))
+
     (with-error-handling
       (let ((any-lint-checker-requires-store?
              (any lint-checker-requires-store? checkers)))
--8<---------------cut here---------------end--------------->8---

where ’show-checkers-and-exit’ takes into account only option ’-c’ or
option ’-x’, displaying something as: “The following checkers would run
for all <number of packages> packages:” and the list of checkers.

And ’show-packages-checkers-and-exit’ would display:

--8<---------------cut here---------------start------------->8---
The following checkers would run for packages: <"short“ "list of package@X.Y>

 + list of all common checkers

The following checkers would be excluded for packages: <"short“ "list of package@X.Y>

 + list of excluded checkers
--8<---------------cut here---------------end--------------->8---

But then I concluded that would not be a true dry-run. ;-)


For the comments, thanks!  That’s better.

Cheers,
simon

[bug#71697] [PATCH v5 0/3] Add dry-run to guix lint

[Simon Tournier]

Hi,

Examples are probably better than many words. :-)

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix lint --dry-run
guix lint: error: too much information to display, did nothing and exit

$ ./pre-inst-env guix lint --dry-run hello
gnu/packages/base.scm:94:2: hello@2.12.1: all the 27 checkers would run.

$ ./pre-inst-env guix lint --dry-run hello -c derivation
gnu/packages/base.scm:94:2: hello@2.12.1: 1/27 checkers would run: derivation.

$ ./pre-inst-env guix lint --dry-run hello -n
gnu/packages/base.scm:94:2: hello@2.12.1: 18/27 checkers would run at the exclusion of: archival cve github-url gnu-description haskell-stackage home-page refresh source synopsis.

$ ./pre-inst-env guix lint --dry-run hello -c derivation -x derivation
gnu/packages/base.scm:94:2: hello@2.12.1: none of 27 checkers would run

$ # Using modified package with the new package properties
$ ./pre-inst-env guix lint --dry-run hello python-scikit-learn -x cve 
gnu/packages/base.scm:94:2: hello@2.12.1: 26/27 checkers would run at the exclusion of: cve.
gnu/packages/machine-learning.scm:1723:2: python-scikit-learn@1.4.2: 23/27 checkers would run at the exclusion of: archival cve name synopsis.
--8<---------------cut here---------------end--------------->8---

I think it addresses all the comments.  The idea is to display the most
relevant information, i.e., the message displays the shortest list of checkers
between the excluded ones and the others, because, IMHO, it eases to get what
would run.

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix lint --dry-run hello -c name,tests-true,compiler-for-target,description,inputs-should-be-native,inputs-should-not-be-input,input-labels,wrapper-inputs,license,optional-tests,mirror-url,source-file-name,source-unstable-tarball,profile-collisions,patch-file-names,patch-headers,formatting,synopsis,gnu-description,home-page,source,github-url,cve,refresh,archival,haskell-stackage
gnu/packages/base.scm:94:2: hello@2.12.1: 26/27 checkers would run at the exclusion of: derivation.
--8<---------------cut here---------------end--------------->8---


Well, since I did some typo when testing, the last patch of the series is
included for the same price. ;-)

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix lint -c dervation
guix lint: error: dervation: invalid checker
hint: Did you mean `derivation'?
--8<---------------cut here---------------end--------------->8---


WDYT?

Cheers,
simon

--8<---------------cut here---------------start------------->8---
diff --git a/gnu/packages/machine-learning.scm b/gnu/packages/machine-learning.scm
index 34dc5c8d26..cbebc6494e 100644
--- a/gnu/packages/machine-learning.scm
+++ b/gnu/packages/machine-learning.scm
@@ -1775,6 +1775,7 @@ (define-public python-scikit-learn
            python-pytest-xdist))
     (propagated-inputs
      (list python-numpy python-threadpoolctl python-scipy python-joblib))
+    (properties '((lint-excluded-checkers . (archival name synopsis))))
     (home-page "https://scikit-learn.org/")
     (synopsis "Machine Learning in Python")
     (description
--8<---------------cut here---------------end--------------->8---


Simon Tournier (3):
  scripts: lint: Add 'dry-run' option.
  scripts: lint: Honor package property to exclude checkers.
  scripts: lint: Add hint for checker typo.

 doc/guix.texi         | 19 +++++++++-
 guix/scripts/lint.scm | 81 +++++++++++++++++++++++++++++++++++++++----
 2 files changed, 92 insertions(+), 8 deletions(-)


base-commit: 9724e61cda80e4c59a2eb419a453887ecc551b9a
-- 
2.41.0

[bug#71697] [PATCH v5 1/3] scripts: lint: Add 'dry-run' option.

[Simon Tournier]

* guix/scripts/lint.scm (guix-lint)[show-package-checkers]: New procedure.
(show-help, %options): Add 'dry-run' option.
* doc/guix.texi: Document it.

Change-Id: I8c96e376d52c0961ccf2ab39f1fc856c762b089d
---
 doc/guix.texi         |  3 +++
 guix/scripts/lint.scm | 54 +++++++++++++++++++++++++++++++++++++++----
 2 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 5b77c84b4a..6043962038 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -15463,6 +15463,9 @@ Invoking guix lint
 List and describe all the available checkers that will be run on packages
 and exit.
 
+@item --dry-run
+Do not run the checkers.
+
 @item --checkers
 @itemx -c
 Only enable the checkers specified in a comma-separated list using the
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index ee3de51fb1..10abb05cf0 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -100,6 +100,8 @@ (define (show-help)
   (display (G_ "Usage: guix lint [OPTION]... [PACKAGE]...
 Run a set of checkers on the specified package; if none is specified,
 run the checkers on all packages.\n"))
+  (display (G_ "
+      --dry-run          do not run checkers "))
   (display (G_ "
   -c, --checkers=CHECKER1,CHECKER2...
                          only run the specified checkers"))
@@ -154,6 +156,9 @@ (define %options
         (option '(#\n "no-network") #f #f
                 (lambda (opt name arg result)
                   (alist-cons 'no-network? #t result)))
+        (option '("dry-run") #f #f
+                (lambda (opt name arg result)
+                  (alist-cons 'dry-run? #t result)))
         (find (lambda (option)
                 (member "load-path" (option-names option)))
               %standard-build-options)
@@ -218,14 +223,55 @@ (define-command (guix-lint . args)
                 (proc store))
               (proc #f)))
 
+        (define (show-package-checkers package checkers)
+          (let* ((name (package-name package))
+                 (version (package-version package))
+                 (loc (package-location package))
+                 (number-checkers (length checkers))
+                 (number-all-checkers (length %all-checkers)))
+            (cond
+             ((= number-all-checkers number-checkers)
+              (info loc (G_ "~a@~a: all the ~d checkers would run.~%")
+                    name version
+                    number-all-checkers))
+             ((= 0 number-checkers)
+              (info loc (G_ "~a@~a: none of ~d checkers would run~%")
+                    name version
+                    number-all-checkers))
+             (else
+              (let* ((excluded-checkers (lset-difference eq? %all-checkers checkers))
+                     (number-excluded-checkers (length excluded-checkers))
+                     (number-difference (- number-all-checkers number-excluded-checkers))
+                     (sorter (lambda (list-checkers)
+                               (sort (map (compose symbol->string lint-checker-name)
+                                          list-checkers)
+                                     string<?))))
+                (info loc (G_ "~a@~a: ~d/~d checkers would run")
+                      name version
+                      number-checkers
+                      number-all-checkers)
+                (if (< number-excluded-checkers number-difference)
+                    (format (current-error-port)
+                            (G_ " at the exclusion of:~{ ~a~}.~%")
+                            (sorter excluded-checkers))
+                    (format (current-error-port)
+                            ":~{ ~a~}.~%"
+                            (sorter checkers))))))))
+
         (call-maybe-with-store
          (lambda (store)
            (cond
             ((null? args)
-             (fold-packages (lambda (p r) (run-checkers p checkers
-                                                        #:store store)) '()))
+             (when (assoc-ref opts 'dry-run?)
+               (leave (G_ "too much information to display, did nothing and exit~%") ""))
+             (fold-packages (lambda (p r)
+                              (unless (assoc-ref opts 'dry-run?)
+                                  (run-checkers p checkers
+                                                #:store store))) '()))
             (else
              (for-each (lambda (package)
-                         (run-checkers package checkers
-                                       #:store store))
+                         (if (assoc-ref opts 'dry-run?)
+                             (show-package-checkers package checkers)
+                             (run-checkers package checkers
+                                           #:store store)))
                        args)))))))))
-- 
2.41.0

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

[Simon Tournier]

* guix/scripts/lint.scm (exclude-package-checkers): New procedure, remove the
checker if the package is marked.
(run-checkers, guix-lint): Use it.
* doc/guix.texi: Document 'lint-excluded-checkers' package property.

Change-Id: Idf8e5c67102a1701ebd917bbc6212cfeb6ea2054
---
 doc/guix.texi         | 16 +++++++++++++++-
 guix/scripts/lint.scm | 16 ++++++++++++++--
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 6043962038..0558532077 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@*
@@ -15448,6 +15448,20 @@ Invoking guix lint
 to the new style.
 @end table
 
+When developing a package, 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.  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-excluded-checkers . (archival home-page)))))
+@end lisp
+
 The general syntax is:
 
 @example
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 10abb05cf0..2df0cba948 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.
@@ -59,6 +59,15 @@ (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))
+         (excluded-checkers (or (assq-ref properties 'lint-excluded-checkers)
+                                '())))
+    (remove (lambda (checker)
+              (member (lint-checker-name checker) excluded-checkers))
+            checkers)))
+
 (define* (run-checkers package checkers #:key store)
   "Run the given CHECKERS on PACKAGE."
   (let ((tty? (isatty? (current-error-port))))
@@ -72,7 +81,8 @@ (define* (run-checkers package checkers #:key store)
                  (if (lint-checker-requires-store? checker)
                      ((lint-checker-check checker) package #:store store)
                      ((lint-checker-check checker) package))))
-              checkers)
+              (exclude-package-checkers
+               package checkers))
     (when tty?
       (format (current-error-port) "\x1b[K")
       (force-output (current-error-port)))))
@@ -227,6 +237,8 @@ (define-command (guix-lint . args)
           (let* ((name (package-name package))
                  (version (package-version package))
                  (loc (package-location package))
+                 (checkers (exclude-package-checkers
+                            package checkers))
                  (number-checkers (length checkers))
                  (number-all-checkers (length %all-checkers)))
             (cond
-- 
2.41.0

[bug#71697] [PATCH v5 3/3] scripts: lint: Add hint for checker typo.

[Simon Tournier]

* guix/scripts/lint.scm (option-checker): Add hint for checker typo.

Change-Id: I432c5b0570d2f413b59c2296666f3a4e5fb8c64c
---
 guix/scripts/lint.scm | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 2df0cba948..f4aa394686 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -31,6 +31,7 @@ (define-module (guix scripts lint)
   #:use-module (guix packages)
   #:use-module (guix lint)
   #:use-module (guix ui)
+  #:use-module ((guix utils) #:select (string-closest))
   #:use-module (guix store)
   #:use-module (guix scripts)
   #:use-module (guix scripts build)
@@ -147,7 +148,15 @@ (define (option-checker short-long)
                                                  ((short long) long)))))
               (for-each (lambda (c)
                           (unless (memq c checker-names)
-                            (leave (G_ "~a: invalid checker~%") c)))
+                            (let* ((name (symbol->string c))
+                                   (checkers (map symbol->string checker-names))
+                                   (hint (string-closest name checkers
+                                                         #:threshold 3)))
+                              (report-error (G_ "~a: invalid checker~%") name)
+                              (when hint
+                                (display-hint
+                                 (format #f (G_ "Did you mean @code{~a}?~%") hint)))
+                              (exit 1))))
                         names)
               (alist-cons option-name
                           (filter (lambda (checker)
-- 
2.41.0

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

[Greg Hogan]

On Fri, Jul 12, 2024 at 1:22 PM Simon Tournier <zimon.toutoune@gmail.com> wrote:
>
> Hi Greg,
>
> On Thu, 27 Jun 2024 at 12:38, Greg Hogan <code@greghogan.com> wrote:
>
> > If this is the best use case for a spurious feature request then I
> > find this a dangerous addition to the project.
>
> Sorry, I do not see the danger.  What I see is the same policy for the
> project – nothing is changed – and the patch set provides an helper for
> third-party channels outside the project.

Please allow me to preface my response by thanking you for your
contributions to the project, and to thank all of the many illustrious
Guix contributors in this thread. There is certainly value in this
feature request and my only care is for consideration of the
implementation.

> When developing or maintaining a third-party channel outside the
> project, one might systematically run:
>
>     guix lint -L . -x refresh,github-urls foobar
>
> because of some reasons of ’foobar’.  I do not see where it is dangerous
> to also have the alternative to configure this exclusion at the package
> level definition.

In addition to adding GUIX_LINT_OPTIONS (modeled on
GUIX_BUILD_OPTIONS) could we extend the exclusions to allow
package-specific definitions as with package transformations? For
example, GUIX_LINT_OPTIONS="--exclude=archival,home-page=mypackage",
which would disable archival for all packages but the home-page check
only for "mypackage".

> >                                                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.
>
> About pressure, I will not rehash here what had been said at length
> elsewhere. :-)  Let me clarify about “restrict”.
>
> For sure, I agree that by definition of free software, one cannot
> restrict its usage.  The key point here seems between a right and an
> obligation.  One has the right to modify and/or share but no obligation;
> it’s still free software. :-)

IANAL but I do not believe this to be the case. The LICENSE does not
apply to or restrict the developer's use of the software, only the
recipient. If the software has not been distributed then it cannot be
considered free software.

> Therefore, if one uses Guix to develop packages, it’s up to them to
> decide how they want to share their developments on free software.
> However, we have the right to use these developments how we want –
> limited by what the license allows.

Guix is restricted by the GNU FSDG
[https://www.gnu.org/distros/free-system-distribution-guidelines.html].
Are the third-party channels referenced above "committed to only
including free software"?

> All in all, I do not see the danger. :-)
>
> Cheers,
> simon

[bug#71697] [PATCH v5 1/3] scripts: lint: Add 'dry-run' option.

[Maxim Cournoyer]

Hi,

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

> * guix/scripts/lint.scm (guix-lint)[show-package-checkers]: New procedure.
> (show-help, %options): Add 'dry-run' option.
> * doc/guix.texi: Document it.
>
> Change-Id: I8c96e376d52c0961ccf2ab39f1fc856c762b089d
> ---
>  doc/guix.texi         |  3 +++
>  guix/scripts/lint.scm | 54 +++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 5b77c84b4a..6043962038 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -15463,6 +15463,9 @@ Invoking guix lint
>  List and describe all the available checkers that will be run on packages
>  and exit.
>  
> +@item --dry-run
> +Do not run the checkers.

Maybe, "Show which checkers would run with the provided lint options." ?

>  @item --checkers
>  @itemx -c
>  Only enable the checkers specified in a comma-separated list using the
> diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
> index ee3de51fb1..10abb05cf0 100644
> --- a/guix/scripts/lint.scm
> +++ b/guix/scripts/lint.scm
> @@ -100,6 +100,8 @@ (define (show-help)
>    (display (G_ "Usage: guix lint [OPTION]... [PACKAGE]...
>  Run a set of checkers on the specified package; if none is specified,
>  run the checkers on all packages.\n"))
> +  (display (G_ "
> +      --dry-run          do not run checkers "))

Perhaps, "print the lint checkers that would run" or similar

>    (display (G_ "
>    -c, --checkers=CHECKER1,CHECKER2...
>                           only run the specified checkers"))
> @@ -154,6 +156,9 @@ (define %options
>          (option '(#\n "no-network") #f #f
>                  (lambda (opt name arg result)
>                    (alist-cons 'no-network? #t result)))
> +        (option '("dry-run") #f #f
> +                (lambda (opt name arg result)
> +                  (alist-cons 'dry-run? #t result)))
>          (find (lambda (option)
>                  (member "load-path" (option-names option)))
>                %standard-build-options)
> @@ -218,14 +223,55 @@ (define-command (guix-lint . args)
>                  (proc store))
>                (proc #f)))
>  
> +        (define (show-package-checkers package checkers)
> +          (let* ((name (package-name package))
> +                 (version (package-version package))
> +                 (loc (package-location package))
> +                 (number-checkers (length checkers))
> +                 (number-all-checkers (length %all-checkers)))
> +            (cond
> +             ((= number-all-checkers number-checkers)
> +              (info loc (G_ "~a@~a: all the ~d checkers would run.~%")
> +                    name version
> +                    number-all-checkers))
> +             ((= 0 number-checkers)
> +              (info loc (G_ "~a@~a: none of ~d checkers would run~%")
> +                    name version
> +                    number-all-checkers))
> +             (else
> +              (let* ((excluded-checkers (lset-difference eq? %all-checkers checkers))
> +                     (number-excluded-checkers (length excluded-checkers))
> +                     (number-difference (- number-all-checkers number-excluded-checkers))
> +                     (sorter (lambda (list-checkers)
> +                               (sort (map (compose symbol->string lint-checker-name)
> +                                          list-checkers)
> +                                     string<?))))

Lines should be <= 80 chars, not 100.

> +                (info loc (G_ "~a@~a: ~d/~d checkers would run")
> +                      name version
> +                      number-checkers
> +                      number-all-checkers)
> +                (if (< number-excluded-checkers number-difference)
> +                    (format (current-error-port)
> +                            (G_ " at the exclusion of:~{ ~a~}.~%")
> +                            (sorter excluded-checkers))
> +                    (format (current-error-port)
> +                            ":~{ ~a~}.~%"
> +                            (sorter checkers))))))))
> +
>          (call-maybe-with-store
>           (lambda (store)
>             (cond
>              ((null? args)
> -             (fold-packages (lambda (p r) (run-checkers p checkers
> -                                                        #:store store)) '()))
> +             (when (assoc-ref opts 'dry-run?)
> +               (leave (G_ "too much information to display, did nothing and exit~%") ""))

Here also, the line width doesn't match our convention.

> +             (fold-packages (lambda (p r)
> +                              (unless (assoc-ref opts 'dry-run?)
> +                                  (run-checkers p checkers
> +                                                #:store store))) '()))
>              (else
>               (for-each (lambda (package)
> -                         (run-checkers package checkers
> -                                       #:store store))
> +                         (if (assoc-ref opts 'dry-run?)
> +                             (show-package-checkers package checkers)
> +                             (run-checkers package checkers
> +                                           #:store store)))
>                         args)))))))))

The rest looks reasonable to me, although like Greg I've come to wonder
whether disabling lint checks via package properties is not a bit of a
misfeature since it wouldn't be used by the Guix project itself, and
could lead to misunderstandings.

-- 
Thanks,
Maxim

[bug#71697] [PATCH v5 3/3] scripts: lint: Add hint for checker typo.

[Maxim Cournoyer]

Hi,

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

>
> Change-Id: I432c5b0570d2f413b59c2296666f3a4e5fb8c64c
> ---
>  guix/scripts/lint.scm | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
> index 2df0cba948..f4aa394686 100644
> --- a/guix/scripts/lint.scm
> +++ b/guix/scripts/lint.scm
> @@ -31,6 +31,7 @@ (define-module (guix scripts lint)
>    #:use-module (guix packages)
>    #:use-module (guix lint)
>    #:use-module (guix ui)
> +  #:use-module ((guix utils) #:select (string-closest))
>    #:use-module (guix store)
>    #:use-module (guix scripts)
>    #:use-module (guix scripts build)
> @@ -147,7 +148,15 @@ (define (option-checker short-long)
>                                                   ((short long) long)))))
>                (for-each (lambda (c)
>                            (unless (memq c checker-names)
> -                            (leave (G_ "~a: invalid checker~%") c)))
> +                            (let* ((name (symbol->string c))
> +                                   (checkers (map symbol->string checker-names))
> +                                   (hint (string-closest name checkers
> +                                                         #:threshold 3)))
> +                              (report-error (G_ "~a: invalid checker~%") name)
> +                              (when hint
> +                                (display-hint
> +                                 (format #f (G_ "Did you mean @code{~a}?~%") hint)))
> +                              (exit 1))))
>                          names)
>                (alist-cons option-name
>                            (filter (lambda (checker)

Patch 1 and 3 LGTM.  Patch 2 I'd rather wait to see if we can think
about a better option to match the original use case (exclude the
archival checker from 'guix lint' for privately developed packages). I
remember that perhaps the simplest thing for that would be to add a
nonfree license to (guix licenses); Guix users developing nonfree
software (ugh) could mark it as such and the lint code could disable
archival for such licensed packages (or maybe more generally if the
license used is unknown to Guix).

-- 
Thanks,
Maxim