GNU bug report logs -
#33575
[PATCH] guix: lint: Add checker to check if inputs are sorted.
Previous Next
Reported by: Arun Isaac <arunisaac <at> systemreboot.net>
Date: Sun, 2 Dec 2018 07:43:02 UTC
Severity: normal
Tags: patch
Done: Arun Isaac <arunisaac <at> systemreboot.net>
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 33575 in the body.
You can then email your comments to 33575 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#33575
; Package
guix-patches
.
(Sun, 02 Dec 2018 07:43:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Arun Isaac <arunisaac <at> systemreboot.net>
:
New bug report received and forwarded. Copy sent to
guix-patches <at> gnu.org
.
(Sun, 02 Dec 2018 07:43:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
* guix/scripts/lint.scm (check-inputs-should-be-sorted): New procedure.
(%checkers): Add it.
---
guix/scripts/lint.scm | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 2314f3b28..37e8a1ec5 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -8,6 +8,7 @@
;;; Copyright © 2017 Alex Kost <alezost <at> gmail.com>
;;; Copyright © 2017 Tobias Geerinckx-Rice <me <at> tobias.gr>
;;; Copyright © 2017 Efraim Flashner <efraim <at> flashner.co.il>
+;;; Copyright © 2018 Arun Isaac <arunisaac <at> systemreboot.net>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -301,6 +302,22 @@ of a package, and INPUT-NAMES, a list of package specifications such as
(package-input-intersection (package-direct-inputs package)
input-names))))
+(define (check-inputs-should-be-sorted package)
+ ;; Emit a warning if inputs, native inputs or propagated inputs of PACKAGE
+ ;; are not lexicographically ordered.
+ (define (check-inputs inputs-accessor input-type)
+ (unless (sorted? (map (match-lambda
+ ((name input) name))
+ (inputs-accessor package))
+ string<?)
+ (emit-warning
+ package
+ (format #f (G_ "~a should be in lexicographic order") input-type))))
+
+ (check-inputs package-inputs (G_ "inputs"))
+ (check-inputs package-native-inputs (G_ "native inputs"))
+ (check-inputs package-propagated-inputs (G_ "propagated inputs")))
+
(define (package-name-regexp package)
"Return a regexp that matches PACKAGE's name as a word at the beginning of a
line."
@@ -1032,6 +1049,10 @@ them for PACKAGE."
(name 'inputs-should-not-be-input)
(description "Identify inputs that shouldn't be inputs at all")
(check check-inputs-should-not-be-an-input-at-all))
+ (lint-checker
+ (name 'inputs-should-be-sorted)
+ (description "Verify that inputs are in lexicographic order")
+ (check check-inputs-should-be-sorted))
(lint-checker
(name 'patch-file-names)
(description "Validate file names and availability of patches")
--
2.19.1
Information forwarded
to
guix-patches <at> gnu.org
:
bug#33575
; Package
guix-patches
.
(Mon, 03 Dec 2018 13:32:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 33575 <at> debbugs.gnu.org (full text, mbox):
Hello,
Arun Isaac <arunisaac <at> systemreboot.net> skribis:
> * guix/scripts/lint.scm (check-inputs-should-be-sorted): New procedure.
> (%checkers): Add it.
> ---
> guix/scripts/lint.scm | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
> index 2314f3b28..37e8a1ec5 100644
> --- a/guix/scripts/lint.scm
> +++ b/guix/scripts/lint.scm
> @@ -8,6 +8,7 @@
> ;;; Copyright © 2017 Alex Kost <alezost <at> gmail.com>
> ;;; Copyright © 2017 Tobias Geerinckx-Rice <me <at> tobias.gr>
> ;;; Copyright © 2017 Efraim Flashner <efraim <at> flashner.co.il>
> +;;; Copyright © 2018 Arun Isaac <arunisaac <at> systemreboot.net>
> ;;;
> ;;; This file is part of GNU Guix.
> ;;;
> @@ -301,6 +302,22 @@ of a package, and INPUT-NAMES, a list of package specifications such as
> (package-input-intersection (package-direct-inputs package)
> input-names))))
>
> +(define (check-inputs-should-be-sorted package)
> + ;; Emit a warning if inputs, native inputs or propagated inputs of PACKAGE
> + ;; are not lexicographically ordered.
It’s something we rarely do so we’d get warnings for most packages. As
a side effect, people may pay less attention to what ‘guix lint’ says.
As for the goal itself, I think sorting is a good idea when there are
lots of inputs (things like IceCat), but otherwise I personally don’t
think it matters that much.
What do people think?
Thanks,
Ludo’.
Information forwarded
to
guix-patches <at> gnu.org
:
bug#33575
; Package
guix-patches
.
(Thu, 06 Dec 2018 00:43:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 33575 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello,
ludo <at> gnu.org (Ludovic Courtès) writes:
[…]
> It’s something we rarely do so we’d get warnings for most packages. As
> a side effect, people may pay less attention to what ‘guix lint’ says.
I think this should not stop us from improving a linter and an option
like --misc-checks=sort-input,foo,bar could be used for such cases.
[…]
Oleg.
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
guix-patches <at> gnu.org
:
bug#33575
; Package
guix-patches
.
(Thu, 06 Dec 2018 12:32:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 33575 <at> debbugs.gnu.org (full text, mbox):
On 2018-12-06 01:42, Oleg Pykhalov wrote:
> Hello,
>
> ludo <at> gnu.org (Ludovic Courtès) writes:
>
> […]
>
>> It’s something we rarely do so we’d get warnings for most packages. As
>> a side effect, people may pay less attention to what ‘guix lint’ says.
>
> I think this should not stop us from improving a linter and an option
> like --misc-checks=sort-input,foo,bar could be used for such cases.
I agree with Oleg here.
If many packages needs inputs to be sorted lets write a guix lint --sort
modelling the updater (that is make the changes in the work tree to be
committed).
--
Cheers
Swedebugia
Information forwarded
to
guix-patches <at> gnu.org
:
bug#33575
; Package
guix-patches
.
(Fri, 07 Dec 2018 13:09:01 GMT)
Full text and
rfc822 format available.
Message #17 received at 33575 <at> debbugs.gnu.org (full text, mbox):
swedebugia <at> riseup.net writes:
> On 2018-12-06 01:42, Oleg Pykhalov wrote:
>> Hello,
>>
>> ludo <at> gnu.org (Ludovic Courtès) writes:
>>
>> […]
>>
>>> It’s something we rarely do so we’d get warnings for most packages. As
>>> a side effect, people may pay less attention to what ‘guix lint’ says.
>>
>> I think this should not stop us from improving a linter and an option
>> like --misc-checks=sort-input,foo,bar could be used for such cases.
>
> I agree with Oleg here.
>
> If many packages needs inputs to be sorted lets write a guix lint --sort
> modelling the updater (that is make the changes in the work tree to be
> committed).
+1
Information forwarded
to
guix-patches <at> gnu.org
:
bug#33575
; Package
guix-patches
.
(Sat, 08 Dec 2018 03:52:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 33575 <at> debbugs.gnu.org (full text, mbox):
Arun Isaac <arunisaac <at> systemreboot.net> writes:
> * guix/scripts/lint.scm (check-inputs-should-be-sorted): New procedure.
> (%checkers): Add it.
> [...]
>
> +(define (check-inputs-should-be-sorted package)
> + ;; Emit a warning if inputs, native inputs or propagated inputs of PACKAGE
> + ;; are not lexicographically ordered.
Hello, consider 'gspell', it has some native-inputs for build and some
for test:
(native-inputs
`(("glib" ,glib "bin")
("pkg-config" ,pkg-config)
("xmllint" ,libxml2)
;; For tests.
("aspell-dict-en" ,aspell-dict-en)
("xorg-server" ,xorg-server)))
Currently I'd seperated them by a comment like this.
If they are sorted, I have to add comment for each test input:
`(("aspell-dict-en", aspecll-dict-en) ; for test
("glib" ,glib "bin")
("pkg-config" ,pkg-config)
("xmllint" ,libxml2)
("xorg-server" ,xorg-server)) ; for test
Which will be a little annoying...
Information forwarded
to
guix-patches <at> gnu.org
:
bug#33575
; Package
guix-patches
.
(Sat, 08 Dec 2018 05:10:01 GMT)
Full text and
rfc822 format available.
Message #23 received at 33575 <at> debbugs.gnu.org (full text, mbox):
Ludo', Arun,
Ludovic Courtès wrote:
>> + ;; Emit a warning if inputs, native inputs or propagated
>> inputs
>> of PACKAGE
>> + ;; are not lexicographically ordered.
>
> It's something we rarely do so we'd get warnings for most
> packages. As
> a side effect, people may pay less attention to what ‘guix lint’
> says.
Even I agree :-) There are valid reasons not to sort them.
> As for the goal itself, I think sorting is a good idea when
> there are
> lots of inputs (things like IceCat), but otherwise I personally
> don't
> think it matters that much.
Do we already check for duplication?
I sometimes order inputs for the same reason I sort module
imports: to catch duplicates. These are usually harmless and
produce no errors.
Kind regards,
T G-R
Information forwarded
to
guix-patches <at> gnu.org
:
bug#33575
; Package
guix-patches
.
(Sat, 08 Dec 2018 07:59:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 33575 <at> debbugs.gnu.org (full text, mbox):
On 2018-12-08 04:51, iyzsong <at> member.fsf.org wrote:
> Arun Isaac <arunisaac <at> systemreboot.net> writes:
>
>> * guix/scripts/lint.scm (check-inputs-should-be-sorted): New procedure.
>> (%checkers): Add it.
>> [...]
>>
>> +(define (check-inputs-should-be-sorted package)
>> + ;; Emit a warning if inputs, native inputs or propagated inputs of PACKAGE
>> + ;; are not lexicographically ordered.
>
> Hello, consider 'gspell', it has some native-inputs for build and some
> for test:
>
> (native-inputs
> `(("glib" ,glib "bin")
> ("pkg-config" ,pkg-config)
> ("xmllint" ,libxml2)
>
> ;; For tests.
> ("aspell-dict-en" ,aspell-dict-en)
> ("xorg-server" ,xorg-server)))
>
> Currently I'd seperated them by a comment like this.
>
> If they are sorted, I have to add comment for each test input:
>
> `(("aspell-dict-en", aspecll-dict-en) ; for test
> ("glib" ,glib "bin")
> ("pkg-config" ,pkg-config)
> ("xmllint" ,libxml2)
> ("xorg-server" ,xorg-server)) ; for test
>
> Which will be a little annoying...
You convinced me sorting is a bad idea. Thanks for providing a good
argument :)
--
Cheers
Swedebugia
Information forwarded
to
guix-patches <at> gnu.org
:
bug#33575
; Package
guix-patches
.
(Sat, 08 Dec 2018 13:31:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 33575 <at> debbugs.gnu.org (full text, mbox):
Hello,
Tobias Geerinckx-Rice <somebody <at> not-sent-or-endorsed-by.tobias.gr>
skribis:
> Ludovic Courtès wrote:
[...]
>> As for the goal itself, I think sorting is a good idea when
>> there are
>> lots of inputs (things like IceCat), but otherwise I personally
>> don't
>> think it matters that much.
>
> Do we already check for duplication?
No but it would be a worthy check!
Ludo’.
Information forwarded
to
guix-patches <at> gnu.org
:
bug#33575
; Package
guix-patches
.
(Sat, 08 Dec 2018 13:36:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 33575 <at> debbugs.gnu.org (full text, mbox):
> If they are sorted, I have to add comment for each test input:
>
> `(("aspell-dict-en", aspecll-dict-en) ; for test
> ("glib" ,glib "bin")
> ("pkg-config" ,pkg-config)
> ("xmllint" ,libxml2)
> ("xorg-server" ,xorg-server)) ; for test
>
> Which will be a little annoying...
I too find this convincing. It's not a good idea to enforce sorted
inputs all the time. If there is sufficient consensus, we can close this
bug report.
Information forwarded
to
guix-patches <at> gnu.org
:
bug#33575
; Package
guix-patches
.
(Sun, 09 Dec 2018 22:50:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 33575 <at> debbugs.gnu.org (full text, mbox):
Hi,
Arun Isaac <arunisaac <at> systemreboot.net> writes:
>> If they are sorted, I have to add comment for each test input:
>>
>> `(("aspell-dict-en", aspecll-dict-en) ; for test
>> ("glib" ,glib "bin")
>> ("pkg-config" ,pkg-config)
>> ("xmllint" ,libxml2)
>> ("xorg-server" ,xorg-server)) ; for test
>>
>> Which will be a little annoying...
>
> I too find this convincing. It's not a good idea to enforce sorted
> inputs all the time. If there is sufficient consensus, we can close this
> bug report.
Maybe our test inputs should have their own field? This would make their
raison d'être explicit and remove the need of using comments.
Maxim
Information forwarded
to
guix-patches <at> gnu.org
:
bug#33575
; Package
guix-patches
.
(Mon, 10 Dec 2018 11:20:01 GMT)
Full text and
rfc822 format available.
Message #38 received at 33575 <at> debbugs.gnu.org (full text, mbox):
Arun Isaac <arunisaac <at> systemreboot.net> writes:
>> If they are sorted, I have to add comment for each test input:
>>
>> `(("aspell-dict-en", aspecll-dict-en) ; for test
>> ("glib" ,glib "bin")
>> ("pkg-config" ,pkg-config)
>> ("xmllint" ,libxml2)
>> ("xorg-server" ,xorg-server)) ; for test
>>
>> Which will be a little annoying...
>
> I too find this convincing. It's not a good idea to enforce sorted
> inputs all the time. If there is sufficient consensus, we can close this
> bug report.
Yes, I think so. Thank you!
Information forwarded
to
guix-patches <at> gnu.org
:
bug#33575
; Package
guix-patches
.
(Mon, 10 Dec 2018 11:46:01 GMT)
Full text and
rfc822 format available.
Message #41 received at 33575 <at> debbugs.gnu.org (full text, mbox):
Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:
> Hi,
>
> Arun Isaac <arunisaac <at> systemreboot.net> writes:
>
>>> If they are sorted, I have to add comment for each test input:
>>>
>>> `(("aspell-dict-en", aspecll-dict-en) ; for test
>>> ("glib" ,glib "bin")
>>> ("pkg-config" ,pkg-config)
>>> ("xmllint" ,libxml2)
>>> ("xorg-server" ,xorg-server)) ; for test
>>>
>>> Which will be a little annoying...
>>
>> I too find this convincing. It's not a good idea to enforce sorted
>> inputs all the time. If there is sufficient consensus, we can close this
>> bug report.
>
> Maybe our test inputs should have their own field? This would make their
> raison d'être explicit and remove the need of using comments.
Yeah, something like:
(package
...
(inputs ...)
(test:inputs ...)
(test:native-inputs ...))
If we plan to support build packages with tests disabled, this would be
the way to go. And due to how build works in guix, if tests are
disabled, it would be considered as a different derivation/package, so
the main use case may be:
- I disable substitute servers to build all packages from sources
locally.
- I want to disable tests for some packages as they are too slow...
I don't have this use case now, and seperate package inputs will be a
big change, so I think the current way is totally ok.
Reply sent
to
Arun Isaac <arunisaac <at> systemreboot.net>
:
You have taken responsibility.
(Tue, 18 Dec 2018 20:38:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Arun Isaac <arunisaac <at> systemreboot.net>
:
bug acknowledged by developer.
(Tue, 18 Dec 2018 20:38:02 GMT)
Full text and
rfc822 format available.
Message #46 received at 33575-done <at> debbugs.gnu.org (full text, mbox):
I'm closing this bug report since I believe we have reached a consensus
that this patch is unnecessary. Thank you all for your thoughts!
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Wed, 16 Jan 2019 12:24:04 GMT)
Full text and
rfc822 format available.
This bug report was last modified 5 years and 96 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.