GNU bug report logs -
#47804
[PATCH] lint: Warn about underscores in package names.
Previous Next
Reported by: Xinglu Chen <public <at> yoctocell.xyz>
Date: Thu, 15 Apr 2021 16:02:02 UTC
Severity: normal
Tags: patch
Done: Ludovic Courtès <ludo <at> gnu.org>
Bug is archived. No further changes may be made.
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.
Report forwarded
to
guix-patches <at> gnu.org
:
bug#47804
; Package
guix-patches
.
(Thu, 15 Apr 2021 16:02:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Xinglu Chen <public <at> yoctocell.xyz>
:
New bug report received and forwarded. Copy sent to
guix-patches <at> gnu.org
.
(Thu, 15 Apr 2021 16:02:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
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
Information forwarded
to
guix-patches <at> gnu.org
:
bug#47804
; Package
guix-patches
.
(Thu, 15 Apr 2021 16:21:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 47804 <at> debbugs.gnu.org (full text, mbox):
[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)]
Information forwarded
to
guix-patches <at> gnu.org
:
bug#47804
; Package
guix-patches
.
(Thu, 15 Apr 2021 18:16:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 47804 <at> debbugs.gnu.org (full text, mbox):
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!
Information forwarded
to
guix-patches <at> gnu.org
:
bug#47804
; Package
guix-patches
.
(Thu, 15 Apr 2021 18:33:02 GMT)
Full text and
rfc822 format available.
Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):
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
Information forwarded
to
guix-patches <at> gnu.org
:
bug#47804
; Package
guix-patches
.
(Fri, 16 Apr 2021 10:20:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 47804 <at> debbugs.gnu.org (full text, mbox):
[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)]
Reply sent
to
Ludovic Courtès <ludo <at> gnu.org>
:
You have taken responsibility.
(Fri, 16 Apr 2021 20:56:01 GMT)
Full text and
rfc822 format available.
Notification sent
to
Xinglu Chen <public <at> yoctocell.xyz>
:
bug acknowledged by developer.
(Fri, 16 Apr 2021 20:56:02 GMT)
Full text and
rfc822 format available.
Message #22 received at 47804-done <at> debbugs.gnu.org (full text, mbox):
[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")
Information forwarded
to
guix-patches <at> gnu.org
:
bug#47804
; Package
guix-patches
.
(Fri, 16 Apr 2021 21:43:01 GMT)
Full text and
rfc822 format available.
Message #25 received at submit <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
guix-patches <at> gnu.org
:
bug#47804
; Package
guix-patches
.
(Fri, 16 Apr 2021 21:43:02 GMT)
Full text and
rfc822 format available.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Sat, 15 May 2021 11:24:04 GMT)
Full text and
rfc822 format available.
This bug report was last modified 2 years and 340 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.