Something happened to the attachments.  Let's try that again.  Sorry 
about that.

`~Eric

On 2015-09-10 15:50, Eric Bavier wrote:
> On Fri, 28 Aug 2015 09:48:48 +0200
> ludo@gnu.org (Ludovic Courtès) wrote:
> 
>> Eric Bavier <ericbavier@openmailbox.org> skribis:
>> 
>> > From 0311d5b383003600ac43d3a9bfdec0ad3c398db2 Mon Sep 17 00:00:00 2001
>> > From: Eric Bavier <bavier@member.fsf.org>
>> > 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.
> 
> Thanks for the review!
> 
>> 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.
> 
> Makes sense.
> 
>> 
>> > --- 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.
> 
> I've left the package updates out of the attached patches.
> 
>> 
>> > +  (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?
> 
> The first attached patch does this.  Is using the basename of the
> source URI always accurate?  I.e. are there cases where the store file
> name might not match the URI's basename?  This uncertainty, I think, is
> what caused me to use store-path-package-name initially.
> 
> This revised patch might actually be considered "more accurate" in that
> the checker now flags origins from 'git-reference' et al where no
> 'file-name' field is declared.
> 
> `~Eric