Eric Bavier <[email protected]> skribis: > From 0311d5b383003600ac43d3a9bfdec0ad3c398db2 Mon Sep 17 00:00:00 2001 > From: Eric Bavier <[email protected]> > Date: Sun, 23 Aug 2015 18:00:45 -0500 > Subject: [PATCH] guix: lint: Check for version-only origin file names. > > * guix/scripts/lint.scm (check-source): Emit warning if source filename > contains only the version of the package. > * tests/lint.scm ("source: filename", "source: filename v", > "source: filename valid"): New tests. > * doc/guix.texi (Invoking guix lint): Mention file name check. > Offending packages updated.
This is useful, thanks for looking into it. I would prefer it to make a separate linter, like ‘source-file-name’. The reason is that ‘source’ is a relatively expensive check, since it needs to probe URLs (so you might want to skip it in some cases), whereas the linter your propose is lightweight. WDYT? > --- a/gnu/packages/algebra.scm > +++ b/gnu/packages/algebra.scm > @@ -386,6 +386,7 @@ cosine/ sine transforms or DCT/DST).") > (method url-fetch) > (uri (string-append "https://bitbucket.org/eigen/eigen/get/"; > version ".tar.bz2")) > + (file-name (string-append name "-" version ".tar.bz2")) Could you make these package updates a separate patch? Some may trigger large rebuilds, so you may have to keep them for ‘core-updates’ or such. > + (define (origin-version-name? origin) > + ;; Return #t if the source file name contains only a version; indicates > + ;; that the origin needs a 'file-name' field. > + (let ((filename (store-path-package-name > + (with-store store > + (derivation->output-path > + (package-source-derivation store origin))))) > + (version (package-version package))) > + (or (string-prefix? version filename) > + ;; Common in many projects is for the filename to start with a "v" > + ;; followed by the version, e.g. "v3.2.0.tar.gz". > + (string-prefix? (string-append "v" version) filename)))) Opening a connection to the store in the middle of the code (‘with-store’) is Bad Practice. ;-) I think this can actually be made simpler, with something akin to what ‘node-full-name’ does in guix/scripts/graph.scm. Maybe we could extract an ‘origin-actual-file-name’ procedure from that and move it to (guix packages). WDYT? > +(test-assert "source: filename" “file name” (two words). Could you send an updated patch? Thanks, Ludo’.
