* [PATCH 0/2] Add "guix lint".
@ 2014-07-21 23:51 Cyril Roelandt
2014-07-21 23:51 ` [PATCH 1/2] scripts: add guix lint Cyril Roelandt
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Cyril Roelandt @ 2014-07-21 23:51 UTC (permalink / raw)
To: guix-devel
Hello,
The first patch adds the "guix lint" command, that can be used to run a few
"checkers" on a given package, or on all defined packages. It can currently be
used to:
- warn users about input packages that should be native inputs (such as
pkg-config);
- find stylistic issues in patch names;
- find stylistic issues in the synopses (trailing period, for instance).
Those checkers are not always right, but I think they might be useful to people
getting started with Guix packaging, as some of these issues have been raised
multiple times during patch reviews.
Other checkers may be added in the future, depending on our needs.
There are currently no tests; I'm willing to write them ASAP but wanted to get
some feedback first, especially because I'm not an experienced Guile programmer.
Cyril.
---
Cyril Roelandt (2):
scripts: add guix lint
gnu/packages: Remove trailing periods in some synopses.
Makefile.am | 1 +
gnu/packages/fontutils.scm | 4 +-
gnu/packages/gnome.scm | 2 +-
gnu/packages/lua.scm | 2 +-
gnu/packages/pdf.scm | 2 +-
gnu/packages/python.scm | 4 +-
gnu/packages/sdl.scm | 2 +-
guix/scripts/lint.scm | 188 +++++++++++++++++++++++++++++++++++++++++++++
8 files changed, 197 insertions(+), 8 deletions(-)
create mode 100644 guix/scripts/lint.scm
--
1.8.4.rc3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] scripts: add guix lint
2014-07-21 23:51 [PATCH 0/2] Add "guix lint" Cyril Roelandt
@ 2014-07-21 23:51 ` Cyril Roelandt
2014-07-22 9:03 ` Ludovic Courtès
2014-07-21 23:51 ` [PATCH 2/2] gnu/packages: Remove trailing periods in some synopses Cyril Roelandt
2014-07-22 19:27 ` [PATCH 0/2] Add "guix lint" Andreas Enge
2 siblings, 1 reply; 15+ messages in thread
From: Cyril Roelandt @ 2014-07-21 23:51 UTC (permalink / raw)
To: guix-devel
* guix/scripts/lint.scm: New file. Defines a 'lint' tool for Guix packages.
* Makefile.am (MODULES): Add it.
---
Makefile.am | 1 +
guix/scripts/lint.scm | 188 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 189 insertions(+)
create mode 100644 guix/scripts/lint.scm
diff --git a/Makefile.am b/Makefile.am
index 41e0e67..e6d2556 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -87,6 +87,7 @@ MODULES = \
guix/scripts/authenticate.scm \
guix/scripts/refresh.scm \
guix/scripts/system.scm \
+ guix/scripts/lint.scm \
guix.scm \
$(GNU_SYSTEM_MODULES)
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
new file mode 100644
index 0000000..f60718f
--- /dev/null
+++ b/guix/scripts/lint.scm
@@ -0,0 +1,188 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2014 Cyril Roelandt <tipecaml@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (guix scripts lint)
+ #:use-module (guix base32)
+ #:use-module (guix packages)
+ #:use-module (guix records)
+ #:use-module (guix ui)
+ #:use-module (guix utils)
+ #:use-module (gnu packages)
+ #:use-module (ice-9 match)
+ #:use-module (srfi srfi-1)
+ #:use-module (srfi srfi-9)
+ #:use-module (srfi srfi-11)
+ #:use-module (srfi srfi-37)
+ #:export (guix-lint))
+
+\f
+;;;
+;;; Command-line options.
+;;;
+
+(define %default-options
+ ;; Alist of default option values.
+ `((format . , bytevector->nix-base32-string)))
+
+(define (show-help)
+ (display (_ "Usage: guix lint [OPTION]... [PACKAGE]
+Run a set of checkers on the specified package; if none is specified, run the checkers on all packages.\n"))
+ (display (_ "
+ -h, --help display this help and exit"))
+ (display (_ "
+ -l, --list-checkers display the list of available lint checkers"))
+ (display (_ "
+ -V, --version display version information and exit"))
+ (newline)
+ (show-bug-report-information))
+
+(define %options
+ ;; Specification of the command-line options.
+ ;; TODO: add some options:
+ ;; * --checkers=checker1,checker2...: only run the specified checkers
+ ;; * --certainty=[low,medium,high]: only run checkers that have at least this
+ ;; 'certainty'.
+ (list (option '(#\h "help") #f #f
+ (lambda args
+ (show-help)
+ (exit 0)))
+ (option '(#\l "list-checkers") #f #f
+ (lambda args
+ (list-checkers-and-exit)))
+ (option '(#\V "version") #f #f
+ (lambda args
+ (show-version-and-exit "guix lint")))))
+
+\f
+;;;
+;;; Helpers
+;;;
+(define (emit-warning package s)
+ (format #t "~a: ~a~%" (package-full-name package) s))
+
+\f
+;;;
+;;; Checkers
+;;;
+(define-record-type* <lint-checker>
+ lint-checker make-lint-checker
+ lint-checker?
+ ;; TODO: add a 'certainty' field that shows how confident we are in the
+ ;; checker. Then allow users to only run checkers that have a certain
+ ;; 'certainty' level.
+ (name lint-checker-name)
+ (description lint-checker-description)
+ (check lint-checker-check))
+
+(define (list-checkers-and-exit)
+ (format #t "Available checkers:~%")
+ (for-each (lambda (checker)
+ (format #t "- ~a: ~a~%"
+ (lint-checker-name checker)
+ (lint-checker-description checker)))
+ %checkers)
+ (exit 0))
+
+(define (check-inputs-should-be-native package)
+ (let ((inputs (package-inputs package)))
+ (if (member "pkg-config" (map car inputs))
+ (emit-warning package "pkg-config should probably be a native input"))))
+
+(define (check-synopsis-style package)
+ (define (check-final-period synopsis)
+ (if (string=? (string-take-right synopsis 1) ".")
+ (emit-warning package
+ "No period allowed at the end of the synopsis.")))
+
+ (define (check-start-article synopsis)
+ (if (or (string=? (string-take synopsis 2) "A ")
+ (string=? (string-take synopsis 3) "An "))
+ (emit-warning package
+ "No article allowed at the beginning of the synopsis.")))
+
+ (let ((synopsis (package-synopsis package)))
+ (if (string? synopsis)
+ (begin
+ (check-final-period synopsis)
+ (check-start-article synopsis)))))
+
+(define (check-patches package)
+ (let ((patches (and=> (package-source package) origin-patches))
+ (name (package-name package))
+ (full-name (package-full-name package)))
+ (if (and patches
+ (any (lambda (patch)
+ (let ((filename (basename patch)))
+ (not (or (eq? (string-contains filename name) 0)
+ (eq? (string-contains filename full-name) 0)))))
+ patches))
+ (emit-warning package
+ "Filenames of patches should start by the package name"))))
+
+(define %checkers
+ (list
+ (make-lint-checker "inputs-should-be-native"
+ "Identify inputs that should be native inputs"
+ check-inputs-should-be-native)
+ (make-lint-checker "patch-filenames"
+ "Validate filenames of patches"
+ check-patches)
+ (make-lint-checker "synopsis"
+ "Validate package synopsis"
+ check-synopsis-style)))
+
+(define (run-checkers package)
+ (for-each (lambda (checker)
+ ((lint-checker-check checker) package))
+ %checkers))
+
+\f
+;;;
+;;; Entry Point
+;;;
+
+(define (guix-lint . args)
+ (define (parse-options)
+ ;; Return the alist of option values.
+ (args-fold* args %options
+ (lambda (opt name arg result)
+ (leave (_ "~A: unrecognized option~%") name))
+ (lambda (arg result)
+ (alist-cons 'argument arg result))
+ %default-options))
+
+ (let* ((opts (parse-options))
+ (args (filter-map (match-lambda
+ (('argument . value)
+ value)
+ (_ #f))
+ (reverse opts)))
+ (fmt (assq-ref opts 'format)))
+
+ (if (null? args)
+ (fold-packages (lambda (p r) (run-checkers p)) '())
+ (for-each (lambda (name)
+ (let*-values (((name version)
+ (package-name->name+version name)))
+ (let ((packages (find-packages-by-name name version)))
+ (case (length packages)
+ ((0) (format #t "No such package")); XXX
+ ((1) (run-checkers (car packages)))
+ (else (format #t
+ "More than one package found, be more precise")))))); XXX
+ args))))
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] gnu/packages: Remove trailing periods in some synopses.
2014-07-21 23:51 [PATCH 0/2] Add "guix lint" Cyril Roelandt
2014-07-21 23:51 ` [PATCH 1/2] scripts: add guix lint Cyril Roelandt
@ 2014-07-21 23:51 ` Cyril Roelandt
2014-07-22 19:27 ` [PATCH 0/2] Add "guix lint" Andreas Enge
2 siblings, 0 replies; 15+ messages in thread
From: Cyril Roelandt @ 2014-07-21 23:51 UTC (permalink / raw)
To: guix-devel
* gnu/packages/fontutils.scm (fontconfig): remove trailing period in synopsis.
* gnu/packages/gnome.scm (gtkglext): Ditto.
* gnu/packages/lua.scm (lua): Ditto.
* gnu/packages/pdf.scm (xpdf): Ditto.
* gnu/packages/python.scm (python-pytz, python2-pyicu): Ditto.
* gnu/packages/sdl.scm (libmikmod): Ditto.
---
gnu/packages/fontutils.scm | 4 ++--
gnu/packages/gnome.scm | 2 +-
gnu/packages/lua.scm | 2 +-
gnu/packages/pdf.scm | 2 +-
gnu/packages/python.scm | 4 ++--
gnu/packages/sdl.scm | 2 +-
6 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/gnu/packages/fontutils.scm b/gnu/packages/fontutils.scm
index d17843e..055f94b 100644
--- a/gnu/packages/fontutils.scm
+++ b/gnu/packages/fontutils.scm
@@ -87,7 +87,7 @@ anti-aliased glyph bitmap generation with 256 gray levels.")
(string-append "--with-add-fonts="
(assoc-ref %build-inputs "gs-fonts")
"/share/fonts"))))
- (synopsis "Fontconfig, a library for configuring and customising font access.")
+ (synopsis "Fontconfig, a library for configuring and customising font access")
(description
"Fontconfig can discover new fonts when installed automatically;
perform font name substitution, so that appropriate alternative fonts can
@@ -118,7 +118,7 @@ high quality, anti-aliased and subpixel rendered text on a display.")
`(#:tests? #f ; no test target
#:make-flags
'("without_doc")))
- (synopsis "T1lib, a library for generating bitmaps from type 1 fonts.")
+ (synopsis "T1lib, a library for generating bitmaps from type 1 fonts")
(description
"T1lib is a library for generating/rasterising bitmaps from Type 1 fonts.
It is based on the code of the X11 rasteriser of the X11 project.
diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
index 893c3e8..043fd1c 100644
--- a/gnu/packages/gnome.scm
+++ b/gnu/packages/gnome.scm
@@ -483,7 +483,7 @@ the API")
("glib" ,glib "bin")))
(propagated-inputs `(("pangox-compat" ,pangox-compat)))
(home-page "https://projects.gnome.org/gtkglext")
- (synopsis "OpenGL extension to GTK+.")
+ (synopsis "OpenGL extension to GTK+")
(description "GtkGLExt is an OpenGL extension to GTK+. It provides
additional GDK objects which support OpenGL rendering in GTK+ and GtkWidget
API add-ons to make GTK+ widgets OpenGL-capable.")
diff --git a/gnu/packages/lua.scm b/gnu/packages/lua.scm
index a85c120..84d32a0 100644
--- a/gnu/packages/lua.scm
+++ b/gnu/packages/lua.scm
@@ -57,7 +57,7 @@
"/share/man/man1")))))
(alist-delete 'configure %standard-phases)))))
(home-page "http://www.lua.org/")
- (synopsis "An embeddable scripting language.")
+ (synopsis "An embeddable scripting language")
(description
"Lua is a powerful, fast, lightweight, embeddable scripting language. Lua
combines simple procedural syntax with powerful data description constructs
diff --git a/gnu/packages/pdf.scm b/gnu/packages/pdf.scm
index 73c5f06..bbdc2cb 100644
--- a/gnu/packages/pdf.scm
+++ b/gnu/packages/pdf.scm
@@ -121,7 +121,7 @@
(string-append gs-fonts "/share/fonts/type1/ghostscript"))
(("#fontFile") "fontFile"))))
%standard-phases)))
- (synopsis "Viewer for pdf files based on the Motif toolkit.")
+ (synopsis "Viewer for pdf files based on the Motif toolkit")
(description
"Xpdf is a viewer for Portable Document Format (PDF) files")
(license license:gpl3) ; or gpl2, but not gpl2+
diff --git a/gnu/packages/python.scm b/gnu/packages/python.scm
index 641a5df..8a5ca87 100644
--- a/gnu/packages/python.scm
+++ b/gnu/packages/python.scm
@@ -256,7 +256,7 @@ data types.")
(build-system python-build-system)
(arguments `(#:tests? #f)) ; no test target
(home-page "https://launchpad.net/pytz")
- (synopsis "The Python timezone library.")
+ (synopsis "The Python timezone library")
(description
"This library allows accurate and cross platform timezone calculations
using Python 2.4 or higher and provides access to the Olson timezone database.")
@@ -715,7 +715,7 @@ Python 3.3+.")
#:tests? #f)) ; no check target
(home-page "http://pyicu.osafoundation.org/")
(synopsis
- "Python extension wrapping the ICU C++ API.")
+ "Python extension wrapping the ICU C++ API")
(description
"PyICU is a python extension wrapping the ICU C++ API.")
(license x11)))
diff --git a/gnu/packages/sdl.scm b/gnu/packages/sdl.scm
index 110cf52..02e9c29 100644
--- a/gnu/packages/sdl.scm
+++ b/gnu/packages/sdl.scm
@@ -104,7 +104,7 @@ joystick, and graphics hardware.")
(build-system gnu-build-system)
(inputs `(("alsa-lib" ,alsa-lib)
("libx11" ,libx11)))
- (synopsis "Library for module sound formats.")
+ (synopsis "Library for module sound formats")
(description
"MikMod is able to play a wide range of module formats, as well as
digital sound files. It can take advantage of particular features of your
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] scripts: add guix lint
2014-07-21 23:51 ` [PATCH 1/2] scripts: add guix lint Cyril Roelandt
@ 2014-07-22 9:03 ` Ludovic Courtès
2014-07-22 13:38 ` Ludovic Courtès
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Ludovic Courtès @ 2014-07-22 9:03 UTC (permalink / raw)
To: Cyril Roelandt; +Cc: guix-devel
Cyril Roelandt <tipecaml@gmail.com> skribis:
> * guix/scripts/lint.scm: New file. Defines a 'lint' tool for Guix packages.
> * Makefile.am (MODULES): Add it.
This is nice!
I think this should be modular so that, when possible, checkers can also
be invoked at macro expansion time. For instance, style checkers for
synopses could run just when you build the file, without even having to
invoke ‘guix lint’ (but it’s nice to have both possibilities.)
Anyway, that’s a very good start, so the comment above is mostly food
for thought.
> +(define %default-options
> + ;; Alist of default option values.
> + `((format . , bytevector->nix-base32-string)))
Should be just '().
> +(define (emit-warning package s)
> + (format #t "~a: ~a~%" (package-full-name package) s))
There should be two indentation spaces, not one (the same problem shows
up in other places.) Could you check that?
Besides, please and use ‘warning’ from (guix ui).
Also, it would be great to report location information, so that the
editor can directly jump to the offending line. See how
build-aux/sync-descriptions.scm does it.
> +(define (list-checkers-and-exit)
> + (format #t "Available checkers:~%")
Should be (_ "Available checkers:~%").
> + (for-each (lambda (checker)
> + (format #t "- ~a: ~a~%"
> + (lint-checker-name checker)
> + (lint-checker-description checker)))
Likewise, (gettext (lint-checker-description checker)).
This file also needs to be added to po/guix/POTFILES.in, and
po/guix/Makevars needs a --keyword=description as well.
> +(define (check-inputs-should-be-native package)
> + (let ((inputs (package-inputs package)))
Please add a docstring to this and other top-level procedures.
> + (if (member "pkg-config" (map car inputs))
> + (emit-warning package "pkg-config should probably be a native input"))))
Please avoid looking at the ‘car’ of inputs, since it’s just a label.
Also, prefer ‘when’ to one-arm ‘if’.
So that should look like:
(match inputs
(((labels packages . _) ...)
(when (member "pkg-config" (map package-name packages))
(emit-warning ...))))
> +(define (check-synopsis-style package)
> + (define (check-final-period synopsis)
> + (if (string=? (string-take-right synopsis 1) ".")
> + (emit-warning package
> + "No period allowed at the end of the synopsis.")))
The warning message should be lower-case, with no period.
> + (emit-warning package
> + "Filenames of patches should start by the package name"))))
“file names”, “start with”.
> +(define %checkers
> + (list
> + (make-lint-checker "inputs-should-be-native"
> + "Identify inputs that should be native inputs"
> + check-inputs-should-be-native)
> + (make-lint-checker "patch-filenames"
> + "Validate filenames of patches"
> + check-patches)
> + (make-lint-checker "synopsis"
> + "Validate package synopsis"
> + check-synopsis-style)))
Instead of ‘make-lint-checker’, use
(lint-checker (name ...) (description "foo") ...)
This will allow xgettext to extract all the ‘description’ things, as
mentioned above.
> +(define (guix-lint . args)
> + (define (parse-options)
> + ;; Return the alist of option values.
> + (args-fold* args %options
> + (lambda (opt name arg result)
> + (leave (_ "~A: unrecognized option~%") name))
> + (lambda (arg result)
> + (alist-cons 'argument arg result))
> + %default-options))
> +
> + (let* ((opts (parse-options))
> + (args (filter-map (match-lambda
> + (('argument . value)
> + value)
> + (_ #f))
> + (reverse opts)))
> + (fmt (assq-ref opts 'format)))
‘fmt’ is unused, can be removed.
> + (if (null? args)
> + (fold-packages (lambda (p r) (run-checkers p)) '())
> + (for-each (lambda (name)
> + (let*-values (((name version)
> + (package-name->name+version name)))
> + (let ((packages (find-packages-by-name name version)))
> + (case (length packages)
> + ((0) (format #t "No such package")); XXX
> + ((1) (run-checkers (car packages)))
> + (else (format #t
> + "More than one package found, be more precise")))))); XXX
Please move ‘specification->package’ from (guix build scripts) to (guix
ui), and then use it instead of the above.
Thanks!
Ludo’.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] scripts: add guix lint
2014-07-22 9:03 ` Ludovic Courtès
@ 2014-07-22 13:38 ` Ludovic Courtès
2014-07-22 14:31 ` Eric Bavier
2014-08-25 1:52 ` [PATCH] " Cyril Roelandt
2 siblings, 0 replies; 15+ messages in thread
From: Ludovic Courtès @ 2014-07-22 13:38 UTC (permalink / raw)
To: Cyril Roelandt; +Cc: guix-devel
Two things I forgot to ask for (you’ll hate me for that ;-)):
1. Could you write tests for the checkers, as tests/lint.scm?
The tests would make sure that for a ‘package’ structure that
exposes the problem being checked for, the checker does emit a
warning on ‘guix-warning-port’, like:
(define (call-with-warnings thunk)
(let ((port (open-output-string)))
(parameterize ((guix-warning-port port))
(thunk))
(get-output-string port)))
(test-assert "synopsis warning emitted"
(->bool
(string-contains (call-with-warnings
(lambda ()
(check-foo-bar pkg)))
"synopsis has a problem")))
2. Add an “Invoking guix lint” node in guix.texi, with links to there
from “Defining Packages”.
TIA. :-)
Ludo’.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] scripts: add guix lint
2014-07-22 9:03 ` Ludovic Courtès
2014-07-22 13:38 ` Ludovic Courtès
@ 2014-07-22 14:31 ` Eric Bavier
2014-07-22 15:26 ` Ludovic Courtès
2014-08-25 1:52 ` [PATCH] " Cyril Roelandt
2 siblings, 1 reply; 15+ messages in thread
From: Eric Bavier @ 2014-07-22 14:31 UTC (permalink / raw)
To: Cyril Roelandt; +Cc: guix-devel
Ludovic Courtès writes:
> Cyril Roelandt <tipecaml@gmail.com> skribis:
>
>> * guix/scripts/lint.scm: New file. Defines a 'lint' tool for Guix packages.
>> * Makefile.am (MODULES): Add it.
>
> This is nice!
>
> I think this should be modular so that, when possible, checkers can also
> be invoked at macro expansion time. For instance, style checkers for
> synopses could run just when you build the file, without even having to
> invoke ‘guix lint’ (but it’s nice to have both possibilities.)
I like this too.
Would it make sense to have 'guix lint' operate at the file/module level
instead of at the package level? That would be consistent with other
lint-type tools, as far as I know. It would also, I think, help with
things like syntax errors that currently result in 'guix build'
reporting something like "guix build: error: foo: unknown package".
--
Eric Bavier
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] scripts: add guix lint
2014-07-22 14:31 ` Eric Bavier
@ 2014-07-22 15:26 ` Ludovic Courtès
0 siblings, 0 replies; 15+ messages in thread
From: Ludovic Courtès @ 2014-07-22 15:26 UTC (permalink / raw)
To: Eric Bavier; +Cc: guix-devel
Eric Bavier <ericbavier@gmail.com> skribis:
> Would it make sense to have 'guix lint' operate at the file/module level
> instead of at the package level?
You really mean at the syntactic level, no?
> That would be consistent with other lint-type tools, as far as I know.
> It would also, I think, help with things like syntax errors that
> currently result in 'guix build' reporting something like "guix build:
> error: foo: unknown package".
Normally you get more details about that when typing ‘make’ (or C-c C-k
in Geiser.)
Detecting that would amount to doing the job of the compiler and module
system, basically.
Ludo’.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Add "guix lint".
2014-07-21 23:51 [PATCH 0/2] Add "guix lint" Cyril Roelandt
2014-07-21 23:51 ` [PATCH 1/2] scripts: add guix lint Cyril Roelandt
2014-07-21 23:51 ` [PATCH 2/2] gnu/packages: Remove trailing periods in some synopses Cyril Roelandt
@ 2014-07-22 19:27 ` Andreas Enge
2014-07-22 20:25 ` Ludovic Courtès
2 siblings, 1 reply; 15+ messages in thread
From: Andreas Enge @ 2014-07-22 19:27 UTC (permalink / raw)
To: Cyril Roelandt; +Cc: guix-devel
This looks interesting, but similarly to some comment about "guix package
--show", I am wondering about the profusion of commands. Maybe we should use
guix devel --lint
instead? It would make clear that this option is only of interest to
developers, and pave the way for more developer related functionalities.
Andreas
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Add "guix lint".
2014-07-22 19:27 ` [PATCH 0/2] Add "guix lint" Andreas Enge
@ 2014-07-22 20:25 ` Ludovic Courtès
0 siblings, 0 replies; 15+ messages in thread
From: Ludovic Courtès @ 2014-07-22 20:25 UTC (permalink / raw)
To: Andreas Enge; +Cc: guix-devel
Andreas Enge <andreas@enge.fr> skribis:
> This looks interesting, but similarly to some comment about "guix package
> --show", I am wondering about the profusion of commands.
I think --show may be useful to non-developers.
> Maybe we should use
> guix devel --lint
> instead? It would make clear that this option is only of interest to
> developers, and pave the way for more developer related functionalities.
The problem is that things like ‘guix hash’, ‘guix refresh’, and ‘guix
lint’ already all have their own set of options. So bundling them
together in a single sub-command would be inconvenient, at best.
However, I sympathize with the idea of distinguishing between developer
and non-developer functionality.
There are two ways I tried to make this apparent: by explicitly stating
it in the manual, and by hiding some sub-commands from the output of
‘guix --help’. Perhaps we could do more of the latter? How well does
that help?
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] scripts: add guix lint
2014-07-22 9:03 ` Ludovic Courtès
2014-07-22 13:38 ` Ludovic Courtès
2014-07-22 14:31 ` Eric Bavier
@ 2014-08-25 1:52 ` Cyril Roelandt
2014-08-25 22:44 ` Ludovic Courtès
2 siblings, 1 reply; 15+ messages in thread
From: Cyril Roelandt @ 2014-08-25 1:52 UTC (permalink / raw)
To: guix-devel
* guix/scripts/lint.scm: New file. Defines a 'lint' tool for Guix packages.
* tests/lint.scm: New file.
* Makefile.am (MODULES, SCM_TESTS): Add them.
* guix/scripts/build.scm (specification->package): Move from here...
* guix/ui.scm: ... to here.
* po/guix/Makevars: Update appropriately.
* po/guix/POTFILES.in: Update appropriately.
* doc/guix.texi: Document "guix lint".
---
Makefile.am | 4 +-
doc/guix.texi | 27 +++++++
guix/scripts/build.scm | 21 -----
guix/scripts/lint.scm | 213 +++++++++++++++++++++++++++++++++++++++++++++++++
guix/ui.scm | 23 ++++++
po/guix/Makevars | 3 +-
po/guix/POTFILES.in | 1 +
tests/lint.scm | 110 +++++++++++++++++++++++++
8 files changed, 379 insertions(+), 23 deletions(-)
create mode 100644 guix/scripts/lint.scm
create mode 100644 tests/lint.scm
diff --git a/Makefile.am b/Makefile.am
index fff5958..371b85c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -89,6 +89,7 @@ MODULES = \
guix/scripts/authenticate.scm \
guix/scripts/refresh.scm \
guix/scripts/system.scm \
+ guix/scripts/lint.scm \
guix.scm \
$(GNU_SYSTEM_MODULES)
@@ -159,7 +160,8 @@ SCM_TESTS = \
tests/nar.scm \
tests/union.scm \
tests/profiles.scm \
- tests/syscalls.scm
+ tests/syscalls.scm \
+ tests/lint.scm
SH_TESTS = \
tests/guix-build.sh \
diff --git a/doc/guix.texi b/doc/guix.texi
index 09ed392..9a43543 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -1459,6 +1459,10 @@ package may actually be built using the @code{guix build} command-line
tool (@pxref{Invoking guix build}). @xref{Packaging Guidelines}, for
more information on how to test package definitions.
+In order to catch common errors and stylistic issues, the packages might
+actually be checked by running the @command{guix lint} command
+(@pxref{Invoking guix lint}).
+
Eventually, updating the package definition to a new upstream version
can be partly automated by the @command{guix refresh} command
(@pxref{Invoking guix refresh}).
@@ -2326,6 +2330,7 @@ programming interface of Guix in a convenient way.
* Invoking guix download:: Downloading a file and printing its hash.
* Invoking guix hash:: Computing the cryptographic hash of a file.
* Invoking guix refresh:: Updating package definitions.
+* Invoking guix lint:: Finding errors in package definitions.
@end menu
@node Invoking guix build
@@ -2703,6 +2708,28 @@ for in @code{$PATH}.
@end table
+@node Invoking guix lint
+@section Invoking @command{guix lint}
+The @command{guix lint} command runs a few checks on a given set of
+packages in order to find common mistakes in their definitions.
+
+The general syntax is:
+
+@example
+guix lint @var{options} @var{package}@dots{}
+@end example
+
+If no package is given on the command line, then all packages are checked.
+The @var{options} may be zero or more of the following:
+
+@table @code
+
+@item --list-checkers
+@itemx -l
+List and describe all the available checkers that will be run on packages
+and exit.
+
+@end table
@c *********************************************************************
@node GNU Distribution
diff --git a/guix/scripts/build.scm b/guix/scripts/build.scm
index 5e4647d..b2682a1 100644
--- a/guix/scripts/build.scm
+++ b/guix/scripts/build.scm
@@ -41,27 +41,6 @@
guix-build))
-(define (specification->package spec)
- "Return a package matching SPEC. SPEC may be a package name, or a package
-name followed by a hyphen and a version number. If the version number is not
-present, return the preferred newest version."
- (let-values (((name version)
- (package-name->name+version spec)))
- (match (find-best-packages-by-name name version)
- ((p) ; one match
- p)
- ((p x ...) ; several matches
- (warning (_ "ambiguous package specification `~a'~%") spec)
- (warning (_ "choosing ~a from ~a~%")
- (package-full-name p)
- (location->string (package-location p)))
- p)
- (_ ; no matches
- (if version
- (leave (_ "~A: package not found for version ~a~%")
- name version)
- (leave (_ "~A: unknown package~%") name))))))
-
(define (register-root store paths root)
"Register ROOT as an indirect GC root for all of PATHS."
(let* ((root (string-append (canonicalize-path (dirname root))
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
new file mode 100644
index 0000000..e3b0697
--- /dev/null
+++ b/guix/scripts/lint.scm
@@ -0,0 +1,213 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2014 Cyril Roelandt <tipecaml@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (guix scripts lint)
+ #:use-module (guix base32)
+ #:use-module (guix packages)
+ #:use-module (guix records)
+ #:use-module (guix ui)
+ #:use-module (guix utils)
+ #:use-module (gnu packages)
+ #:use-module (ice-9 match)
+ #:use-module (srfi srfi-1)
+ #:use-module (srfi srfi-9)
+ #:use-module (srfi srfi-11)
+ #:use-module (srfi srfi-37)
+ #:export (guix-lint
+ check-inputs-should-be-native
+ check-patches
+ check-synopsis-style))
+
+\f
+;;;
+;;; Command-line options.
+;;;
+
+(define %default-options
+ ;; Alist of default option values.
+ '())
+
+(define (show-help)
+ (display (_ "Usage: guix lint [OPTION]... [PACKAGE]...
+Run a set of checkers on the specified package; if none is specified, run the checkers on all packages.\n"))
+ (display (_ "
+ -h, --help display this help and exit"))
+ (display (_ "
+ -l, --list-checkers display the list of available lint checkers"))
+ (display (_ "
+ -V, --version display version information and exit"))
+ (newline)
+ (show-bug-report-information))
+
+(define %options
+ ;; Specification of the command-line options.
+ ;; TODO: add some options:
+ ;; * --checkers=checker1,checker2...: only run the specified checkers
+ ;; * --certainty=[low,medium,high]: only run checkers that have at least this
+ ;; 'certainty'.
+ (list (option '(#\h "help") #f #f
+ (lambda args
+ (show-help)
+ (exit 0)))
+ (option '(#\l "list-checkers") #f #f
+ (lambda args
+ (list-checkers-and-exit)))
+ (option '(#\V "version") #f #f
+ (lambda args
+ (show-version-and-exit "guix lint")))))
+
+\f
+;;;
+;;; Helpers
+;;;
+(define* (emit-warning package message #:optional field)
+ ;; Emit a warning about PACKAGE, printing the location of FIELD if it is
+ ;; given, the location of PACKAGE otherwise, the full name of PACKAGE and the
+ ;; provided MESSAGE.
+ (let ((loc (or (package-field-location package field)
+ (package-location package))))
+ (warning (_ "~a: ~a: ~a~%")
+ (location->string loc)
+ (package-full-name package)
+ message)))
+
+\f
+;;;
+;;; Checkers
+;;;
+(define-record-type* <lint-checker>
+ lint-checker make-lint-checker
+ lint-checker?
+ ;; TODO: add a 'certainty' field that shows how confident we are in the
+ ;; checker. Then allow users to only run checkers that have a certain
+ ;; 'certainty' level.
+ (name lint-checker-name)
+ (description lint-checker-description)
+ (check lint-checker-check))
+
+(define (list-checkers-and-exit)
+ ;; Print information about all available checkers and exit.
+ (format #t (_ "Available checkers:~%"))
+ (for-each (lambda (checker)
+ (format #t "- ~a: ~a~%"
+ (lint-checker-name checker)
+ (lint-checker-description checker)))
+ %checkers)
+ (exit 0))
+
+(define (check-inputs-should-be-native package)
+ ;; Emit a warning if some inputs of PACKAGE are likely to belong to its
+ ;; native inputs.
+ (let ((inputs (package-inputs package)))
+ (match inputs
+ (((labels packages . _) ...)
+ (when (member "pkg-config"
+ (map package-name (filter package? packages)))
+ (emit-warning package
+ "pkg-config should probably be a native input"
+ 'inputs))))))
+
+
+(define (check-synopsis-style package)
+ ;; Emit a warning if stylistic issues are found in the synopsis of PACKAGE.
+ (define (check-final-period synopsis)
+ ;; Synopsis should not end with a period, except for some special cases.
+ (if (and (string=? (string-take-right synopsis 1) ".")
+ (not (string=? (string-take-right synopsis 4) "etc.")))
+ (emit-warning package
+ "no period allowed at the end of the synopsis"
+ 'synopsis)))
+
+ (define (check-start-article synopsis)
+ (if (or (string=? (string-take synopsis 2) "A ")
+ (string=? (string-take synopsis 3) "An "))
+ (emit-warning package
+ "no article allowed at the beginning of the synopsis"
+ 'synopsis)))
+
+ (let ((synopsis (package-synopsis package)))
+ (if (string? synopsis)
+ (begin
+ (check-final-period synopsis)
+ (check-start-article synopsis)))))
+
+(define (check-patches package)
+ ;; Emit a warning if the patches requires by PACKAGE are badly named.
+ (let ((patches (and=> (package-source package) origin-patches))
+ (name (package-name package))
+ (full-name (package-full-name package)))
+ (if (and patches
+ (any (lambda (patch)
+ (let ((filename (basename patch)))
+ (not (or (eq? (string-contains filename name) 0)
+ (eq? (string-contains filename full-name) 0)))))
+ patches))
+ (emit-warning package
+ "file names of patches should start with the package name"
+ 'patches))))
+
+(define %checkers
+ (list
+ (lint-checker
+ (name "inputs-should-be-native")
+ (description "Identify inputs that should be native inputs")
+ (check check-inputs-should-be-native))
+ (lint-checker
+ (name "patch-filenames")
+ (description "Validate filenames of patches")
+ (check check-patches))
+ (lint-checker
+ (name "synopsis")
+ (description "Validate package synopsis")
+ (check check-synopsis-style))))
+
+(define (run-checkers package)
+ ;; Run all the checkers on PACKAGE.
+ (for-each (lambda (checker)
+ ((lint-checker-check checker) package))
+ %checkers))
+
+\f
+;;;
+;;; Entry Point
+;;;
+
+(define (guix-lint . args)
+ (define (parse-options)
+ ;; Return the alist of option values.
+ (args-fold* args %options
+ (lambda (opt name arg result)
+ (leave (_ "~A: unrecognized option~%") name))
+ (lambda (arg result)
+ (alist-cons 'argument arg result))
+ %default-options))
+
+ (let* ((opts (parse-options))
+ (args (filter-map (match-lambda
+ (('argument . value)
+ value)
+ (_ #f))
+ (reverse opts))))
+
+
+ (if (null? args)
+ (fold-packages (lambda (p r) (run-checkers p)) '())
+ (for-each
+ (lambda (spec)
+ (run-checkers spec))
+ (map specification->package args)))))
diff --git a/guix/ui.scm b/guix/ui.scm
index f11c2e9..86bd8f7 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -37,6 +37,7 @@
#:use-module (ice-9 match)
#:use-module (ice-9 format)
#:use-module (ice-9 regex)
+ #:autoload (gnu packages) (find-best-packages-by-name)
#:export (_
N_
P_
@@ -64,6 +65,7 @@
program-name
guix-warning-port
warning
+ specification->package
guix-main))
;;; Commentary:
@@ -669,6 +671,27 @@ found."
(define guix-warning-port
(make-parameter (current-warning-port)))
+(define (specification->package spec)
+ "Return a package matching SPEC. SPEC may be a package name, or a package
+name followed by a hyphen and a version number. If the version number is not
+present, return the preferred newest version."
+ (let-values (((name version)
+ (package-name->name+version spec)))
+ (match (find-best-packages-by-name name version)
+ ((p) ; one match
+ p)
+ ((p x ...) ; several matches
+ (warning (_ "ambiguous package specification `~a'~%") spec)
+ (warning (_ "choosing ~a from ~a~%")
+ (package-full-name p)
+ (location->string (package-location p)))
+ p)
+ (_ ; no matches
+ (if version
+ (leave (_ "~A: package not found for version ~a~%")
+ name version)
+ (leave (_ "~A: unknown package~%") name))))))
+
(define (guix-main arg0 . args)
(initialize-guix)
(let ()
diff --git a/po/guix/Makevars b/po/guix/Makevars
index 87bb438..f5b498c 100644
--- a/po/guix/Makevars
+++ b/po/guix/Makevars
@@ -10,7 +10,8 @@ top_builddir = ../..
XGETTEXT_OPTIONS = \
--language=Scheme --from-code=UTF-8 \
--keyword=_ --keyword=N_ \
- --keyword=message
+ --keyword=message \
+ --keyword=description
COPYRIGHT_HOLDER = Ludovic Courtès
diff --git a/po/guix/POTFILES.in b/po/guix/POTFILES.in
index bf2d313..5cc68ff 100644
--- a/po/guix/POTFILES.in
+++ b/po/guix/POTFILES.in
@@ -10,6 +10,7 @@ guix/scripts/pull.scm
guix/scripts/substitute-binary.scm
guix/scripts/authenticate.scm
guix/scripts/system.scm
+guix/scripts/lint.scm
guix/gnu-maintenance.scm
guix/ui.scm
guix/http-client.scm
diff --git a/tests/lint.scm b/tests/lint.scm
new file mode 100644
index 0000000..f6dae47
--- /dev/null
+++ b/tests/lint.scm
@@ -0,0 +1,110 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2012, 2013 Cyril Roelandt <tipecaml@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
+
+
+(define-module (test-packages)
+ #:use-module (guix build download)
+ #:use-module (guix build-system gnu)
+ #:use-module (guix packages)
+ #:use-module (guix scripts lint)
+ #:use-module (guix ui)
+ #:use-module (gnu packages)
+ #:use-module (gnu packages pkg-config)
+ #:use-module (srfi srfi-64))
+
+;; Test the linter.
+
+\f
+(test-begin "lint")
+
+(define-syntax-rule (dummy-package name* extra-fields ...)
+ (package extra-fields ... (name name*) (version "0") (source #f)
+ (build-system gnu-build-system)
+ (synopsis #f) (description #f)
+ (home-page #f) (license #f) ))
+
+(define (call-with-warnings thunk)
+ (let ((port (open-output-string)))
+ (parameterize ((guix-warning-port port))
+ (thunk))
+ (get-output-string port)))
+
+(test-assert "synopsis: ends with a period"
+ (->bool
+ (string-contains (call-with-warnings
+ (lambda ()
+ (let ((pkg (dummy-package "x"
+ (synopsis "Bad synopsis."))))
+ (check-synopsis-style pkg))))
+ "no period allowed at the end of the synopsis")))
+
+(test-assert "synopsis: ends with 'etc.'"
+ (->bool
+ (string-null? (call-with-warnings
+ (lambda ()
+ (let ((pkg (dummy-package "x"
+ (synopsis "Foo, bar, etc."))))
+ (check-synopsis-style pkg)))))))
+
+(test-assert "synopsis: starts with 'A'"
+ (->bool
+ (string-contains (call-with-warnings
+ (lambda ()
+ (let ((pkg (dummy-package "x"
+ (synopsis "A bad synopŝis"))))
+ (check-synopsis-style pkg))))
+ "no article allowed at the beginning of the synopsis")))
+
+(test-assert "synopsis: starts with 'An'"
+ (->bool
+ (string-contains (call-with-warnings
+ (lambda ()
+ (let ((pkg (dummy-package "x"
+ (synopsis "An awful synopsis"))))
+ (check-synopsis-style pkg))))
+ "no article allowed at the beginning of the synopsis")))
+
+(test-assert "inputs: pkg-config is probably a native input"
+ (->bool
+ (string-contains
+ (call-with-warnings
+ (lambda ()
+ (let ((pkg (dummy-package "x"
+ (inputs `(("pkg-config" ,pkg-config))))))
+ (check-inputs-should-be-native pkg))))
+ "pkg-config should probably be a native input")))
+
+(test-assert "patches: file names"
+ (->bool
+ (string-contains
+ (call-with-warnings
+ (lambda ()
+ (let ((pkg (dummy-package "x"
+ (source
+ (origin
+ (method url-fetch)
+ (uri "someurl")
+ (sha256 "somesha")
+ (patches (list "/path/to/y.patch")))))))
+ (check-patches pkg))))
+ "file names of patches should start with the package name")))
+
+(test-end "lint")
+
+\f
+(exit (= (test-runner-fail-count (test-runner-current)) 0))
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] scripts: add guix lint
2014-08-25 1:52 ` [PATCH] " Cyril Roelandt
@ 2014-08-25 22:44 ` Ludovic Courtès
2014-09-01 0:39 ` [PATCH 1/2] Move specification->package to gnu/packages.scm Cyril Roelandt
0 siblings, 1 reply; 15+ messages in thread
From: Ludovic Courtès @ 2014-08-25 22:44 UTC (permalink / raw)
To: Cyril Roelandt; +Cc: guix-devel
Cyril Roelandt <tipecaml@gmail.com> skribis:
> * guix/scripts/lint.scm: New file. Defines a 'lint' tool for Guix packages.
> * tests/lint.scm: New file.
> * Makefile.am (MODULES, SCM_TESTS): Add them.
> * guix/scripts/build.scm (specification->package): Move from here...
> * guix/ui.scm: ... to here.
> * po/guix/Makevars: Update appropriately.
> * po/guix/POTFILES.in: Update appropriately.
> * doc/guix.texi: Document "guix lint".
Cool!
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -1459,6 +1459,10 @@ package may actually be built using the @code{guix build} command-line
> tool (@pxref{Invoking guix build}). @xref{Packaging Guidelines}, for
> more information on how to test package definitions.
>
> +In order to catch common errors and stylistic issues, the packages might
> +actually be checked by running the @command{guix lint} command
> +(@pxref{Invoking guix lint}).
What about just making it like this:
[...] @xref{Packaging Guidelines}, for more information on how to test
package definitions, and @ref{Invoking guix lint}, for information
on how to check a definition for style conformance.
> Eventually, updating the package definition to a new upstream version
> can be partly automated by the @command{guix refresh} command
> (@pxref{Invoking guix refresh}).
> @@ -2326,6 +2330,7 @@ programming interface of Guix in a convenient way.
> * Invoking guix download:: Downloading a file and printing its hash.
> * Invoking guix hash:: Computing the cryptographic hash of a file.
> * Invoking guix refresh:: Updating package definitions.
> +* Invoking guix lint:: Finding errors in package definitions.
> @end menu
>
> @node Invoking guix build
> @@ -2703,6 +2708,28 @@ for in @code{$PATH}.
>
> @end table
>
> +@node Invoking guix lint
> +@section Invoking @command{guix lint}
> +The @command{guix lint} command runs a few checks on a given set of
> +packages in order to find common mistakes in their definitions.
To make the target audience clearer, what about this:
The @command{guix lint} is meant to help package developers avoid
common errors and use a consistent style. It runs a series of checks
on the given set of packages [...]
> -(define (specification->package spec)
> - "Return a package matching SPEC. SPEC may be a package name, or a package
> -name followed by a hyphen and a version number. If the version number is not
> -present, return the preferred newest version."
> - (let-values (((name version)
> - (package-name->name+version spec)))
> - (match (find-best-packages-by-name name version)
Apologies for giving incorrect advice in my previous review; could you
move this procedure to (gnu packages) (and not (guix ui), as I
wrongfully suggested)? Ideally in a separate commit.
The reason is to avoid introducing a dependency from (guix ui) to (gnu
packages).
OK to commit with these minor issues fixed.
Thanks!
Ludo’.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] Move specification->package to gnu/packages.scm.
2014-08-25 22:44 ` Ludovic Courtès
@ 2014-09-01 0:39 ` Cyril Roelandt
2014-09-01 0:39 ` [PATCH 2/2] scripts: add guix lint Cyril Roelandt
2014-09-01 8:55 ` [PATCH 1/2] Move specification->package to gnu/packages.scm Ludovic Courtès
0 siblings, 2 replies; 15+ messages in thread
From: Cyril Roelandt @ 2014-09-01 0:39 UTC (permalink / raw)
To: guix-devel
* guix/scripts/build.scm (specification->package): Move from here...
* gnu/packages.scm: ... to here.
---
gnu/packages.scm | 26 +++++++++++++++++++++++++-
guix/scripts/build.scm | 23 +----------------------
2 files changed, 26 insertions(+), 23 deletions(-)
diff --git a/gnu/packages.scm b/gnu/packages.scm
index 14ad755..26d87c6 100644
--- a/gnu/packages.scm
+++ b/gnu/packages.scm
@@ -28,6 +28,7 @@
#:use-module (ice-9 vlist)
#:use-module (ice-9 match)
#:use-module (srfi srfi-1)
+ #:use-module (srfi srfi-11)
#:use-module (srfi srfi-26)
#:use-module (srfi srfi-39)
#:export (search-patch
@@ -45,7 +46,9 @@
package-transitive-dependents
package-covering-dependents
- check-package-freshness))
+ check-package-freshness
+
+ specification->package))
;;; Commentary:
;;;
@@ -326,3 +329,24 @@ but ~a is available upstream~%")
(case key
((getaddrinfo-error ftp-error) #f)
(else (apply throw key args))))))
+
+(define (specification->package spec)
+ "Return a package matching SPEC. SPEC may be a package name, or a package
+name followed by a hyphen and a version number. If the version number is not
+present, return the preferred newest version."
+ (let-values (((name version)
+ (package-name->name+version spec)))
+ (match (find-best-packages-by-name name version)
+ ((p) ; one match
+ p)
+ ((p x ...) ; several matches
+ (warning (_ "ambiguous package specification `~a'~%") spec)
+ (warning (_ "choosing ~a from ~a~%")
+ (package-full-name p)
+ (location->string (package-location p)))
+ p)
+ (_ ; no matches
+ (if version
+ (leave (_ "~A: package not found for version ~a~%")
+ name version)
+ (leave (_ "~A: unknown package~%") name))))))
diff --git a/guix/scripts/build.scm b/guix/scripts/build.scm
index 5e4647d..09401e9 100644
--- a/guix/scripts/build.scm
+++ b/guix/scripts/build.scm
@@ -33,7 +33,7 @@
#:use-module (srfi srfi-26)
#:use-module (srfi srfi-34)
#:use-module (srfi srfi-37)
- #:autoload (gnu packages) (find-best-packages-by-name)
+ #:autoload (gnu packages) (specification->package)
#:autoload (guix download) (download-to-store)
#:export (%standard-build-options
set-build-options-from-command-line
@@ -41,27 +41,6 @@
guix-build))
-(define (specification->package spec)
- "Return a package matching SPEC. SPEC may be a package name, or a package
-name followed by a hyphen and a version number. If the version number is not
-present, return the preferred newest version."
- (let-values (((name version)
- (package-name->name+version spec)))
- (match (find-best-packages-by-name name version)
- ((p) ; one match
- p)
- ((p x ...) ; several matches
- (warning (_ "ambiguous package specification `~a'~%") spec)
- (warning (_ "choosing ~a from ~a~%")
- (package-full-name p)
- (location->string (package-location p)))
- p)
- (_ ; no matches
- (if version
- (leave (_ "~A: package not found for version ~a~%")
- name version)
- (leave (_ "~A: unknown package~%") name))))))
-
(define (register-root store paths root)
"Register ROOT as an indirect GC root for all of PATHS."
(let* ((root (string-append (canonicalize-path (dirname root))
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] scripts: add guix lint
2014-09-01 0:39 ` [PATCH 1/2] Move specification->package to gnu/packages.scm Cyril Roelandt
@ 2014-09-01 0:39 ` Cyril Roelandt
2014-09-01 21:19 ` Ludovic Courtès
2014-09-01 8:55 ` [PATCH 1/2] Move specification->package to gnu/packages.scm Ludovic Courtès
1 sibling, 1 reply; 15+ messages in thread
From: Cyril Roelandt @ 2014-09-01 0:39 UTC (permalink / raw)
To: guix-devel
* guix/scripts/lint.scm: New file. Defines a 'lint' tool for Guix packages.
* tests/lint.scm: New file.
* Makefile.am (MODULES, SCM_TESTS): Add them.
* po/guix/Makevars: Update appropriately.
* po/guix/POTFILES.in: Update appropriately.
* doc/guix.texi: Document "guix lint".
---
Makefile.am | 4 +-
doc/guix.texi | 29 ++++++-
guix/scripts/lint.scm | 213 ++++++++++++++++++++++++++++++++++++++++++++++++++
po/guix/Makevars | 3 +-
po/guix/POTFILES.in | 1 +
tests/lint.scm | 110 ++++++++++++++++++++++++++
6 files changed, 357 insertions(+), 3 deletions(-)
create mode 100644 guix/scripts/lint.scm
create mode 100644 tests/lint.scm
diff --git a/Makefile.am b/Makefile.am
index fff5958..371b85c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -89,6 +89,7 @@ MODULES = \
guix/scripts/authenticate.scm \
guix/scripts/refresh.scm \
guix/scripts/system.scm \
+ guix/scripts/lint.scm \
guix.scm \
$(GNU_SYSTEM_MODULES)
@@ -159,7 +160,8 @@ SCM_TESTS = \
tests/nar.scm \
tests/union.scm \
tests/profiles.scm \
- tests/syscalls.scm
+ tests/syscalls.scm \
+ tests/lint.scm
SH_TESTS = \
tests/guix-build.sh \
diff --git a/doc/guix.texi b/doc/guix.texi
index e34a70f..748480a 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -1459,7 +1459,10 @@ definitions like the one above may be automatically converted from the
Nixpkgs distribution using the @command{guix import} command.}, the
package may actually be built using the @code{guix build} command-line
tool (@pxref{Invoking guix build}). @xref{Packaging Guidelines}, for
-more information on how to test package definitions.
+more information on how to test package definitions, and
+@ref{Invoking guix lint}, for information on how to check a definition
+for style conformance.
+
Eventually, updating the package definition to a new upstream version
can be partly automated by the @command{guix refresh} command
@@ -2328,6 +2331,7 @@ programming interface of Guix in a convenient way.
* Invoking guix download:: Downloading a file and printing its hash.
* Invoking guix hash:: Computing the cryptographic hash of a file.
* Invoking guix refresh:: Updating package definitions.
+* Invoking guix lint:: Finding errors in package definitions.
@end menu
@node Invoking guix build
@@ -2705,6 +2709,29 @@ for in @code{$PATH}.
@end table
+@node Invoking guix lint
+@section Invoking @command{guix lint}
+The @command{guix lint} is meant to help package developers avoid common
+errors and use a consistent style. It runs a few checks on a given set of
+packages in order to find common mistakes in their definitions.
+
+The general syntax is:
+
+@example
+guix lint @var{options} @var{package}@dots{}
+@end example
+
+If no package is given on the command line, then all packages are checked.
+The @var{options} may be zero or more of the following:
+
+@table @code
+
+@item --list-checkers
+@itemx -l
+List and describe all the available checkers that will be run on packages
+and exit.
+
+@end table
@c *********************************************************************
@node GNU Distribution
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
new file mode 100644
index 0000000..e3b0697
--- /dev/null
+++ b/guix/scripts/lint.scm
@@ -0,0 +1,213 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2014 Cyril Roelandt <tipecaml@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (guix scripts lint)
+ #:use-module (guix base32)
+ #:use-module (guix packages)
+ #:use-module (guix records)
+ #:use-module (guix ui)
+ #:use-module (guix utils)
+ #:use-module (gnu packages)
+ #:use-module (ice-9 match)
+ #:use-module (srfi srfi-1)
+ #:use-module (srfi srfi-9)
+ #:use-module (srfi srfi-11)
+ #:use-module (srfi srfi-37)
+ #:export (guix-lint
+ check-inputs-should-be-native
+ check-patches
+ check-synopsis-style))
+
+\f
+;;;
+;;; Command-line options.
+;;;
+
+(define %default-options
+ ;; Alist of default option values.
+ '())
+
+(define (show-help)
+ (display (_ "Usage: guix lint [OPTION]... [PACKAGE]...
+Run a set of checkers on the specified package; if none is specified, run the checkers on all packages.\n"))
+ (display (_ "
+ -h, --help display this help and exit"))
+ (display (_ "
+ -l, --list-checkers display the list of available lint checkers"))
+ (display (_ "
+ -V, --version display version information and exit"))
+ (newline)
+ (show-bug-report-information))
+
+(define %options
+ ;; Specification of the command-line options.
+ ;; TODO: add some options:
+ ;; * --checkers=checker1,checker2...: only run the specified checkers
+ ;; * --certainty=[low,medium,high]: only run checkers that have at least this
+ ;; 'certainty'.
+ (list (option '(#\h "help") #f #f
+ (lambda args
+ (show-help)
+ (exit 0)))
+ (option '(#\l "list-checkers") #f #f
+ (lambda args
+ (list-checkers-and-exit)))
+ (option '(#\V "version") #f #f
+ (lambda args
+ (show-version-and-exit "guix lint")))))
+
+\f
+;;;
+;;; Helpers
+;;;
+(define* (emit-warning package message #:optional field)
+ ;; Emit a warning about PACKAGE, printing the location of FIELD if it is
+ ;; given, the location of PACKAGE otherwise, the full name of PACKAGE and the
+ ;; provided MESSAGE.
+ (let ((loc (or (package-field-location package field)
+ (package-location package))))
+ (warning (_ "~a: ~a: ~a~%")
+ (location->string loc)
+ (package-full-name package)
+ message)))
+
+\f
+;;;
+;;; Checkers
+;;;
+(define-record-type* <lint-checker>
+ lint-checker make-lint-checker
+ lint-checker?
+ ;; TODO: add a 'certainty' field that shows how confident we are in the
+ ;; checker. Then allow users to only run checkers that have a certain
+ ;; 'certainty' level.
+ (name lint-checker-name)
+ (description lint-checker-description)
+ (check lint-checker-check))
+
+(define (list-checkers-and-exit)
+ ;; Print information about all available checkers and exit.
+ (format #t (_ "Available checkers:~%"))
+ (for-each (lambda (checker)
+ (format #t "- ~a: ~a~%"
+ (lint-checker-name checker)
+ (lint-checker-description checker)))
+ %checkers)
+ (exit 0))
+
+(define (check-inputs-should-be-native package)
+ ;; Emit a warning if some inputs of PACKAGE are likely to belong to its
+ ;; native inputs.
+ (let ((inputs (package-inputs package)))
+ (match inputs
+ (((labels packages . _) ...)
+ (when (member "pkg-config"
+ (map package-name (filter package? packages)))
+ (emit-warning package
+ "pkg-config should probably be a native input"
+ 'inputs))))))
+
+
+(define (check-synopsis-style package)
+ ;; Emit a warning if stylistic issues are found in the synopsis of PACKAGE.
+ (define (check-final-period synopsis)
+ ;; Synopsis should not end with a period, except for some special cases.
+ (if (and (string=? (string-take-right synopsis 1) ".")
+ (not (string=? (string-take-right synopsis 4) "etc.")))
+ (emit-warning package
+ "no period allowed at the end of the synopsis"
+ 'synopsis)))
+
+ (define (check-start-article synopsis)
+ (if (or (string=? (string-take synopsis 2) "A ")
+ (string=? (string-take synopsis 3) "An "))
+ (emit-warning package
+ "no article allowed at the beginning of the synopsis"
+ 'synopsis)))
+
+ (let ((synopsis (package-synopsis package)))
+ (if (string? synopsis)
+ (begin
+ (check-final-period synopsis)
+ (check-start-article synopsis)))))
+
+(define (check-patches package)
+ ;; Emit a warning if the patches requires by PACKAGE are badly named.
+ (let ((patches (and=> (package-source package) origin-patches))
+ (name (package-name package))
+ (full-name (package-full-name package)))
+ (if (and patches
+ (any (lambda (patch)
+ (let ((filename (basename patch)))
+ (not (or (eq? (string-contains filename name) 0)
+ (eq? (string-contains filename full-name) 0)))))
+ patches))
+ (emit-warning package
+ "file names of patches should start with the package name"
+ 'patches))))
+
+(define %checkers
+ (list
+ (lint-checker
+ (name "inputs-should-be-native")
+ (description "Identify inputs that should be native inputs")
+ (check check-inputs-should-be-native))
+ (lint-checker
+ (name "patch-filenames")
+ (description "Validate filenames of patches")
+ (check check-patches))
+ (lint-checker
+ (name "synopsis")
+ (description "Validate package synopsis")
+ (check check-synopsis-style))))
+
+(define (run-checkers package)
+ ;; Run all the checkers on PACKAGE.
+ (for-each (lambda (checker)
+ ((lint-checker-check checker) package))
+ %checkers))
+
+\f
+;;;
+;;; Entry Point
+;;;
+
+(define (guix-lint . args)
+ (define (parse-options)
+ ;; Return the alist of option values.
+ (args-fold* args %options
+ (lambda (opt name arg result)
+ (leave (_ "~A: unrecognized option~%") name))
+ (lambda (arg result)
+ (alist-cons 'argument arg result))
+ %default-options))
+
+ (let* ((opts (parse-options))
+ (args (filter-map (match-lambda
+ (('argument . value)
+ value)
+ (_ #f))
+ (reverse opts))))
+
+
+ (if (null? args)
+ (fold-packages (lambda (p r) (run-checkers p)) '())
+ (for-each
+ (lambda (spec)
+ (run-checkers spec))
+ (map specification->package args)))))
diff --git a/po/guix/Makevars b/po/guix/Makevars
index 87bb438..f5b498c 100644
--- a/po/guix/Makevars
+++ b/po/guix/Makevars
@@ -10,7 +10,8 @@ top_builddir = ../..
XGETTEXT_OPTIONS = \
--language=Scheme --from-code=UTF-8 \
--keyword=_ --keyword=N_ \
- --keyword=message
+ --keyword=message \
+ --keyword=description
COPYRIGHT_HOLDER = Ludovic Courtès
diff --git a/po/guix/POTFILES.in b/po/guix/POTFILES.in
index bf2d313..5cc68ff 100644
--- a/po/guix/POTFILES.in
+++ b/po/guix/POTFILES.in
@@ -10,6 +10,7 @@ guix/scripts/pull.scm
guix/scripts/substitute-binary.scm
guix/scripts/authenticate.scm
guix/scripts/system.scm
+guix/scripts/lint.scm
guix/gnu-maintenance.scm
guix/ui.scm
guix/http-client.scm
diff --git a/tests/lint.scm b/tests/lint.scm
new file mode 100644
index 0000000..f6dae47
--- /dev/null
+++ b/tests/lint.scm
@@ -0,0 +1,110 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2012, 2013 Cyril Roelandt <tipecaml@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
+
+
+(define-module (test-packages)
+ #:use-module (guix build download)
+ #:use-module (guix build-system gnu)
+ #:use-module (guix packages)
+ #:use-module (guix scripts lint)
+ #:use-module (guix ui)
+ #:use-module (gnu packages)
+ #:use-module (gnu packages pkg-config)
+ #:use-module (srfi srfi-64))
+
+;; Test the linter.
+
+\f
+(test-begin "lint")
+
+(define-syntax-rule (dummy-package name* extra-fields ...)
+ (package extra-fields ... (name name*) (version "0") (source #f)
+ (build-system gnu-build-system)
+ (synopsis #f) (description #f)
+ (home-page #f) (license #f) ))
+
+(define (call-with-warnings thunk)
+ (let ((port (open-output-string)))
+ (parameterize ((guix-warning-port port))
+ (thunk))
+ (get-output-string port)))
+
+(test-assert "synopsis: ends with a period"
+ (->bool
+ (string-contains (call-with-warnings
+ (lambda ()
+ (let ((pkg (dummy-package "x"
+ (synopsis "Bad synopsis."))))
+ (check-synopsis-style pkg))))
+ "no period allowed at the end of the synopsis")))
+
+(test-assert "synopsis: ends with 'etc.'"
+ (->bool
+ (string-null? (call-with-warnings
+ (lambda ()
+ (let ((pkg (dummy-package "x"
+ (synopsis "Foo, bar, etc."))))
+ (check-synopsis-style pkg)))))))
+
+(test-assert "synopsis: starts with 'A'"
+ (->bool
+ (string-contains (call-with-warnings
+ (lambda ()
+ (let ((pkg (dummy-package "x"
+ (synopsis "A bad synopŝis"))))
+ (check-synopsis-style pkg))))
+ "no article allowed at the beginning of the synopsis")))
+
+(test-assert "synopsis: starts with 'An'"
+ (->bool
+ (string-contains (call-with-warnings
+ (lambda ()
+ (let ((pkg (dummy-package "x"
+ (synopsis "An awful synopsis"))))
+ (check-synopsis-style pkg))))
+ "no article allowed at the beginning of the synopsis")))
+
+(test-assert "inputs: pkg-config is probably a native input"
+ (->bool
+ (string-contains
+ (call-with-warnings
+ (lambda ()
+ (let ((pkg (dummy-package "x"
+ (inputs `(("pkg-config" ,pkg-config))))))
+ (check-inputs-should-be-native pkg))))
+ "pkg-config should probably be a native input")))
+
+(test-assert "patches: file names"
+ (->bool
+ (string-contains
+ (call-with-warnings
+ (lambda ()
+ (let ((pkg (dummy-package "x"
+ (source
+ (origin
+ (method url-fetch)
+ (uri "someurl")
+ (sha256 "somesha")
+ (patches (list "/path/to/y.patch")))))))
+ (check-patches pkg))))
+ "file names of patches should start with the package name")))
+
+(test-end "lint")
+
+\f
+(exit (= (test-runner-fail-count (test-runner-current)) 0))
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] Move specification->package to gnu/packages.scm.
2014-09-01 0:39 ` [PATCH 1/2] Move specification->package to gnu/packages.scm Cyril Roelandt
2014-09-01 0:39 ` [PATCH 2/2] scripts: add guix lint Cyril Roelandt
@ 2014-09-01 8:55 ` Ludovic Courtès
1 sibling, 0 replies; 15+ messages in thread
From: Ludovic Courtès @ 2014-09-01 8:55 UTC (permalink / raw)
To: Cyril Roelandt; +Cc: guix-devel
Cyril Roelandt <tipecaml@gmail.com> skribis:
> * guix/scripts/build.scm (specification->package): Move from here...
> * gnu/packages.scm: ... to here.
LGTM, please push.
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] scripts: add guix lint
2014-09-01 0:39 ` [PATCH 2/2] scripts: add guix lint Cyril Roelandt
@ 2014-09-01 21:19 ` Ludovic Courtès
0 siblings, 0 replies; 15+ messages in thread
From: Ludovic Courtès @ 2014-09-01 21:19 UTC (permalink / raw)
To: Cyril Roelandt; +Cc: guix-devel
Cyril Roelandt <tipecaml@gmail.com> skribis:
> * guix/scripts/lint.scm: New file. Defines a 'lint' tool for Guix packages.
> * tests/lint.scm: New file.
> * Makefile.am (MODULES, SCM_TESTS): Add them.
> * po/guix/Makevars: Update appropriately.
> * po/guix/POTFILES.in: Update appropriately.
> * doc/guix.texi: Document "guix lint".
Looks good, please push.
Thank you!
Ludo’.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2014-09-01 21:20 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-21 23:51 [PATCH 0/2] Add "guix lint" Cyril Roelandt
2014-07-21 23:51 ` [PATCH 1/2] scripts: add guix lint Cyril Roelandt
2014-07-22 9:03 ` Ludovic Courtès
2014-07-22 13:38 ` Ludovic Courtès
2014-07-22 14:31 ` Eric Bavier
2014-07-22 15:26 ` Ludovic Courtès
2014-08-25 1:52 ` [PATCH] " Cyril Roelandt
2014-08-25 22:44 ` Ludovic Courtès
2014-09-01 0:39 ` [PATCH 1/2] Move specification->package to gnu/packages.scm Cyril Roelandt
2014-09-01 0:39 ` [PATCH 2/2] scripts: add guix lint Cyril Roelandt
2014-09-01 21:19 ` Ludovic Courtès
2014-09-01 8:55 ` [PATCH 1/2] Move specification->package to gnu/packages.scm Ludovic Courtès
2014-07-21 23:51 ` [PATCH 2/2] gnu/packages: Remove trailing periods in some synopses Cyril Roelandt
2014-07-22 19:27 ` [PATCH 0/2] Add "guix lint" Andreas Enge
2014-07-22 20:25 ` Ludovic Courtès
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).