[PATCH] lint: Warn about underscores in package names.

DoneSubmitted by Xinglu Chen.
Details
4 participants
  • Julien Lepiller
  • Ludovic Courtès
  • Maxime Devos
  • Xinglu Chen
Owner
unassigned
Severity
normal
X
X
Xinglu Chen wrote on 15 Apr 18:00 +0200
(address . guix-patches@gnu.org)
dc3d1c690b40861eab4d623fb8bca88f4194e209.1618502119.git.public@yoctocell.xyz
As per section '16.4.2 Package Naming' in the manual, use hyphensinstead of underscores in package names.
* guix/lint.scm (check-name): Check whether the package name containsunderscores.---There was some discussion about this on guix-devel[1], but no consensuswas reached. Should we whitelist certain names like “86_64” or something?
[1]: https://yhetil.org/guix-devel/87v991vkpi.fsf@nckx
guix/lint.scm | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)
Toggle diff (45 lines)diff --git a/guix/lint.scm b/guix/lint.scmindex a7d6bbba4f..d5ad69475e 100644--- a/guix/lint.scm+++ b/guix/lint.scm@@ -11,6 +11,7 @@ ;;; Copyright © 2018, 2019 Arun Isaac <arunisaac@systemreboot.net> ;;; Copyright © 2020 Chris Marusich <cmmarusich@gmail.com> ;;; Copyright © 2020 Timothy Sample <samplet@ngyro.com>+;;; Copyright © 2021 Xinglu Chen <public@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
M
M
Maxime Devos wrote on 15 Apr 18:19 +0200
e1a120c2fbd81850ec7a8281f6cc14cbdc0344fb.camel@telenet.be
On Thu, 2021-04-15 at 18:00 +0200, Xinglu Chen wrote:
Toggle quote (9 lines)> 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 issometimes overly strict, and punt the exceptions for later?
Toggle quote (44 lines)> [1]: https://yhetil.org/guix-devel/87v991vkpi.fsf@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@systemreboot.net>> ;;; Copyright © 2020 Chris Marusich <cmmarusich@gmail.com>> ;;; Copyright © 2020 Timothy Sample <samplet@ngyro.com>> +;;; Copyright © 2021 Xinglu Chen <public@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.
-----BEGIN PGP SIGNATURE-----
iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYHhnqRccbWF4aW1lZGV2b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7qHoAQCJkjMTs4202E/7IFSOPxia4GVu4OHuE4d2j4Vici9O0QEAp56CBgX7ASGNK2RUFme7pCmtXujKI6TxpHcpx/804QE==pJJG-----END PGP SIGNATURE-----

X
X
Xinglu Chen wrote on 15 Apr 20:15 +0200
8735vrh0t6.fsf@yoctocell.xyz
On Thu, Apr 15 2021, Maxime Devos wrote:
Toggle quote (7 lines)>> 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.
Toggle quote (4 lines)> 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.
Toggle quote (3 lines)> 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 misssomething?
Thanks for the review!
X
X
Xinglu Chen wrote on 15 Apr 20:31 +0200
[PATCH v2] lint: Warn about underscores in package names.
(address . guix-patches@gnu.org)(name . Maxime Devos)(address . maximedevos@telenet.be)
94d0b8cb2322ed634442c0ebb79589e5dc05e526.1618511382.git.public@yoctocell.xyz
As per section '16.4.2 Package Naming' in the manual, use hyphensinstead of underscores in package names.
* guix/lint.scm (check-name): Check whether the package name containsunderscores.* 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(-)
Toggle diff (78 lines)diff --git a/guix/lint.scm b/guix/lint.scmindex a7d6bbba4f..38699e2927 100644--- a/guix/lint.scm+++ b/guix/lint.scm@@ -11,6 +11,7 @@ ;;; Copyright © 2018, 2019 Arun Isaac <arunisaac@systemreboot.net> ;;; Copyright © 2020 Chris Marusich <cmmarusich@gmail.com> ;;; Copyright © 2020 Timothy Sample <samplet@ngyro.com>+;;; Copyright © 2021 Xinglu Chen <public@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.scmindex bd8604f589..a2c8665142 100644--- a/tests/lint.scm+++ b/tests/lint.scm@@ -8,6 +8,7 @@ ;;; Copyright © 2017 Efraim Flashner <efraim@flashner.co.il> ;;; Copyright © 2018, 2019 Arun Isaac <arunisaac@systemreboot.net> ;;; Copyright © 2020 Timothy Sample <samplet@ngyro.com>+;;; Copyright © 2021 Xinglu Chen <public@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
M
M
Maxime Devos wrote on 16 Apr 12:19 +0200
Re: [bug#47804] [PATCH] lint: Warn about underscores in package names.
762053ed2175f1ded96da8d85855843d73936364.camel@telenet.be
On Thu, 2021-04-15 at 20:15 +0200, Xinglu Chen wrote:
Toggle quote (9 lines)> 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.htmlThe freeze if for both the manual and guix itself (but not the packages).
Greetings,Maxime.
-----BEGIN PGP SIGNATURE-----
iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYHlkuxccbWF4aW1lZGV2b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7lSBAQCIDxvJ6k1JE0Eqj3baGyyz6uB5PuXkpzcu20JXXLv7MQD/bsGmJZOwzsa2bGecPOdtIwbN18VF8ZEP140SvpbRUQA==btmY-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 16 Apr 22:54 +0200
Re: bug#47804: [PATCH] lint: Warn about underscores in package names.
(name . Xinglu Chen)(address . public@yoctocell.xyz)
87eefa2bmw.fsf_-_@gnu.org
Hi,
Xinglu Chen <public@yoctocell.xyz> skribis:
Toggle quote (7 lines)> 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’.
Toggle diff (13 lines)diff --git a/guix/lint.scm b/guix/lint.scmindex 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")
Closed
J
J
Julien Lepiller wrote on 16 Apr 23:42 +0200
E609DF3C-68B3-4A32-B103-5C38F64834E0@lepiller.eu
Le 16 avril 2021 16:54:47 GMT-04:00, "Ludovic Courtès" <ludo@gnu.org> a écrit :
Toggle quote (18 lines)>Hi,>>Xinglu Chen <public@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.
?