diff mbox series

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

Message ID 4e0382d4b36b12d59774d23e0e9177889e2398f2.1718994792.git.zimon.toutoune@gmail.com
State New
Headers show
Series [bug#71697,v2] guix: scripts: lint: Honor package property to exclude chercker. | expand

Commit Message

Simon Tournier June 21, 2024, 6:33 p.m. UTC
* 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(-)


base-commit: bc8a41f4a8d9f1f0525d7bc97c67ed3c8aea3111

Comments

Liliana Marie Prikler June 21, 2024, 9:09 p.m. UTC | #1
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
MSavoritias June 22, 2024, 2:29 p.m. UTC | #2
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
Simon Tournier June 22, 2024, 3:40 p.m. UTC | #3
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
MSavoritias June 24, 2024, 8:21 a.m. UTC | #4
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
diff mbox series

Patch

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