Re: [PATCH] gnu-maintenance: Replace 'official-gnu-packages' with 'find-packages'.

[ludo@gnu.org (Ludovic Courtès)]

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>> Also, I think it should process fields, and return an alist, or even
>> better, a ‘gnu-package-descriptor’ record (say).
>>
>>   (define-record-type <gnu-package-descriptor>
>>     gnu-package-descriptor?
>>     (gnu-package-descriptor name logo-url doc-category ...)
>>     ...
>>     )
>>
>>   (official-gnu-packages)
>>   => (#<gnu-package-descriptor name: "guix" logo-url: "http://..." ...#> ...)
>>
>> WDYT?
>
> Oh, I forgot about this when I was writing the attached patch.  Is it
> fine?  Should I rewrite it using records?

Yes please, that would be better.

> I guess that it will work faster if I use 'cons' in 'loop' instead of
> 'append'.  Is it worth it?

In general yes: always cons, and reverse at the end (that makes the
complexity linear.)

> Examples:
>
> scheme@(guile-user)> ,use (guix gnu-maintenance)
> scheme@(guile-user)> (find-packages "guix")
> $1 = (("package: guix" "logo: /software/guix/graphics/guix-logo.small.png" "doc-category: Sysadmin" "doc-summary: Managing installed software packages and versions" "doc-url: none" "gplv3-status: should-be-ok" "activity-status: newpkg/20121117"))

Since it has to read all the package file, I’d rather keep an
‘official-gnu-packages’ that would return a list of all the packages
(we’re talking about less than 1 MiB of RAM), and then use ‘filter’ on
that.

(Alternately, ‘official-gnu-packages’ could be a stream (in the sense of
(ice-9 stream)), but then there’s going to be issues with connection
timeouts, so a plain old list is better.)

> +  "Return an input port with the textual data at URI, a string."
> +  (let*-values (((resp port)
> +                 (http-get* (string->uri uri)))

This code is run by the user’s Guile, which may be older than 2.0.7
(‘http-get*’ was introduced in 2.0.7), so you can’t rely on it.

What you can do is something along the lines of what (guix build
download) does, but always return a port.  Maybe there’s a way to share
code.

A couple of stylistic comments:

> +    (define (group-packages port state)
> +      ;; Return a list of packages.
> +      (let ((line (read-line port)))
> +        (define (loop str)
> +          (match str
> +            ('""
> +             (group-packages port (append state '(()))))
> +            (str
> +             (group-packages port (append (drop-right state 1)
> +                                          (list (append (last state)
> +                                                        (list str))))))))

It’s called ‘loop’ but it doesn’t loop.

I’d rather change ‘group-packages’ to ‘read-package-fields’ or something
like that.

> +            (filter (lambda (lst)
> +                      (not (null-list? lst)))
> +                    state)

(remove null-list? state)

> +                  (and=> (regexp-exec package-line-rx (first sublst))
> +                         (lambda _
> +                           sublst)))

identity

Thanks,
Ludo’.