[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