GNU bug report logs - #47804
[PATCH] lint: Warn about underscores in package names.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 47804 in the body.
You can then email your comments to 47804 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Xinglu Chen <public <at> yoctocell.xyz>
To: guix-patches <at> gnu.org
Subject: [PATCH] lint: Warn about underscores in package names.
Date: Thu, 15 Apr 2021 18:00:46 +0200

As per section '16.4.2 Package Naming' in the manual, use hyphens
instead of underscores in package names.

* guix/lint.scm (check-name): Check whether the package name contains
underscores.
---
There was some discussion about this on guix-devel[1], but no consensus
was reached.  Should we whitelist certain names like “86_64” or something?

[1]: https://yhetil.org/guix-devel/87v991vkpi.fsf <at> nckx

 guix/lint.scm | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/guix/lint.scm b/guix/lint.scm
index a7d6bbba4f..d5ad69475e 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -11,6 +11,7 @@
 ;;; Copyright © 2018, 2019 Arun Isaac <arunisaac <at> systemreboot.net>
 ;;; Copyright © 2020 Chris Marusich <cmmarusich <at> gmail.com>
 ;;; Copyright © 2020 Timothy Sample <samplet <at> ngyro.com>
+;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -173,14 +174,20 @@
 (define (check-name package)
   "Check whether PACKAGE's name matches our guidelines."
   (let ((name (package-name package)))
-    ;; Currently checks only whether the name is too short.
-    (if (and (<= (string-length name) 1)
-             (not (string=? name "r"))) ; common-sense exception
-        (list
-         (make-warning package
-                       (G_ "name should be longer than a single character")
-                       #:field 'name))
-        '())))
+    (cond
+     ;; Currently checks only whether the name is too short.
+     ((and (<= (string-length name) 1)
+           (not (string=? name "r"))) ; common-sense exception
+      (list
+       (make-warning package
+                     (G_ "name should be longer than a single character")
+                     #:field 'name)))
+     ((string-match "_" name)
+      (list
+       (make-warning package
+                     (G_ "name should use hyphens instead of underscores")
+                     #:field 'name)))
+     (else '()))))
 
 (define (properly-starts-sentence? s)
   (string-match "^[(\"'`[:upper:][:digit:]]" s))

base-commit: a5bbd38fd131282e928144e869dcdf1e09259085
-- 
2.31.1

Message #8 received at 47804 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Xinglu Chen <public <at> yoctocell.xyz>, 47804 <at> debbugs.gnu.org
Subject: Re: [bug#47804] [PATCH] lint: Warn about underscores in package names.
Date: Thu, 15 Apr 2021 18:19:53 +0200

[Message part 1 (text/plain, inline)]

On Thu, 2021-04-15 at 18:00 +0200, Xinglu Chen wrote:
> As per section '16.4.2 Package Naming' in the manual, use hyphens
> instead of underscores in package names.
> 
> * guix/lint.scm (check-name): Check whether the package name contains
> underscores.
> ---
> There was some discussion about this on guix-devel[1], but no consensus
> was reached.  Should we whitelist certain names like “86_64” or something?

I dunno.  Perhaps for now, we can accept that the 'check-name' linter is
sometimes overly strict, and punt the exceptions for later?

> [1]: https://yhetil.org/guix-devel/87v991vkpi.fsf <at> nckx
> 
>  guix/lint.scm | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/guix/lint.scm b/guix/lint.scm
> index a7d6bbba4f..d5ad69475e 100644
> --- a/guix/lint.scm
> +++ b/guix/lint.scm
> @@ -11,6 +11,7 @@
>  ;;; Copyright © 2018, 2019 Arun Isaac <arunisaac <at> systemreboot.net>
>  ;;; Copyright © 2020 Chris Marusich <cmmarusich <at> gmail.com>
>  ;;; Copyright © 2020 Timothy Sample <samplet <at> ngyro.com>
> +;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -173,14 +174,20 @@
>  (define (check-name package)
>    "Check whether PACKAGE's name matches our guidelines."
>    (let ((name (package-name package)))
> -    ;; Currently checks only whether the name is too short.
> -    (if (and (<= (string-length name) 1)
> -             (not (string=? name "r"))) ; common-sense exception
> -        (list
> -         (make-warning package
> -                       (G_ "name should be longer than a single character")
> -                       #:field 'name))
> -        '())))
> +    (cond
> +     ;; Currently checks only whether the name is too short.
> +     ((and (<= (string-length name) 1)
> +           (not (string=? name "r"))) ; common-sense exception
> +      (list
> +       (make-warning package
> +                     (G_ "name should be longer than a single character")
> +                     #:field 'name)))
> +     ((string-match "_" name)
> +      (list
> +       (make-warning package
> +                     (G_ "name should use hyphens instead of underscores")
> +                     #:field 'name)))
> +     (else '()))))

I didn't test this, but that seems about right.
Ideally, you should add a corresponding test in tests/lint.scm
(IIRC, maybe the linter tests are elsewhere).

However, Guix is currently in the "string freeze", so this cannot yet be merged,
IIUC.

Greetings,
Maxime.

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 47804 <at> debbugs.gnu.org (full text, mbox):

From: Xinglu Chen <public <at> yoctocell.xyz>
To: Maxime Devos <maximedevos <at> telenet.be>, 47804 <at> debbugs.gnu.org
Subject: Re: [bug#47804] [PATCH] lint: Warn about underscores in package names.
Date: Thu, 15 Apr 2021 20:15:01 +0200

On Thu, Apr 15 2021, Maxime Devos wrote:

>> There was some discussion about this on guix-devel[1], but no
>> consensus was reached.  Should we whitelist certain names like
>> “86_64” or something?
>
> I dunno.  Perhaps for now, we can accept that the 'check-name' linter
> is sometimes overly strict, and punt the exceptions for later?

Ok, that sounds like a good idea.

> I didn't test this, but that seems about right.  Ideally, you should
> add a corresponding test in tests/lint.scm (IIRC, maybe the linter
> tests are elsewhere).

Will do.

> However, Guix is currently in the "string freeze", so this cannot yet
> be merged, IIUC.

I thought the “string freeze” was for the manual, or did I miss
something?

Thanks for the review!

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Xinglu Chen <public <at> yoctocell.xyz>
To: guix-patches <at> gnu.org
Cc: Maxime Devos <maximedevos <at> telenet.be>
Subject: [PATCH v2] lint: Warn about underscores in package names.
Date: Thu, 15 Apr 2021 20:31:44 +0200

As per section '16.4.2 Package Naming' in the manual, use hyphens
instead of underscores in package names.

* guix/lint.scm (check-name): Check whether the package name contains
underscores.
* tests/lint.scm ("name: use underscore in package name"): New test.
---
Changes since v1:
- Add test for package name with underscore.

 guix/lint.scm  | 24 ++++++++++++++++--------
 tests/lint.scm |  7 +++++++
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/guix/lint.scm b/guix/lint.scm
index a7d6bbba4f..38699e2927 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -11,6 +11,7 @@
 ;;; Copyright © 2018, 2019 Arun Isaac <arunisaac <at> systemreboot.net>
 ;;; Copyright © 2020 Chris Marusich <cmmarusich <at> gmail.com>
 ;;; Copyright © 2020 Timothy Sample <samplet <at> ngyro.com>
+;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -81,6 +82,7 @@
             check-synopsis-style
             check-derivation
             check-home-page
+            check-name
             check-source
             check-source-file-name
             check-source-unstable-tarball
@@ -173,14 +175,20 @@
 (define (check-name package)
   "Check whether PACKAGE's name matches our guidelines."
   (let ((name (package-name package)))
-    ;; Currently checks only whether the name is too short.
-    (if (and (<= (string-length name) 1)
-             (not (string=? name "r"))) ; common-sense exception
-        (list
-         (make-warning package
-                       (G_ "name should be longer than a single character")
-                       #:field 'name))
-        '())))
+    (cond
+     ;; Currently checks only whether the name is too short.
+     ((and (<= (string-length name) 1)
+           (not (string=? name "r"))) ; common-sense exception
+      (list
+       (make-warning package
+                     (G_ "name should be longer than a single character")
+                     #:field 'name)))
+     ((string-match "_" name)
+      (list
+       (make-warning package
+                     (G_ "name should use hyphens instead of underscores")
+                     #:field 'name)))
+     (else '()))))
 
 (define (properly-starts-sentence? s)
   (string-match "^[(\"'`[:upper:][:digit:]]" s))
diff --git a/tests/lint.scm b/tests/lint.scm
index bd8604f589..a2c8665142 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2017 Efraim Flashner <efraim <at> flashner.co.il>
 ;;; Copyright © 2018, 2019 Arun Isaac <arunisaac <at> systemreboot.net>
 ;;; Copyright © 2020 Timothy Sample <samplet <at> ngyro.com>
+;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -270,6 +271,12 @@
                             (description "Imagine this is Taylor UUCP."))))
     (check-synopsis-style pkg)))
 
+(test-equal "name: use underscore in package name"
+  "name should use hyphens instead of underscores"
+  (single-lint-warning-message
+   (let ((pkg (dummy-package "under_score")))
+     (check-name pkg))))
+
 (test-equal "inputs: pkg-config is probably a native input"
   "'pkg-config' should probably be a native input"
   (single-lint-warning-message

base-commit: a5bbd38fd131282e928144e869dcdf1e09259085
-- 
2.31.1

Message #17 received at 47804 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Xinglu Chen <public <at> yoctocell.xyz>, 47804 <at> debbugs.gnu.org
Subject: Re: [bug#47804] [PATCH] lint: Warn about underscores in package names.
Date: Fri, 16 Apr 2021 12:19:39 +0200

[Message part 1 (text/plain, inline)]

On Thu, 2021-04-15 at 20:15 +0200, Xinglu Chen wrote:
> On Thu, Apr 15 2021, Maxime Devos wrote:
> [...]
> 
> > However, Guix is currently in the "string freeze", so this cannot yet
> > be merged, IIUC.
> 
> I thought the “string freeze” was for the manual, or did I miss
> something?

<https://lists.gnu.org/archive/html/guix-devel/2021-04/msg00180.html>
The freeze if for both the manual and guix itself (but not the packages).

Greetings,
Maxime.

[signature.asc (application/pgp-signature, inline)]

Message #22 received at 47804-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Xinglu Chen <public <at> yoctocell.xyz>
Cc: 47804-done <at> debbugs.gnu.org, Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: bug#47804: [PATCH] lint: Warn about underscores in package names.
Date: Fri, 16 Apr 2021 22:54:47 +0200

[Message part 1 (text/plain, inline)]

Hi,

Xinglu Chen <public <at> yoctocell.xyz> skribis:

> As per section '16.4.2 Package Naming' in the manual, use hyphens
> instead of underscores in package names.
>
> * guix/lint.scm (check-name): Check whether the package name contains
> underscores.
> * tests/lint.scm ("name: use underscore in package name"): New test.

Applied with the minor change below, which avoids regexps
(‘string-match’ performs regexp matches, which is overkill here).

Thank you and thanks Maxime for the review!

Ludo’.

[Message part 2 (text/x-patch, inline)]

diff --git a/guix/lint.scm b/guix/lint.scm
index 38699e2927..1bebfe03d3 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -183,7 +183,7 @@
        (make-warning package
                      (G_ "name should be longer than a single character")
                      #:field 'name)))
-     ((string-match "_" name)
+     ((string-index name #\_)
       (list
        (make-warning package
                      (G_ "name should use hyphens instead of underscores")

Message #25 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Julien Lepiller <julien <at> lepiller.eu>
To: guix-patches <at> gnu.org, Ludovic Courtès <ludo <at> gnu.org>,
 Xinglu Chen <public <at> yoctocell.xyz>
Cc: 47804-done <at> debbugs.gnu.org, Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: bug#47804: [PATCH] lint: Warn about underscores in package names.
Date: Fri, 16 Apr 2021 17:42:30 -0400

Le 16 avril 2021 16:54:47 GMT-04:00, "Ludovic Courtès" <ludo <at> gnu.org> a écrit :
>Hi,
>
>Xinglu Chen <public <at> yoctocell.xyz> skribis:
>
>> As per section '16.4.2 Package Naming' in the manual, use hyphens
>> instead of underscores in package names.
>>
>> * guix/lint.scm (check-name): Check whether the package name contains
>> underscores.
>> * tests/lint.scm ("name: use underscore in package name"): New test.
>
>Applied with the minor change below, which avoids regexps
>(‘string-match’ performs regexp matches, which is overkill here).
>
>Thank you and thanks Maxime for the review!
>
>Ludo’.

Hm… unless I'm mistaken, this patch adds a string to the guix domain, which is part of the string freeze.

The guix-packages is not part of the string freeze, that is to say synopsis and description of packages, only.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.